If else not running as expected

Hi again, this project is from my wifi air freshener that I did a while back and trying to expand on it.
It runs on d1 wemos board and a board I had made and to be honest it worked :slight_smile: anyways, It has a separate output for LED and one for airfreshener. The LED is intened to be an indicator its about to spray, but havnt gotten it to work as intended. It is currently a "night light" lol
It currently works as designed, I can control LED or airfreshner with Alexa.

What I want to do is:
Make the LED blink 5 times as a warning before activation, but I couldn't have a long code / delay or even milis before Alexa says device is not responding. I eventually figured out I need to set a state, and then read the state and then running a blink without delay function which works :

void loop()
{

  if (wifiConnected)
  {
    upnpBroadcastResponder.serverLoop();
    kitchen->serverLoop();
  }


  // blinks led, does not cause Alexa to say device not responding :)
  if (wifiState == HIGH) {               // Set HIGH from Alexa ON

    unsigned long currentMillis = millis();
    if (currentMillis - previousMillis >= interval && blinkCount <=10) {           // on and off counts as 1 each , 10 = 10 blinks

      previousMillis = currentMillis;            // save the last time you blinked the LED

      if (wifiState1 == HIGH) {                   // if the LED is off turn it on and vice-versa:
        wifiState1 = LOW;
      } 
      else {
        wifiState1 = HIGH;
      }
      digitalWrite(wifi_led, wifiState1);        // set the LED with the ledState of the variable:
      blinkCount++;
      Serial.println (blinkCount);
      Serial.println ("blink");
      Serial.println ();
    }
//     else {
//      digitalWrite(wifi_led, LOW);              // LOW turns LED on
//      Serial.println (blinkCount);
//      Serial.println ("else");
//      Serial.println ();
//    }
  }
}
'''

output is as expected, it blinks every even number. It counts off as 1 "blink" thats fine

21:11:18.547 -> 
21:11:20.434 -> 2
21:11:20.434 -> blink
21:11:20.434 -> 
21:11:23.429 -> 3
21:11:23.429 -> blink
21:11:23.429 -> 
21:11:26.410 -> 4
21:11:26.410 -> blink
21:11:26.410 -> 
21:11:29.408 -> 5
21:11:29.408 -> blink
21:11:29.408 -> 
21:11:32.404 -> 6
21:11:32.404 -> blink

led blinks 5 times and stops, end in OFF

The problem is when I want an else after, to turn on the the LED ON and stay on (in this sketch above uncommented ) the serial monitor output is always showing "else" and the counter increases and the LED does not blink.

20:54:11.052 -> 0
20:54:11.052 -> else
20:54:11.052 -> 
20:54:11.052 -> 0
20:54:11.052 -> else
20:54:11.052 -> 
20:54:11.052 -> 0
20:54:11.052 -> else
20:54:11.052 -> 
20:54:11.052 -> 0
20:54:11.052 -> else
20:54:11.052 -> 
20:54:11.052 -> 0
20:54:11.052 -> else
20:54:11.052 -> 
20:54:11.052 -> 0
20:54:11.052 -> else
20:54:11.052 -> 
20:54:11.052 -> 0
20:54:11.052 -> else
20:54:11.052 -> 
20:54:11.052 -> 0
20:54:11.052 -> else
20:54:11.052 -> 
20:54:11.052 -> 0
20:54:11.052 -> else
20:54:11.052 -> 
20:54:11.052 -> 0

I expected
if (currentMillis - previousMillis >= interval && blinkCount <=10)
to run the blink + else count
and then when it gets to 10, it runs the final else

      digitalWrite(wifi_led, LOW);              // LOW turns LED on
      Serial.println (blinkCount);
      Serial.println ("else");
      Serial.println ();
   }

but the led stays on and counts to 11 and stops

21:15:37.275 -> 11
21:15:37.275 -> else
21:15:37.275 -> 
21:15:37.275 -> 11
21:15:37.275 -> else
21:15:37.275 -> 
21:15:37.275 -> 11

I'm missing why it goes strait to the final else.
Thanks
Clint

the whole part of the main sketch


#include <ESP8266WiFi.h>
#include <ESP8266WebServer.h>
#include <WiFiUdp.h>
#include <functional>
#include "Switch.h"
#include "UpnpBroadcastResponder.h"
#include "CallbackFunction.h"
#include "credentials.h"

unsigned long currentMillis;             // will store current millis value
unsigned long previousMillis = 0;        // will store last time request was requested
const long interval = 3000;               // .5 seconds interval

int wifi_led = 2;                        // 2 = BUILTIN_LED 2 by antenna
int ledState = LOW;
int wifiState = 3;
int wifiState1 = 4;
int buttonState = 0;
int lastButtonState = 0;
int blinkCount = 0; // Initialize the blink counter

boolean connectWifi();

// on/off callbacks

bool kitchenOn();
bool kitchenOff();


// Change this before you flash
//#######################################
const char* ssid = WIFI_SSID;            // enter your access point/wifi router name
const char* password = WIFI_PASS;        // enter router password
//#######################################

boolean wifiConnected = false;

UpnpBroadcastResponder upnpBroadcastResponder;

Switch *kitchen = NULL;

bool iskitchenOn = true;


void setup()
{
  Serial.begin(115200);
  pinMode(wifi_led, OUTPUT);
  digitalWrite(wifi_led, HIGH);         // turn LED off, not sure why HIGH = off but it works as intended


  // Initialise wifi connection
  wifiConnected = connectWifi();

  if (wifiConnected) {
    upnpBroadcastResponder.beginUdpMulticast();

    // Define your switches here. Max 14
    // Format: Alexa invocation name, local port no, on callback, off callback


    kitchen = new Switch("kitchen", 83, kitchenOn, kitchenOff);


    Serial.println("Adding switches upnp broadcast responder");

    upnpBroadcastResponder.addDevice(*kitchen);

  }
}

void loop()
{

  if (wifiConnected)
  {
    upnpBroadcastResponder.serverLoop();
    kitchen->serverLoop();
  }


  // blinks led, does not cause Alexa to say device not responding :)
  if (wifiState == HIGH) {               // Set HIGH from Alexa ON

    unsigned long currentMillis = millis();
    if (currentMillis - previousMillis >= interval && blinkCount <=10) {           // on and off counts as 1 each , 10 = 10 blinks

      previousMillis = currentMillis;            // save the last time you blinked the LED

      if (wifiState1 == HIGH) {                   // if the LED is off turn it on and vice-versa:
        wifiState1 = LOW;
      } 
      else {
        wifiState1 = HIGH;
      }
      digitalWrite(wifi_led, wifiState1);        // set the LED with the ledState of the variable:
      blinkCount++;
      Serial.println (blinkCount);
      Serial.println ("blink");
      Serial.println ();
    }
//     else {
//      digitalWrite(wifi_led, LOW);              // LOW turns LED on
//      Serial.println (blinkCount);
//      Serial.println ("else");
//      Serial.println ();
//    }
  }
}


bool kitchenOn() {
  Serial.print("wifi_led turned on ...");
  wifiState = HIGH;

  //  af_on();                                      // look at bottom for funcion
  //  Serial.print("wifi_led turned on ...");
  //  digitalWrite(wifi_led, LOW);
  //  previousMillis = millis();                    // time when command was given
  return true;                                      // Shows ON in app
}

bool kitchenOff() {
  Serial.print("wifi_led turned off ...");
  wifiState = LOW;                                   // LOW = LED ON
  digitalWrite (wifi_led, HIGH);
  return false;                                      // Shows OFF in app
}


// connect to wifi – returns true if successful or false if not
boolean connectWifi() {
  boolean state = true;
  int i = 0;

  WiFi.mode(WIFI_STA);
  WiFi.begin(ssid, password);
  Serial.println("");
  Serial.println("Connecting to WiFi");

  // Wait for connection
  Serial.print("Connecting ...");
  while (WiFi.status() != WL_CONNECTED) {
    delay(700);
    Serial.print(".");
    if (i > 15) {
      state = false;
      break;
    }
    i++;
  }

  if (state) {
    Serial.println("");
    Serial.print("Connected to ");
    Serial.println(ssid);
    Serial.print("IP address: ");
    Serial.println(WiFi.localIP());
    digitalWrite(wifi_led, LOW);                   // LOW turns the led by wifi antenna on
    delay(500);
    digitalWrite(wifi_led, HIGH);
    previousMillis = millis();                     // time stamp when command was given
  }
  else {
    Serial.println("");
    Serial.println("Connection failed.");
  }
  return state;
}

The then part of if (currentMillis - previousMillis >= interval && blinkCount <=10) { get executed once every interval milliseconds while blinkCount is less than or equal to 10.

The else part gets executed Every.Single.Time. except those 10 times.

You might want to think about that.

And try running the sketch below with whatever your wifi_led is defined as. And think about whether your LED turns on when its pin is set LOW or HIGH...

const byte wifi_led = LED_BUILTIN;
unsigned blinkCount = 0;
int wifiState1 = LOW;
unsigned long currentMillis, previousMillis;
const unsigned interval = 500;

void setup() {
   Serial.begin(115200);
   pinMode(wifi_led, OUTPUT);
}

void loop() {
   unsigned long currentMillis = millis();
   if( currentMillis - previousMillis >= interval ) {
      previousMillis = currentMillis;
      if( blinkCount <= 10 ) {  // on and off counts as 1 each , 10 = 10 blinks

         previousMillis = currentMillis;  // save the last time you blinked the LED

         if( wifiState1 == HIGH ) {  // if the LED is off turn it on and vice-versa:
            wifiState1 = LOW;
         } else {
            wifiState1 = HIGH;
         }
         digitalWrite(wifi_led, wifiState1);  // set the LED with the ledState of the variable:
         blinkCount++;
         Serial.println(blinkCount);
         Serial.println("blink");
         Serial.println();
      } else if( blinkCount == 11 ) {
         ++blinkCount;
         digitalWrite(wifi_led, LOW);  // LOW turns LED on
         Serial.println(blinkCount);
         Serial.println("else");
         Serial.println();
      }
   }
}

And as @LarryD might be along any time to add, think about defining some constants:

#define LED_ON LOW
#define LED_OFF HIGH
.
.
.
int wifiState1 = LED_OFF;
.
.
.
void setup() {
   Serial.begin(115200)
   pinMode(wifi_led, OUTPUT);
   digitalWrite(wifi_led, LED_OFF);
.
.
.
         if( wifiState1 == LED_ON ) {  // if the LED is off turn it on and vice-versa:
            wifiState1 = LED_OFF;
         } else {
            wifiState1 = LED_ON;
         }
         digitalWrite(wifi_led, wifiState1);
.
.
.
      } else if( blinkCount == 11 ) {
         ++blinkCount;
         digitalWrite(wifi_led, LED_OFF);

PS Your LED is on when a LOW is written because the cathode is connected to the output pin and the anode is connected + 3.3V. There's a resistor in there as well, but it's not germane to the subject at hand.

1 Like

why does the final else run every time? I only want it to run after its done counting and then and then run the final else when it ran out of counts.

ill have to try this tomorrow after work, i copied the void loop and currently didnt work as expected.
the void loop just runs the bool

bool kitchenOn() {
  Serial.print("wifi_led turned on ...");
  wifiState = HIGH;

  //  af_on();                                      // look at bottom for funcion
  //  Serial.print("wifi_led turned on ...");
  //  digitalWrite(wifi_led, LOW);
  //  previousMillis = millis();                    // time when command was given
  return true;                                      // Shows ON in app
}

and sets
wifiState = HIGH;
the loop checks and sees if it is high, and then runs the hopes and dreams goal
blink 5 times
then
stay on

this sketch, the alexa word is called "kitchen" to not mess up the current airfrehsner wemos setup **

You're not thinking it through.

loop is being constantly called. Many hundreds if not thousands of time each millisecond.

Each time loop() is executed, the if then... else statement is evaluated.

Every interval milliseconds until blinkCount exceeds 10, the if statement will evaluate to true and the then portion will be executed.

Every other time through loop (and there will be thousands of times through loop in the interval), the if is false, and the else portion will be executed.

look this over

const byte PinLed = LED_BUILTIN;
enum { LedOff = HIGH, LedOn = LOW };

const unsigned long MsecBlink = 200;
      unsigned long previousMillis;
unsigned blinkCount;

// -----------------------------------------------------------------------------
void loop ()
{

    // -------------------------------------
    // timer
    unsigned long currentMillis = millis();

    if (0 < blinkCount && currentMillis - previousMillis >= MsecBlink)  {
        previousMillis = currentMillis;         // restart timer

        if (0 == --blinkCount)
            digitalWrite (PinLed, LedOff);
        else
            digitalWrite (PinLed, ! digitalRead (PinLed));
    }

    // -------------------------------------
    // other stuff
    if (Serial.available ())  {
        switch (Serial.read ())  {
        case 'x':
            blinkCount     = 20;
            previousMillis = currentMillis;
            break;
        }
    }
}

// -----------------------------------------------------------------------------
void setup ()
{
    Serial.begin (9600);

    pinMode      (PinLed, OUTPUT);
    digitalWrite (PinLed, LedOff);
}
1 Like

it's the condition when the timer doesn't expire -- < interval

thank you @van_der_decken @gcjr for taking time to help but I am still not under standing why the this else runs 1st when the if is true up untill count 10.

  void loop()
  {
  
    if (wifiConnected)
    {
      upnpBroadcastResponder.serverLoop();
      kitchen->serverLoop();
    }
  
    if (wifiState == HIGH) {                         // Set HIGH from Alexa ON command
  
      unsigned long currentMillis = millis();
      if (currentMillis - previousMillis >= interval && blinkCount <= 10) {
        previousMillis = currentMillis;             // save the last time you blinked the LED
        if (wifiState1 == HIGH) {                    // if the LED is off turn it on and vice-versa:
          wifiState1 = LOW;
        } else {
          wifiState1 = HIGH;
        }
        digitalWrite(wifi_led, wifiState1);         // set the LED with the ledState of the variable:
        blinkCount++;                               // Increment the blink count
      }
      
//      else {                                       //goal with final else to turn on led and stay on
//        wifiState = LOW ;  
//        Serial.print ("wifiState = ");
//        Serial.println (wifiState);
//        digitalWrite (wifi_led, LOW);
//        blinkCount =0;
//        Serial.print ("blinkCount = ");
//        Serial.println (blinkCount);
//        Serial.println ();
//      }
    }
  }

wemos LED blinks led 5 times and turns off as expected when I tell Alexa turn on "kitchen".

When I uncomment out the final else:

//      else {                                       //goal with final else to turn on led and stay on
//        wifiState = LOW ;  
//        Serial.print ("wifiState = ");
//        Serial.println (wifiState);
//        digitalWrite (wifi_led, LOW);
//        blinkCount =0;
//        Serial.print ("blinkCount = ");
//        Serial.println (blinkCount);
//        Serial.println ();
//      }

and upload it, it runs the ELSE instantly, led in this code is off, and displays

19:19:05.861 -> wifiState = 0
19:19:05.861 -> blinkCount = 0

even though the IF above code runs "true" (untill 10th count). This is what has me confused.

enum is something I never heard of before (beginner),I googled it and super cool. I will incorporate it into the code later once i get it the ELSE working as expected.

From my limited experience the
if (currentMillis - previousMillis >= interval && blinkCount <= 10)
is true, it wont run untill false , 11th count and then else will run

else {                                       //goal with final else to turn on led and stay on
        wifiState = LOW ;  
        Serial.print ("wifiState = ");
        Serial.println (wifiState);
        digitalWrite (wifi_led, LOW);
        blinkCount =0;
        Serial.print ("blinkCount = ");
        Serial.println (blinkCount);
        Serial.println ();
      } 

Thanks again for taking time to help with this :slight_smile:
clint

edit, maybe its too many else statements? Ill try and remove the last ELSE, and run another if statement, ... if over 11 counts, run this code.... ill try that in a bit. I probably just be misunderstanding how to use else and maybe brackets are placed incorrectly?

void loop()
{

  if (wifiConnected)
  {
    upnpBroadcastResponder.serverLoop();
    kitchen->serverLoop();
  }

  if (wifiState == HIGH) {                         // Set HIGH from Alexa ON command

    unsigned long currentMillis = millis();
    if (currentMillis - previousMillis >= interval && blinkCount <= 10) {
      previousMillis = currentMillis;              // save the last time you blinked the LED
      if (wifiState1 == HIGH) {                    // if the LED is off turn it on and vice-versa:
        wifiState1 = LOW;
        Serial.println (" blinking ON");
      } else {
        wifiState1 = HIGH;
        Serial.println (" blinking OFF ");
      }
      digitalWrite(wifi_led, wifiState1);         // set the LED with the ledState of the variable:
      blinkCount++;                               // Increment the blink count

    }
    if (blinkCount >= 11) {                       //goal with final else to turn on led and stay on
      wifiState = LOW ;                           // LED is on
      Serial.println ("wifiState = ");
      Serial.println (wifiState);
      digitalWrite (wifi_led, LOW);
      blinkCount = 0;
      Serial.print ("blinkCount = ");
      Serial.println (blinkCount);
      Serial.println ();
    }
  }
}

with the last ELSE removed / and moved into and new IF will run as expected.. now to expand on this

and wow, its working thus far :slight_smile: besides not understanding why inital else didnt work,
the updated the void loop ()

void loop()
{

  if (wifiConnected)
  {
    upnpBroadcastResponder.serverLoop();
    kitchen->serverLoop();
  }

  if (wifiState == HIGH) {                         // Set HIGH from Alexa ON command

    unsigned long currentMillis = millis();
    if (currentMillis - previousMillis >= interval && blinkCount <= 10) {
      previousMillis = currentMillis;              // save the last time you blinked the LED
      if (wifiState1 == HIGH) {                    // if the LED is off turn it on and vice-versa:
        wifiState1 = LOW;
        Serial.println (" blinking ON");
      } else {
        wifiState1 = HIGH;
        Serial.println (" blinking OFF ");
      }
      digitalWrite(wifi_led, wifiState1);         // set the LED with the ledState of the variable:
      blinkCount++;                               // Increment the blink count

    }
    if (blinkCount >= 11) {                       //goal with final else to turn on led and stay on
      wifiState = LOW ;                           // LED is on
      Serial.print ("wifiState = ");
      Serial.println (wifiState);
      digitalWrite (wifi_led, LOW);
      blinkCount = 0;
      Serial.print ("blinkCount reset to = ");
      Serial.println (blinkCount);
      Serial.println ();
    }
  }

  if (wifiState == LOW) {                       //  LED turned on from ls
    if (millis() - previousMillis >= interval2) {
      digitalWrite (wifi_led, HIGH);
      Serial.println(" wifi_led timer turned OFF");
      wifiState = 3;
    }

  }
}

As untidy as this current code might be right now, when Alexa is told to turn "kitchen on" the Wemos blinks 5 times and stays on for interval , and then turns off after interval2 (as intended)!!! Woot woot!! so the purpose of turning this off at the end is to not give the air freshener power and waste sprays of freshness when no one is home or asleep.... It takes about 3 seconds to go through a power up cycle and one spray cycle. Once off, it doesnt spray/waste

20:35:29.312 -> </s:Body> </s:Envelope>
20:35:29.312 -> 
20:35:29.874 ->  blinking OFF 
20:35:30.810 ->  blinking ON
20:35:31.840 ->  blinking OFF 
20:35:32.822 ->  blinking ON
20:35:33.805 ->  blinking OFF 
20:35:34.835 ->  blinking ON
20:35:35.818 ->  blinking OFF 
20:35:36.802 ->  blinking ON
20:35:37.833 ->  blinking OFF 
20:35:38.816 ->  blinking ON
20:35:38.816 -> wifiState = 0
20:35:38.816 -> blinkCount reset to = 0
20:35:38.816 -> 
20:35:41.800 ->  wifi_led timer turned OFF

Im not going to say how many hours it took to get this, this far to get an led to blink and not get Alexa to report "Device unsponisve" :wink: I want to include enum as it makes code look easier to read!

I am about to research the use of -- before a int... it is confusing to me at the moment.
I see it as If ( zero is equal less than blinkCount) ??

clint

Also known as "decrement"

1 Like

This topic was automatically closed 180 days after the last reply. New replies are no longer allowed.