If/Else issue... seems so simple.

Preparing for float data from a pH probe. For now I’m simulating pH values through the serial monitor. I take the serial input, atof it to float, then multiply it by 100 into an int. The float to int is I think the recommended way to do comparisons, right?

Ok so at boot the value is 0. The LED blinks slowly. Good.

I send it a 6.7. The value to assess is int 670. Since it’s still below the threshold if 710, an LED continues to blink slowly. Good.

I then feed it a 7.3. The value to assess moves to 730. Since that’s above the 710 floor and below the 760 ceiling, the LED comes on solid. Good.

I feed it an 8.4. The value to assess moves to 840. That’s above the ceiling and the LED blinks in double time. Good.

I feed it a 7.2 to put it back in the range the LED goes solid, we’re still good.

NOW THE TROUBLE

Sending in anything below the floor 7.1 does EITHER nothing or turns the LED out! I can go back above the ceiling OK and back into range OK, the LED acts properly, just not below the floor 710. The serial output still shows the int is (for example) 688 in this case, but the LED is out.

So I can go up the ladder, but not back down, so to speak.

Any thoughts?

int phVal;
float ph;
boolean newData = false;
const byte numChars = 32;
char receivedChars[numChars]; // an array to store the received data
byte phLoops;
unsigned long previousMillis;
const int PHLED  = 12;  //ph LED

void setup() {
 Serial.begin(115200);
   pinMode(PHLED, OUTPUT); 
   digitalWrite(PHLED, LOW); 
}

void loop() {
  recvWithEndMarker();
  showNewData();  

 phVal = ph * 100;

 if (phVal > 760) {
  ledTwoBlinker(PHLED,phLoops,200,200,200,1000);
 }
 else if (phVal < 710) {
  ledOneBlinker(PHLED,phLoops,200,1000);
 }
 else {
  digitalWrite(PHLED, HIGH);

 }
}


void recvWithEndMarker() {
 static byte ndx = 0;
 char endMarker = '\n';
 char rc;

 while (Serial.available() > 0 && newData == false) {
 rc = Serial.read();

 if (rc != endMarker) {
  receivedChars[ndx] = rc;
  ndx++;
  if (ndx >= numChars) {
    ndx = numChars - 1;
  }
 }
 else {
  receivedChars[ndx] = '\0'; // terminate the string
  ndx = 0;
  newData = true;
 }
 }
}


void showNewData() {
 if (newData == true) {
 Serial.print("New pH value from serial >>  ");
 Serial.println(receivedChars);
 
 ph = (atof(receivedChars));

 newData = false;  
 }
}

void ledOneBlinker (int pin, byte pass, int on1, int off1) {
  if (pass == 0) {
    digitalWrite(pin, HIGH);
    previousMillis = millis();
    phLoops++;
  }
  if (millis() - previousMillis >= on1 && pass == 1) { //ready to turn off
    digitalWrite(pin, LOW);
    previousMillis = millis();
    phLoops++;
  }
  if (millis() - previousMillis >= off1 && pass == 2) { //waiting for next round
    phLoops = 0;
  }
}


void ledTwoBlinker (int pin, byte pass, int on1, int off1, int on2, int off2) {
  if (pass == 0) {
    digitalWrite(pin, HIGH);
    previousMillis = millis();
    phLoops++;
  }
  if (millis() - previousMillis >= on1 && pass == 1) { //ready to turn off
    digitalWrite(pin, LOW);
    previousMillis = millis();
    phLoops++;
  }
  if (millis() - previousMillis >= off1 && pass == 2) { //ready to turn on for 2nd blink
    digitalWrite(pin, HIGH);
    previousMillis = millis();
    phLoops++;
  }
  if (millis() - previousMillis >= on2 && pass == 3) { //ready to turn off 2nd blink
    digitalWrite(pin, LOW);
    previousMillis = millis();
    phLoops++;
  }
  if (millis() - previousMillis >= off2 && pass == 4) { //waiting for next round
    phLoops = 0;
  }
}

recvWithEndMarker is never called. It is dead code. The optimizer eliminates it from the image.

Ditto for showNewData.

Ditto for ledThreeBlinker.

Neither phVal nor ph ever change.

Ya I might have missed some code... I'm trying to slim it down to fit here... added the omissions.

In the running sketch here's what is sent to serial for a value of 6.8. This is after it working at 0, then 6.8, solid at 7.4, 2x blink at 8.0, then back to 6.8. The int has the value but the if/else isn't firing.

21:23:07 7/12/2017
pH FLOAT..... 6.80000019
pH INT....... 680
Uptime....... 00:05:10

What you really need to do is sprinkle a few serial prints around there to show you whats going on, what gets called when, and what the value of things like phLoops is at those particular points.

What happens if the ledTwoBlinker is running and phLoops get bumped up to 3 and then the pH changes and you start calling ledOneBlinker without ledTwoBlinker getting to the last step and setting it back to 0. phLoops would still be 3 and you'd get nothing out of the led.

Those two functions shouldn't be trying to share a state variable. Take the parameter pass out and give each of those functions a static local variable to track state separately for each function.

Delta_G:
What you really need to do is sprinkle a few serial prints around there to show you whats going on, what gets called when, and what the value of things like phLoops is at those particular points.

What happens if the ledTwoBlinker is running and phLoops get bumped up to 3 and then the pH changes and you start calling ledOneBlinker without ledTwoBlinker getting to the last step and setting it back to 0. phLoops would still be 3 and you’d get nothing out of the led.

Those two functions shouldn’t be trying to share a state variable. Take the parameter pass out and give each of those functions a static local variable to track state separately for each function.

That looks like the problem. I was able to band aid it for now and will tear it apart tomorrow when my eyes are fresh. For now, I added a var to see which pH tier I was in (1, 2 or 3 for low, med or hi), and if I ejected from one of the ledBlinkers early like you mentioned, I check for it and reset phLoops to 0 before calling the new function. Thanks for the help!

Band aid:

    if (phVal > 759) {
      if (phInLoop != 3) phLoops = 0;
      phInLoop = 3;
      ledTwoBlinker(PHLED,phLoops,200,200,200,1000);
    }
    else if (phVal < 710) {
      if (phInLoop != 1) phLoops = 0;
      phInLoop = 1;
      ledOneBlinker(PHLED,phLoops,200,1000);
    }
    else {
      if (phInLoop != 1) phLoops = 0;
      phInLoop = 2;
      digitalWrite(PHLED, HIGH);
    }

[quote author=Coding Badly link=msg=3337002 date=1499919519] recvWithEndMarker is never called. It is dead code. The optimizer eliminates it from the image. [/quote] Why do you say that? It is the first line in loop()

And the OP's comments seem to imply that it is working.

...R

Robin2: Why do you say that?

@dotJason modified the original post.

@dotJason read section 14... http://forum.arduino.cc/index.php/topic,148850.0.html

[quote author=Coding Badly link=msg=3337109 date=1499929978] @dotJason modified the original post. [/quote] Thanks. That's always a real PITA

...R

Jeez, sorry guys, thought I was doing the right thing. Having now been shamed for violating section 14.7 (and probably 14.5), I hope to see more references to section 10 violations. They are rampant but strangely never pointed out.

dotJason: They are rampant but strangely never pointed out.

We thought we'd leave something for you to point out :)

...R

Robin2:
We thought we’d leave something for you to point out :slight_smile:

…R

lol. well done