If statement not working as intended

Hi there;

I'm having a problem with an if statement that should be comparing a string of text and only if the text matches it should reset a tick timer for a specific idletimer that the string applies to. The project is running on an ESP32-dev module. All other aspects of the sketch is working as intended. I'm using serial print to see which timer is being reset so I first include serial prints of where the if statement functions as intended, then one where it doesn't.

There are two types of message, an armed message and a fired message, this armed message is functioning as intended.

This is the serial print when a "fired message" is received. It should only reset one idletimer but resets both.

I appreciate that i'm new to coding and therefore my code might be bloated, but as I've said everything BUT the Fired! message filters are working as intended. Where I think the error is is in void MessageRecieved () section. I've included the full code, so thanks in advance for anyone who is able to help, or even taken the time to read this far.

#include <Arduino.h>
#include <WiFi.h>
#include <MQTT.h>

WiFiClient wifiClient;
MQTTClient client;

#define WIFI_NETWORK "redacted"
#define WIFI_PASSWORD "redacted"
#define WIFI_TIMEOUT_MS 20000

//Sets LED data table
int ledCount = 7;
struct led {
  String LedName;
  int    LedPinNumber;
  boolean LedPinState;
  String LedChannel;
  boolean TrapEmailStatus;
  String StatusMessage;
  String NodeName;
  String StatusMessageArmed;
} LED[] = {
  { "trap1A",              19, 0, "/WorkShop/Trap1Status",      0, "WS Trap 1 Fired!", "WorkShop",     "WS Trap 1 is armed.",   },
  { "trap2A",               5, 0, "/WorkShop/Trap2Status",      0, "WS Trap 2 Fired!", "WorkShop",     "WS Trap 2 is armed.",   },
  { "Trap3A",              18, 0, "/WorkShop/Trap3Status",      0, "WS Trap 3 Fired!", "WorkShop",     "WS Trap 3 is armed.",   },
  { "trap1B",              13, 0, "/SummerHouse/Trap1Status",   0, "SH Trap 1 Fired!", "SummerHouse",  "SH Trap 1 is armed.",   },
  { "trap2B",              12, 0, "/SummerHouse/Trap2Status",   0, "SH Trap 2 Fired!", "SummerHouse",  "SH Trap 2 is armed.",   },
  { "trap3B",              14, 0, "/SummerHouse/Trap3Status",   0, "SH Trap 3 Fired!", "SummerHouse",  "SH Trap 3 is armed.",   },
  { "trap4B",              16, 0, "/SummerHouse/Trap4Status",   0, "SH Trap 4 Fired!", "SummerHouse",  "SH Trap 4 is armed.",   },
};

//Data table MQTT subscriptions
int SubTopicsMax = 7;
struct SubTopics {
  String SubTopicName;
} SUBTOPIC[] = {
  {"/WorkShop/Trap1",     },
  {"/WorkShop/Trap2",     },
  {"/WorkShop/Trap3",     },
  {"/SummerHouse/Trap1",  },
  {"/SummerHouse/Trap2",  },
  {"/SummerHouse/Trap3",  },
  {"/SummerHouse/Trap4",  },
};

//Data table for IDLEcounters
int idleCounters = 2;
struct    idleCount {
  String  IdleName;
  int     MaxIdleCount;
  int     CurrentIdleCount;
  boolean GreenStatus;
  int     GreenPinNumber;
  int     RedPinNumber;
} IDLECOUNT[] = {
  { "idleCountA",   10000, 0, 1,  15, 2,  },
  { "idleCountB", 1800000, 0, 1,   4, 21, },
} ;

//Connects to Wifi
void connectToWifi() {
  Serial.print("Connecting to WiFi");
  WiFi.mode(WIFI_STA);
  WiFi.begin(WIFI_NETWORK, WIFI_PASSWORD);
  unsigned long startAttemptTime = millis();
  while (WiFi.status() != WL_CONNECTED && millis() - startAttemptTime < WIFI_TIMEOUT_MS) {
    Serial.print(".");
    delay(200);
  }
  if (WiFi.status() != WL_CONNECTED) {
    Serial.println(" Failed!");
  } else {
    Serial.println(" Connected");
    Serial.println(WiFi.localIP());
  }
}

//Connects to MQTT
void connectToMQTT() {
  Serial.println("Connecting to MQTT Server...");
  client.begin("192.168.0.9", wifiClient);
  while (!client.connect("ESPHue", "Public", "Public")) {
    Serial.print(".");
    delay(1000);
  }
  Serial.println("Connected to MQTT Broker!");
}

//Sets pins for LEDs from Data table
void setpinMode() {
  int i = 0;
  for (i; i < ledCount; i++) {
    pinMode(LED[i].LedPinNumber, OUTPUT);
  }
  i = 0;
  for (i; i < idleCounters; i++) {
    pinMode(IDLECOUNT[i].GreenPinNumber, OUTPUT);
    pinMode(IDLECOUNT[i].RedPinNumber, OUTPUT);
  }
}

//Subscribes to MQTT topics
void subscribeToTopics() {
  int i = 0;
  for (i; i < SubTopicsMax; i++) {
    client.subscribe(SUBTOPIC[i].SubTopicName);
    Serial.print("Subscribing to channel");
    Serial.println(SUBTOPIC[i].SubTopicName);
  }
}

//When message is recieves on MQTT channel, strips topic and payload
void messageRecieved(String &topic, String &payload) {
  // String response;
  Serial.print("MQTT Message Received on topic [:");
  Serial.print(topic);
  Serial.print("]");
  Serial.print(" MQTT Message: ");
  Serial.println(payload);
  Serial.print("Idletime A = ");
  Serial.print(IDLECOUNT[0].CurrentIdleCount);
  Serial.print(". Idletime B = ");
  Serial.println(IDLECOUNT[1].CurrentIdleCount);

  //Comapares payload to = trap fired messsage, if it hasn't sent an email message it will, but if it has it wont. Should reset NodeName timer
  int i = 0;
  for (i; i < ledCount; i++) {
    if (payload.equals(LED[i].StatusMessage)) {
      if (LED[i].TrapEmailStatus == 0) {
        client.publish(LED[i].LedChannel, "Mousetrap has fired, please check");
        LED[i].TrapEmailStatus = 1;
        LED[i].LedPinState = 1;
        Serial.print("Sending MQQT message to email ");
        Serial.println(LED[i].LedName);
      }
    }
    //Checks node of messsage, and should reset the idle timer of that node.
    if (LED[i].NodeName == "WorkShop"); {
      IDLECOUNT[0].CurrentIdleCount = 0;
      Serial.println("Trap has fired on WS, Resetting WS idle");
    }
    if (LED[i].NodeName == "SummerHouse"); {
      Serial.println("Trap has fired on SH, Resseting SH idle");
      IDLECOUNT[1].CurrentIdleCount = 0;
    }
  }

  //Compares payload to = trap armed message, if it matches, resets node timer.
  i = 0;
  for (i; i < ledCount; i++) {
    if (payload.equals(LED[i].StatusMessageArmed)) {
      LED[i].LedPinState = 0;
      if (LED[i].NodeName == "WorkShop") {
        IDLECOUNT[0].CurrentIdleCount = 0;
        Serial.println("Armed message recieved via WS, resetting WS idle time.");
      }
      if (LED[i].NodeName == "SummerHouse") {
        IDLECOUNT[1].CurrentIdleCount = 0;
        Serial.println("Armed message recieved via SH, reseeting SH idle time.");
      }
    }
  }
}

//Tick counts timers, if timers are overmax, then light the warning light
void idle() {
  int i = 0;
  for (i; i <= idleCounters; i++) {
    IDLECOUNT[i].CurrentIdleCount = IDLECOUNT[i].CurrentIdleCount + 1;
  }
  i = 0;
  for (i; i < idleCounters; i++) {
    if (IDLECOUNT[i].CurrentIdleCount >= IDLECOUNT[i].MaxIdleCount) {
      IDLECOUNT[i].GreenStatus = 0;
    }
    else {
      IDLECOUNT[i].GreenStatus = 1;;
    }
  }
}

//Set pin status from data tables
void writepinState() {
  int i = 0;
  for (i; i < ledCount; i++) {
    digitalWrite(LED[i].LedPinNumber, LED[i].LedPinState);
  }
  i = 0;
  for (i; i < idleCounters; i++) {
    if (IDLECOUNT[i].GreenStatus == 1) {
      digitalWrite(IDLECOUNT[i].GreenPinNumber, HIGH);
      digitalWrite(IDLECOUNT[i].RedPinNumber, LOW);
    } else {
      digitalWrite(IDLECOUNT[i].GreenPinNumber, LOW);
      digitalWrite(IDLECOUNT[i].RedPinNumber, HIGH);
    }
  }
}

void setup() {
  delay(5000);
  Serial.begin(9600);
  Serial.println("");
  Serial.println("Booting...");
  connectToWifi();
  connectToMQTT();
  setpinMode();
  subscribeToTopics();
  Serial.println("Waiting for MQTT message!");
}

void loop() {
  client.loop();
  delay(10);
  if (!client.connected()) {
    connectToMQTT();
  }
  idle();
  client.onMessage(messageRecieved);
  writepinState();
}

Thanks in advance for anyone who has taken the time to check this out. I appreciate any comments as I'm relatively new to coding.

Thanks

Josh

common mistake -- the ";" should be removed

anywhere else as well

Thanks for the suggestion,

I've removed the two instances of the extra ; in the block of code below, unfortunately this now gives a different unintended consequence.

    //Checks node of messsage, and should reset the idle timer of that node.
    if (LED[i].NodeName == "WorkShop") {
      IDLECOUNT[0].CurrentIdleCount = 0;
      Serial.println("Trap has fired on WS, Resetting WS idle");
    }
    if (LED[i].NodeName == "SummerHouse"){
      Serial.println("Trap has fired on SH, Resseting SH idle");
      IDLECOUNT[1].CurrentIdleCount = 0;
    }
  }

  //Compares payload to = trap armed message, if it matches, resets node timer.
  i = 0;
  for (i; i < ledCount; i++) {
    if (payload.equals(LED[i].StatusMessageArmed)) {
      LED[i].LedPinState = 0;
      if (LED[i].NodeName == "WorkShop") {
        IDLECOUNT[0].CurrentIdleCount = 0;
        Serial.println("Armed message recieved via WS, resetting WS idle time.");
      }
      if (LED[i].NodeName == "SummerHouse") {
        IDLECOUNT[1].CurrentIdleCount = 0;
        Serial.println("Armed message recieved via SH, reseeting SH idle time.");
      }
    }
  }
}

Serial output shows the unintended consequence.

11:27:16.370 -> MQTT Message Received on topic [:/SummerHouse/Trap4] MQTT Message: SH Trap 4 is armed.
11:27:16.480 -> Idletime A = 0. Idletime B = 0
11:27:16.480 -> Trap has fired on WS, Resetting WS idle
11:27:16.527 -> Trap has fired on WS, Resetting WS idle
11:27:16.570 -> Trap has fired on WS, Resetting WS idle
11:27:16.617 -> Trap has fired on SH, Resseting SH idle
11:27:16.670 -> Trap has fired on SH, Resseting SH idle
11:27:16.717 -> Trap has fired on SH, Resseting SH idle
11:27:16.771 -> Trap has fired on SH, Resseting SH idle
11:27:16.818 -> Armed message recieved via SH, reseeting SH idle time.

**Edited to add the full snippet of code for armed and fired messages.

what is the error? what was the input and corresponding output?

Hi gcjr, thanks again for your original suggestion.

I've put the serial output in the response that shows a message payload of "SH Trap 4 is armed" triggers all instances of of IDLE reset even thought the payload message does not match the == filter.

When a payload of "SH Trap 4 is armed", the filter should not apply a fired message on a different node (WS) and should not fire on other traps of the same node (SH).

In this instance, all traps listed in the datastructure are being triggered as 'fired' even though its a single trap sending a message of 'armed'.

The Armed message is correctly being handled by the armed IF, but not by the fired IF

i don't know what NodeName is?

i misunderstood

i thought NodeName is some input value. but now i see it's a predefined string and that the code does some action for each LED for a specified string ... in other words, it doesn't matter what the input is

it seems the actions under these if statements should only be done when "payload.equals" the "StatusMessage" and a specific LED[i]

don't understand why you have 2 for loops (for ledCount)

hope this is clear enough

Hi, perhaps it helps if i explain a bit more.

The system is degisned to read two mouse trap nodes, each node has multiple traps (2 on WS and 4 on SH), when a message is received on a specific node it should reset the idle timer of that node to 0. That way when a message isn't received within a specified time the node is considered broken or inactive and therefore requires attention because most likely the wifi of the node has broken.

This is the display of the two nodes, so when a trap fires the light goes red, if a node has been inactive a light goes off.

My problem is that i need to filter the mqtt message to know which node each message has been sent from, but my filtering isn't working properly.

I noticed an error since your replies with regards to the depth of { } so i'm just fixing that to see what affects that has. It has already stopped the accidental triggering of all the LED[i] messages.

So, after testing this is where I'm at.

Here is the message from the serial output. The unintended behaviour is the 2nd MQTT message, the first one (SH Trap 4 is armed) is being handled correctly. The Second (WS Trap 3 Fired!) is not filtered appropriately and is not resetting the WS idle time.

11:57:15.886 -> MQTT Message Received on topic [:/SummerHouse/Trap4] MQTT Message: SH Trap 4 is armed.
11:57:15.993 -> Idletime A = 30431. Idletime B = 0
11:57:16.040 -> Armed message recieved via SH, reseeting SH idle time.
11:58:32.149 -> MQTT Message Received on topic [:/WorkShop/Trap1] MQTT Message: WS Trap 3 Fired!
11:58:32.243 -> Idletime A = 38048. Idletime B = 7617

The code for this section is here. The rest of the code remains unchanged from the OP

void messageRecieved(String &topic, String &payload) {
  // String response;
  Serial.print("MQTT Message Received on topic [:");
  Serial.print(topic);
  Serial.print("]");
  Serial.print(" MQTT Message: ");
  Serial.println(payload);
  Serial.print("Idletime A = ");
  Serial.print(IDLECOUNT[0].CurrentIdleCount);
  Serial.print(". Idletime B = ");
  Serial.println(IDLECOUNT[1].CurrentIdleCount);

  //Comapares payload to = trap fired messsage, if it hasn't sent an email message it will, but if it has it wont. Should reset NodeName timer
  int i = 0;
  for (i; i < ledCount; i++) {
    if (payload.equals(LED[i].StatusMessage)) {
      if (LED[i].TrapEmailStatus == 0) {
        client.publish(LED[i].LedChannel, "Mousetrap has fired, please check");
        LED[i].TrapEmailStatus = 1;
        LED[i].LedPinState = 1;
        Serial.print("Sending MQQT message to email ");
        Serial.println(LED[i].LedName);
      }
      //Checks node of messsage, and should reset the idle timer of that node.
      if (LED[i].NodeName == "WorkShop") {
        IDLECOUNT[0].CurrentIdleCount = 0;
        Serial.println("Trap has fired on WS, Resetting WS idle");
      }
      if (LED[i].NodeName == "SummerHouse") {
        Serial.println("Trap has fired on SH, Resseting SH idle");
        IDLECOUNT[1].CurrentIdleCount = 0;
      }
    }
    //Compares payload to = trap armed message, if it matches, resets node timer.
    i = 0;
    for (i; i < ledCount; i++) {
      if (payload.equals(LED[i].StatusMessageArmed)) {
        LED[i].LedPinState = 0;
        if (LED[i].NodeName == "WorkShop") {
          IDLECOUNT[0].CurrentIdleCount = 0;
          Serial.println("Armed message recieved via WS, resetting WS idle time.");
        }
        if (LED[i].NodeName == "SummerHouse") {
          IDLECOUNT[1].CurrentIdleCount = 0;
          Serial.println("Armed message recieved via SH, reseeting SH idle time.");
        }
      }
    }
  }
}

Part of the issue is that as I'm noticing, or being informed of one mistake it is changing the behaviour of the filtering, so the unintended behaviour is changing slightly but it still remains that I don't seem to have the payload filtering working correctly.

Also a very sincere thank you for your help so far, I do really appreciate it.

there are multiple problems. it's confusing when expecting things to work after fixing one when other problems exist. also over complicated code

i don't understand 1) why you have those ifs testing NodeName and 2) the 2nd, nested for loop.

if payload.equals, why not just print LED[i].StatusMessageArmed?

Observation: when you post repaired snippets, implicitly expecting us to patch it in, you set us up to fail. Please, repost complete code, as that makes it error-proof to move the code into the IDE to examine.
Just a suggestion.

Thank you for that, i didn't want to repeatedly post the full code if it wasn't required, but will do so now. As you point out, it may be linked to an error elsewhere.

Thank you both for your ongoing help. GCJR, in response to your question, i'm quite new to coding and therefore may not be doing this the most appropriate way.

I have two types of message that i expect to recieve from two nodes
Type 1 - Fired message
Type 2 - Armed message

I would like the filtering to take the payload message and decide if it is a 'fired' message if it is, work out which node it is from, reset that node idle time, then do a few other functions for fired (and turn an LED pn, and send an mqtt output message if it's the first time, which will via node-red send an email).

I would like the filtering to take the payload message and decide if it is an 'armed' message, if it is, to work out what node it is from, reset just that node idle timer and ensure the LED is off.

As per camsysca suggestion, here is the current full sketch (i have fixed a few typos in serial.print functions)

#include <Arduino.h>
#include <WiFi.h>
#include <MQTT.h>

WiFiClient wifiClient;
MQTTClient client;

#define WIFI_NETWORK "VM7650217"
#define WIFI_PASSWORD "fmw7twzgKty9"
#define WIFI_TIMEOUT_MS 20000

//Sets LED data table
int ledCount = 7;
struct led {
  String LedName;
  int    LedPinNumber;
  boolean LedPinState;
  String LedChannel;
  boolean TrapEmailStatus;
  String StatusMessage;
  String NodeName;
  String StatusMessageArmed;
} LED[] = {
  { "trap1A",              19, 0, "/WorkShop/Trap1Status",      0, "WS Trap 1 Fired!", "WorkShop",     "WS Trap 1 is armed.",   },
  { "trap2A",               5, 0, "/WorkShop/Trap2Status",      0, "WS Trap 2 Fired!", "WorkShop",     "WS Trap 2 is armed.",   },
  { "Trap3A",              18, 0, "/WorkShop/Trap3Status",      0, "WS Trap 3 Fired!", "WorkShop",     "WS Trap 3 is armed.",   },
  { "trap1B",              13, 0, "/SummerHouse/Trap1Status",   0, "SH Trap 1 Fired!", "SummerHouse",  "SH Trap 1 is armed.",   },
  { "trap2B",              12, 0, "/SummerHouse/Trap2Status",   0, "SH Trap 2 Fired!", "SummerHouse",  "SH Trap 2 is armed.",   },
  { "trap3B",              14, 0, "/SummerHouse/Trap3Status",   0, "SH Trap 3 Fired!", "SummerHouse",  "SH Trap 3 is armed.",   },
  { "trap4B",              16, 0, "/SummerHouse/Trap4Status",   0, "SH Trap 4 Fired!", "SummerHouse",  "SH Trap 4 is armed.",   },
};

//Data table MQTT subscriptions
int SubTopicsMax = 7;
struct SubTopics {
  String SubTopicName;
} SUBTOPIC[] = {
  {"/WorkShop/Trap1",     },
  {"/WorkShop/Trap2",     },
  {"/WorkShop/Trap3",     },
  {"/SummerHouse/Trap1",  },
  {"/SummerHouse/Trap2",  },
  {"/SummerHouse/Trap3",  },
  {"/SummerHouse/Trap4",  },
};

//Data table for IDLEcounters
int idleCounters = 2;
struct    idleCount {
  String  IdleName;
  int     MaxIdleCount;
  int     CurrentIdleCount;
  boolean GreenStatus;
  int     GreenPinNumber;
  int     RedPinNumber;
} IDLECOUNT[] = {
  { "idleCountA",   10000, 0, 1,  15, 2,  },
  { "idleCountB", 1800000, 0, 1,   4, 21, },
} ;

//Connects to Wifi
void connectToWifi() {
  Serial.print("Connecting to WiFi");
  WiFi.mode(WIFI_STA);
  WiFi.begin(WIFI_NETWORK, WIFI_PASSWORD);
  unsigned long startAttemptTime = millis();
  while (WiFi.status() != WL_CONNECTED && millis() - startAttemptTime < WIFI_TIMEOUT_MS) {
    Serial.print(".");
    delay(200);
  }
  if (WiFi.status() != WL_CONNECTED) {
    Serial.println(" Failed!");
  } else {
    Serial.println(" Connected");
    Serial.println(WiFi.localIP());
  }
}

//Connects to MQTT
void connectToMQTT() {
  Serial.println("Connecting to MQTT Server...");
  client.begin("192.168.0.9", wifiClient);
  while (!client.connect("ESPHue", "Public", "Public")) {
    Serial.print(".");
    delay(1000);
  }
  Serial.println("Connected to MQTT Broker!");
}

//Sets pins for LEDs from Data table
void setpinMode() {
  int i = 0;
  for (i; i < ledCount; i++) {
    pinMode(LED[i].LedPinNumber, OUTPUT);
  }
  i = 0;
  for (i; i < idleCounters; i++) {
    pinMode(IDLECOUNT[i].GreenPinNumber, OUTPUT);
    pinMode(IDLECOUNT[i].RedPinNumber, OUTPUT);
  }
}

//Subscribes to MQTT topics
void subscribeToTopics() {
  int i = 0;
  for (i; i < SubTopicsMax; i++) {
    client.subscribe(SUBTOPIC[i].SubTopicName);
    Serial.print("Subscribing to channel");
    Serial.println(SUBTOPIC[i].SubTopicName);
  }
}

//When message is recieves on MQTT channel, strips topic and payload
void messageRecieved(String &topic, String &payload) {
  // String response;
  Serial.print("MQTT Message Received on topic [:");
  Serial.print(topic);
  Serial.print("]");
  Serial.print(" MQTT Message: ");
  Serial.println(payload);
  Serial.print("Idletime A = ");
  Serial.print(IDLECOUNT[0].CurrentIdleCount);
  Serial.print(". Idletime B = ");
  Serial.println(IDLECOUNT[1].CurrentIdleCount);

  //Comapares payload to = trap fired messsage, if it hasn't sent an email message it will, but if it has it wont. Should reset NodeName timer
  int i = 0;
  for (i; i < ledCount; i++) {
    if (payload.equals(LED[i].StatusMessage)) {
      if (LED[i].TrapEmailStatus == 0) {
        client.publish(LED[i].LedChannel, "Mousetrap has fired, please check");
        LED[i].TrapEmailStatus = 1;
        LED[i].LedPinState = 1;
        Serial.print("Sending MQQT message to email ");
        Serial.println(LED[i].LedName);
      }
      //  Checks node of messsage, and should reset the idle timer of that node.
      if (LED[i].NodeName == "WorkShop") {
        IDLECOUNT[0].CurrentIdleCount = 0;
        Serial.println("Trap has fired on WS, Resetting WS idle");
      }
      if (LED[i].NodeName == "SummerHouse") {
        Serial.println("Trap has fired on SH, Resetting SH idle");
        IDLECOUNT[1].CurrentIdleCount = 0;
      }
    }
    //Compares payload to = trap armed message, if it matches, resets node timer.
    i = 0;
    for (i; i < ledCount; i++) {
      if (payload.equals(LED[i].StatusMessageArmed)) {
        LED[i].LedPinState = 0;
        if (LED[i].NodeName == "WorkShop") {
          IDLECOUNT[0].CurrentIdleCount = 0;
          Serial.println("Armed message recieved via WS, resetting WS idle time.");
        }
        if (LED[i].NodeName == "SummerHouse") {
          IDLECOUNT[1].CurrentIdleCount = 0;
          Serial.println("Armed message recieved via SH, reset/++ting SH idle time.");
        }
      }
    }
  }
}

//Tick counts timers, if timers are overmax, then light the warning light
void idle() {
  int i = 0;
  for (i; i <= idleCounters; i++) {
    IDLECOUNT[i].CurrentIdleCount = IDLECOUNT[i].CurrentIdleCount + 1;
  }
  i = 0;
  for (i; i < idleCounters; i++) {
    if (IDLECOUNT[i].CurrentIdleCount >= IDLECOUNT[i].MaxIdleCount) {
      IDLECOUNT[i].GreenStatus = 0;
    }
    else {
      IDLECOUNT[i].GreenStatus = 1;;
    }
  }
}

//Set pin status from data tables
void writepinState() {
  int i = 0;
  for (i; i < ledCount; i++) {
    digitalWrite(LED[i].LedPinNumber, LED[i].LedPinState);
  }
  i = 0;
  for (i; i < idleCounters; i++) {
    if (IDLECOUNT[i].GreenStatus == 1) {
      digitalWrite(IDLECOUNT[i].GreenPinNumber, HIGH);
      digitalWrite(IDLECOUNT[i].RedPinNumber, LOW);
    } else {
      digitalWrite(IDLECOUNT[i].GreenPinNumber, LOW);
      digitalWrite(IDLECOUNT[i].RedPinNumber, HIGH);
    }
  }
}

void setup() {
  delay(5000);
  Serial.begin(9600);
  Serial.println("");
  Serial.println("Booting...");
  connectToWifi();
  connectToMQTT();
  setpinMode();
  subscribeToTopics();
  Serial.println("Waiting for MQTT message!");
}

void loop() {
  client.loop();
  delay(10);
  if (!client.connected()) {
    connectToMQTT();
  }
  idle();
  client.onMessage(messageRecieved);
  writepinState();
}

The current function of this is still now working, the serial output is showing that it is not filtering the Payload "SH Trap 1 Fired!" message.

The serial output below shows that the armed message has been handled correctly, which is then followed by a fired message, which isn't filtered properly and is not having the intended outcome.

12:48:32.943 -> MQTT Message Received on topic [:/SummerHouse/Trap1] MQTT Message: SH Trap 1 is armed.
12:48:33.037 -> Idletime A = 6493. Idletime B = 6493
12:48:33.084 -> Armed message recieved via SH, reset/++ting SH idle time.
12:48:46.461 -> MQTT Message Received on topic [:/SummerHouse/Trap1] MQTT Message: SH Trap 1 Fired!
12:48:46.554 -> Idletime A = 7839. Idletime B = 1346

Hi,

Thank you for the assistance camsysca and gcjr. Thank you for your time taken to help trouble shoot and ultimately help me fix the issue. Consider this post now solved.

For completeness I found another {} issue within the Fired for loop, which wasn't cycling through the traps under the fired section properly;

I found this error by adding serial output to see what the trap was being evaluated for filter, and it was only checking the 1st then stopping, whilst the armed loop was cycling fully.

I attach the full code should anyone have interest (which i doubt they will).

#include <Arduino.h>
#include <WiFi.h>
#include <MQTT.h>

WiFiClient wifiClient;
MQTTClient client;

#define WIFI_NETWORK "VM7650217"
#define WIFI_PASSWORD "fmw7twzgKty9"
#define WIFI_TIMEOUT_MS 20000

//Sets LED data table
int ledCount = 7;
struct led {
  String LedName;
  int    LedPinNumber;
  boolean LedPinState;
  String LedChannel;
  boolean TrapEmailStatus;
  String StatusMessage;
  String NodeName;
  String StatusMessageArmed;
} LED[] = {
  { "trap1A",              19, 0, "/WorkShop/Trap1Status",      0, "WS Trap 1 Fired!", "WorkShop",     "WS Trap 1 is armed.",   },
  { "trap2A",               5, 0, "/WorkShop/Trap2Status",      0, "WS Trap 2 Fired!", "WorkShop",     "WS Trap 2 is armed.",   },
  { "Trap3A",              18, 0, "/WorkShop/Trap3Status",      0, "WS Trap 3 Fired!", "WorkShop",     "WS Trap 3 is armed.",   },
  { "trap1B",              13, 0, "/SummerHouse/Trap1Status",   0, "SH Trap 1 Fired!", "SummerHouse",  "SH Trap 1 is armed.",   },
  { "trap2B",              12, 0, "/SummerHouse/Trap2Status",   0, "SH Trap 2 Fired!", "SummerHouse",  "SH Trap 2 is armed.",   },
  { "trap3B",              14, 0, "/SummerHouse/Trap3Status",   0, "SH Trap 3 Fired!", "SummerHouse",  "SH Trap 3 is armed.",   },
  { "trap4B",              16, 0, "/SummerHouse/Trap4Status",   0, "SH Trap 4 Fired!", "SummerHouse",  "SH Trap 4 is armed.",   },
};

//Data table MQTT subscriptions
int SubTopicsMax = 7;
struct SubTopics {
  String SubTopicName;
} SUBTOPIC[] = {
  {"/WorkShop/Trap1",     },
  {"/WorkShop/Trap2",     },
  {"/WorkShop/Trap3",     },
  {"/SummerHouse/Trap1",  },
  {"/SummerHouse/Trap2",  },
  {"/SummerHouse/Trap3",  },
  {"/SummerHouse/Trap4",  },
};

//Data table for IDLEcounters
int idleCounters = 2;
struct    idleCount {
  String  IdleName;
  int     MaxIdleCount;
  int     CurrentIdleCount;
  boolean GreenStatus;
  int     GreenPinNumber;
  int     RedPinNumber;
} IDLECOUNT[] = {
  { "idleCountA",   10000, 0, 1,  15, 2,  },
  { "idleCountB", 1800000, 0, 1,   4, 21, },
} ;

//Connects to Wifi
void connectToWifi() {
  Serial.print("Connecting to WiFi");
  WiFi.mode(WIFI_STA);
  WiFi.begin(WIFI_NETWORK, WIFI_PASSWORD);
  unsigned long startAttemptTime = millis();
  while (WiFi.status() != WL_CONNECTED && millis() - startAttemptTime < WIFI_TIMEOUT_MS) {
    Serial.print(".");
    delay(200);
  }
  if (WiFi.status() != WL_CONNECTED) {
    Serial.println(" Failed!");
  } else {
    Serial.println(" Connected");
    Serial.println(WiFi.localIP());
  }
}

//Connects to MQTT
void connectToMQTT() {
  Serial.println("Connecting to MQTT Server...");
  client.begin("192.168.0.9", wifiClient);
  while (!client.connect("ESPHue", "Public", "Public")) {
    Serial.print(".");
    delay(1000);
  }
  Serial.println("Connected to MQTT Broker!");
}

//Sets pins for LEDs from Data table
void setpinMode() {
  int i = 0;
  for (i; i < ledCount; i++) {
    pinMode(LED[i].LedPinNumber, OUTPUT);
  }
  i = 0;
  for (i; i < idleCounters; i++) {
    pinMode(IDLECOUNT[i].GreenPinNumber, OUTPUT);
    pinMode(IDLECOUNT[i].RedPinNumber, OUTPUT);
  }
}

//Subscribes to MQTT topics
void subscribeToTopics() {
  int i = 0;
  for (i; i < SubTopicsMax; i++) {
    client.subscribe(SUBTOPIC[i].SubTopicName);
    Serial.print("Subscribing to channel");
    Serial.println(SUBTOPIC[i].SubTopicName);
  }
}

//When message is recieves on MQTT channel, strips topic and payload
void messageRecieved(String &topic, String &payload) {
  // String response;
  Serial.print("MQTT Message Received on topic [:");
  Serial.print(topic);
  Serial.print("]");
  Serial.print(" MQTT Message: ");
  Serial.println(payload);
  Serial.print("Idletime A = ");
  Serial.print(IDLECOUNT[0].CurrentIdleCount);
  Serial.print(". Idletime B = ");
  Serial.println(IDLECOUNT[1].CurrentIdleCount);

  //Comapares payload to = trap fired messsage, if it hasn't sent an email message it will, but if it has it wont. Should reset NodeName timer
  int i = 0;
  for (i; i < ledCount; i++) {
    Serial.print("Checking fired Status: ");
    Serial.println(LED[i].LedName);
    if (payload.equals(LED[i].StatusMessage)) {
      if (LED[i].TrapEmailStatus == 0) {
        client.publish(LED[i].LedChannel, "Mousetrap has fired, please check");
        LED[i].TrapEmailStatus = 1;
        LED[i].LedPinState = 1;
        Serial.print("Sending MQQT message to email ");
        Serial.println(LED[i].LedName);
      }
      //  Checks node of messsage, and should reset the idle timer of that node.
      if (LED[i].NodeName == "WorkShop") {
        IDLECOUNT[0].CurrentIdleCount = 0;
        Serial.println("Trap has fired on WS, Resetting WS idle");
      }
      if (LED[i].NodeName == "SummerHouse") {
        Serial.println("Trap has fired on SH, Resetting SH idle");
        IDLECOUNT[1].CurrentIdleCount = 0;
      }
    }
  }

  //Compares payload to = trap armed message, if it matches, resets node timer.
  i = 0;
  for (i; i < ledCount; i++) {
    Serial.print("Checking Armed Status: ");
    Serial.println(LED[i].LedName);
    if (payload.equals(LED[i].StatusMessageArmed)) {
      LED[i].LedPinState = 0;
      if (LED[i].NodeName == "WorkShop") {
        IDLECOUNT[0].CurrentIdleCount = 0;
        Serial.println("Armed message recieved via WS, resetting WS idle time.");
      }
      if (LED[i].NodeName == "SummerHouse") {
        IDLECOUNT[1].CurrentIdleCount = 0;
        Serial.println("Armed message recieved via SH, resetting SH idle time.");
      }
    }
  }
}


//Tick counts timers, if timers are overmax, then light the warning light
void idle() {
  int i = 0;
  for (i; i <= idleCounters; i++) {
    IDLECOUNT[i].CurrentIdleCount = IDLECOUNT[i].CurrentIdleCount + 1;
  }
  i = 0;
  for (i; i < idleCounters; i++) {
    if (IDLECOUNT[i].CurrentIdleCount >= IDLECOUNT[i].MaxIdleCount) {
      IDLECOUNT[i].GreenStatus = 0;
    }
    else {
      IDLECOUNT[i].GreenStatus = 1;;
    }
  }
}

//Set pin status from data tables
void writepinState() {
  int i = 0;
  for (i; i < ledCount; i++) {
    digitalWrite(LED[i].LedPinNumber, LED[i].LedPinState);
  }
  i = 0;
  for (i; i < idleCounters; i++) {
    if (IDLECOUNT[i].GreenStatus == 1) {
      digitalWrite(IDLECOUNT[i].GreenPinNumber, HIGH);
      digitalWrite(IDLECOUNT[i].RedPinNumber, LOW);
    } else {
      digitalWrite(IDLECOUNT[i].GreenPinNumber, LOW);
      digitalWrite(IDLECOUNT[i].RedPinNumber, HIGH);
    }
  }
}

void setup() {
  delay(5000);
  Serial.begin(9600);
  Serial.println("");
  Serial.println("Booting...");
  connectToWifi();
  connectToMQTT();
  setpinMode();
  subscribeToTopics();
  Serial.println("Waiting for MQTT message!");
}

void loop() {
  client.loop();
  delay(10);
  if (!client.connected()) {
    connectToMQTT();
  }
  idle();
  client.onMessage(messageRecieved);
  writepinState();
}

i'm confused about the use of String in your code. i don't believe this is cause by your code, simply the use of String in Arduino which is problematic -- not the C++ implementation, i'm told more like the java implementation

it looks like a String won't be printed properly depending on the values/sizes of other tables including Strings in the code

i get the following depending on whether the last table entry in either table is #of'd out or not

Waiting for MQTT message!

Waiting for MQTT message!
/WorkShopgreg
Waiting for MQTT message!

Waiting for MQTT message!
/WorkShopgreg
Waiting for MQTT message!
# include <Arduino.h>

//Sets LED data table
int ledCount = 7;
struct led {
    String LedName;
    int    LedPinNumber;
    boolean LedPinState;
    String LedChannel;
    boolean TrapEmailStatus;
    String StatusMessage;
    String NodeName;
    String StatusMessageArmed;
}

LED[] = {
    { "trap1A",              19, 0, "/WorkShop/Trap1Status",      0, "WS Trap 1 Fired!", "WorkShop",     "WS Trap 1 is armed.",   },
    { "trap2A",               5, 0, "/WorkShop/Trap2Status",      0, "WS Trap 2 Fired!", "WorkShop",     "WS Trap 2 is armed.",   },
    { "Trap3A",              18, 0, "/WorkShop/Trap3Status",      0, "WS Trap 3 Fired!", "WorkShop",     "WS Trap 3 is armed.",   },
    { "trap1B",              13, 0, "/SummerHouse/Trap1Status",   0, "SH Trap 1 Fired!", "SummerHouse",  "SH Trap 1 is armed.",   },
    { "trap2B",              12, 0, "/SummerHouse/Trap2Status",   0, "SH Trap 2 Fired!", "SummerHouse",  "SH Trap 2 is armed.",   },
    { "trap3B",              14, 0, "/SummerHouse/Trap3Status",   0, "SH Trap 3 Fired!", "SummerHouse",  "SH Trap 3 is armed.",   },
#if 1
    { "trap4B",              16, 0, "/SummerHouse/Trap4Status",   0, "SH Trap 4 Fired!", "SummerHouse",  "SH Trap 4 is armed.",   },
#endif
};

//Data table MQTT subscriptions
int SubTopicsMax = 7;
struct SubTopics {
    String SubTopicName;
}

SUBTOPIC[] = {
    {"/WorkShop/Trap1",     },
    {"/WorkShop/Trap2",     },
    {"/WorkShop/Trap3",     },
    {"/SummerHouse/Trap1",  },
    {"/SummerHouse/Trap2",  },
    {"/SummerHouse/Trap3",  },
#if 0
    {"/SummerHouse/Trap4",  },
#endif
};

// -----------------------------------------------------------------------------
String myPayload = "/WorkShopgreg";

void setup ()
{
    Serial.begin (9600);
        Serial.println (myPayload);
    Serial.println ("Waiting for MQTT message!");
}

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

i created a thread concerning the problem printing a String i found String not printing depending on table size(?)

note the comments about memory usage String vs const char*.

as mentioned String is problematic and it may be worth avoiding

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