Go Down

Topic: Bool condition issue (Read 367 times) previous topic - next topic

Paul3

Jan 17, 2018, 08:49 pm Last Edit: Jan 17, 2018, 08:56 pm by Paul3
I have a bool named "power" in the code below, it is updated from an MQTT broker, when it receives a "1" it is changed to true and otherwise false. The bool in the first statement seems to be working correctly if I serial print it, however, the second statement, which is nested in another statement and checks if power is true does not make and always does the else.

Is this because the condition is nested? Any suggestions on a better method to check if the state is on or off?

(P.S - Open to suggestions for improvements elsewhere as well)


Code: [Select]
void callback(char* topic, byte* payload, unsigned int length) {

  //convert topic to string to make it easier to work with
  String topicStr = topic;
  bool power;

  //Print out some debugging info
  Serial.println("Callback update.");
  Serial.print("Topic: ");
  Serial.println(topicStr);

  //turn the light on if the payload is '1'
  if (strcmp(topic,"/Lamp/Power")==0) {
      if(payload[0] == '1') {
         power = 1;
         setColor(r_Old,g_Old,b_Old,0);
      } else {
         power = 0;
         setColor(0,0,0,0);
      }
  }


  
  if (strcmp(topic,"/Lamp/Color")==0) {  
      String hex = String((char*)payload);
      hex.toUpperCase();
      char c[6];
      hex.toCharArray(c,7);
            
      Serial.println(hex);
          long r = convertToInt(c[0],c[1]); //red
          long g = convertToInt(c[2],c[3]); //green
          long b = convertToInt(c[4],c[5]); //blue

      if (power) {
        setColor(r, g, b, 0);
        r_Old = r;
        g_Old = g;
        b_Old = b;
      } else  {
        r_Old = r;
        g_Old = g;
        b_Old = b;
      }
    
    }
}

BulldogLowell

Code: [Select]
bool power;

 :) you need to initialize local variables to a known value

Paul3

changed to bool power = 0;

still seems to have same issue

gfvalvo

Since you didn't post your full code, what's there is out of context. But, just guessing, perhaps 'power' needs to be static?

Paul3

yeah, it's only required in this function. Just used to check if power is on so the color can be set, otherwise just storing it for when it is on.

aarg

  ... with a transistor and a large sum of money to spend ...
Please don't PM me with technical questions. Post them in the forum.

BulldogLowell

#6
Jan 17, 2018, 09:18 pm Last Edit: Jan 17, 2018, 09:19 pm by BulldogLowell
Code: [Select]
 //convert topic to string to make it easier to work with
  String topicStr = topic;


you don't need this... you already have pointers to char arrays (C strings)

just test that the message topic and payload are what you expect by printing them out:
Code: [Select]

  bool power = false;

  //Print out some debugging info);
  Serial.print(F("Topic: "));
  Serial.println(topic);
  Serial.print(F("Payload: "));
  Serial.println(payload);

Paul3

#7
Jan 17, 2018, 09:57 pm Last Edit: Jan 17, 2018, 10:02 pm by Paul3
Thanks for the tip!

So I noticed that I used power as a char* for another variable earlier so I changed the bool to isOn, however, still seems to be the same issue.

I checked payload and it is giving me the hex version of 1 instead (49) but it still matches 1, I also printed the value of isOn after updating it to 1 and it gives me a blank response being:

isOn value:

using this code to check:

Code: [Select]
//turn the light on if the payload is '1'
  if (strcmp(topic,"/Lamp/Power")==0) {
      Serial.print(F("Payload: "));
      Serial.println(payload[0]);
      if(payload[0] == '1') {
         isOn = 1;
         Serial.println("isON value: ");
         Serial.print(isOn);
         setColor(r_Old,g_Old,b_Old,0);
      } else {
         isOn = 0;
         setColor(0,0,0,0);
      }
  }



Here is my full code as well:
Code: [Select]
#include <PubSubClient.h>
#include <ESP8266WiFi.h>

//EDIT THESE LINES TO MATCH YOUR SETUP
#define MQTT_SERVER "{MQTTServerAddress}"
const char* ssid = "{SSID}";
const char* password = "{PASSWD}";

//LED on ESP8266 GPIO2
static const uint8_t redPin = D1;
static const uint8_t greenPin = D2;
static const uint8_t bluePin = D5;
static const uint8_t whitePin = D6;

char* lightTopic = "/Lamp/Brightness";
char* power = "/Lamp/Power";
char* color = "/Lamp/Color";


float r, g, b, w;
float r_Old, g_Old, b_Old, w_Old;

void callback(char* topic, byte* payload, unsigned int length);

WiFiClient wifiClient;
PubSubClient client(MQTT_SERVER, 1883, callback, wifiClient);

void setup() {
  //initialize the light as an output and set to LOW (off)
  pinMode(redPin, OUTPUT);
  pinMode(greenPin, OUTPUT);
  pinMode(bluePin, OUTPUT);

  //start the serial line for debugging
  Serial.begin(115200);
  delay(100);

  //start wifi subsystem
  WiFi.begin(ssid, password);
  //attempt to connect to the WIFI network and then connect to the MQTT server
  reconnect();

  //wait a bit before starting the main loop
      delay(2000);
}

void loop(){

  //reconnect if connection is lost

  if (!client.connected() && WiFi.status() == 3) {reconnect();}

  //maintain MQTT connection
  client.loop();

  //MUST delay to allow ESP8266 WIFI functions to run
  delay(10);
}


void callback(char* topic, byte* payload, unsigned int length) {

  bool isOn = 0;
  
  //Print out some debugging info);
  Serial.println("Callback update.");
  Serial.print(F("Topic: "));
  Serial.println(topic);
  Serial.print(F("Payload: "));
  int i;
  for (i = 0; i < 5; i = i + 1) {
    Serial.println(payload[i]);
}

  //turn the light on if the payload is '1'
  if (strcmp(topic,"/Lamp/Power")==0) {
      Serial.print(F("Payload: "));
      Serial.println(payload[0]);
      if(payload[0] == '1') {
         isOn = 1;
         Serial.println("isON value: ");
         Serial.print(isOn);
         setColor(r_Old,g_Old,b_Old,0);
      } else {
         isOn = 0;
         setColor(0,0,0,0);
      }
  }
  
  if (strcmp(topic,"/Lamp/Color")==0) {  
      String hex = String((char*)payload);
      hex.toUpperCase();
      char c[6];
      hex.toCharArray(c,7);
            
      Serial.println(hex);
          long r = convertToInt(c[0],c[1]); //red
          long g = convertToInt(c[2],c[3]); //green
          long b = convertToInt(c[4],c[5]); //blue

      if (isOn) {
        setColor(r, g, b, 0);
        r_Old = r;
        g_Old = g;
        b_Old = b;
      } else  {
        r_Old = r;
        g_Old = g;
        b_Old = b;
      }
 
    }
}

// Function for converting the hex digit to int
int convertToInt(char upper,char lower)
{
  int uVal = (int)upper;
  int lVal = (int)lower;
  uVal = uVal >64 ? uVal - 55 : uVal - 48;
  uVal = uVal << 4;
  lVal = lVal >64 ? lVal - 55 : lVal - 48;
  return uVal + lVal;
}

// Function to set the PWM signal on the color pins
void setColor(int r, int g, int b, int w) {
    float red = (float(r) / 255) * 1023;
    float green = (float(g) / 255) * 1023;
    float blue = (float(b) / 255) * 1023;
    float white = (float(w) / 255) * 1023;

    // write code to fade between colors HERE
      
    Serial.println(red);
    Serial.println(green);
    Serial.println(blue);
      
    analogWrite(redPin, red);
    analogWrite(greenPin, green);
    analogWrite(bluePin, blue);
    analogWrite(whitePin, white);
  }

void reconnect() {

  //attempt to connect to the wifi if connection is lost
  if(WiFi.status() != WL_CONNECTED){
    //debug printing
    Serial.print("Connecting to ");
    Serial.println(ssid);

    //loop while we wait for connection
    while (WiFi.status() != WL_CONNECTED) {
      delay(500);
      Serial.print(".");
    }

    //print out some more debug once connected
    Serial.println("");
    Serial.println("WiFi connected");  
    Serial.println("IP address: ");
    Serial.println(WiFi.localIP());
  }

  //make sure we are connected to WIFI before attemping to reconnect to MQTT
  if(WiFi.status() == WL_CONNECTED){
  // Loop until we're reconnected to the MQTT server
    while (!client.connected()) {
      Serial.print("Attempting MQTT connection...");

      // Generate client name based on MAC address and last 8 bits of microsecond counter
      String clientName;
      clientName += "esp8266-";
      uint8_t mac[6];
      WiFi.macAddress(mac);
      clientName += macToStr(mac);

      //if connected, subscribe to the topic(s) we want to be notified about
      if (client.connect((char*) clientName.c_str())) {
        Serial.print("\tMTQQ Connected");
        client.subscribe(lightTopic);
        client.subscribe(power);
        client.subscribe(color);
      }

      //otherwise print failed for debugging
      else{Serial.println("\tFailed."); abort();}
    }
  }
}

//generate unique name from MAC addr
String macToStr(const uint8_t* mac){

  String result;

  for (int i = 0; i < 6; ++i) {
    result += String(mac[i], 16);

    if (i < 5){
      result += ':';
    }
  }

  return result;
}





aarg

You go to the trouble of this:
Code: [Select]
char* power = "/Lamp/Power";

Then you toss it in the garbage can and use
Code: [Select]
  if (strcmp(topic,"/Lamp/Power")==0) {

instead of
Code: [Select]
  if (strcmp(topic,power)==0) {
  ... with a transistor and a large sum of money to spend ...
Please don't PM me with technical questions. Post them in the forum.

Paul3

thanks for the call out, changed color as well!

gfvalvo


Paul3

Yup, and that is exactly what solved the issue,

Thank you all!

PaulS

Quote
I have a bool named "power" in the code below
What does power = true mean? Nothing to me. powerOn = true suggests that the power is on, while false suggests that the power is on. Meaningful names really do help understanding code.
The art of getting good answers lies in asking good questions.

Go Up