Looking for advise on publishing state change to MQTT

I am hoping someone will be willing to point my error. This is only my second program and I am missing something.

Here is the setup:
I am using an Arduino uno, an Ethernet shield, 4 relays and 4 magnetic switches. Mqtt on an raspberry pi.

The plan:
Use Mqtt to send command to cycle the relay, I have this working.
the switch will tell Mqtt if the door is open or closed, partially working.

This is the portion of code in question

 state_1 = digitalRead (2);
  if (state_1 == HIGH){
    client.publish("outTopic","Garage door 1 open");
  }

else{
  client.publish("outTopic", "Garage door 1 Closed");
}

When I boot up the arduino, I get an open or closed based on the pin state. The problem I run into is that I don’t know where to place this portion of code. If I place it in void loop, it runs and runs, I know it should, but it seems to overload my broker. Also I only need it to report the when it changes not constantly.

I created what I think is called a function, void open() and placed the code there, but I did not get any result. What I would like to know is where to place this so it reports open or close when the pin high/low changes.

Here is the full code I am working with:

#include <SPI.h>
#include <Ethernet.h>
#include <PubSubClient.h>

// Update these with values for your network
byte mac[] = { 0x00, 0xAA, 0xBB, 0xCC, 0xDE, 0x02 };
IPAddress ip(192, 168, 1, 18);      //arduino IP, static
IPAddress server(192, 168, 1 ,7);   //MQTT IP



// Callback function header
void callback(char* topic, byte* payload, unsigned int length);

EthernetClient ethClient;
PubSubClient client(server, 1883, callback, ethClient);

// Callback function
void callback(char* topic, byte* payload, unsigned int length) {
    
  //Cycle relay and publish to the MQTT server a confirmation message
  if(payload[0] == '1'){
    digitalWrite(6, LOW);
    client.publish("outTopic", "Garage Door 1 Button Pressed");
    delay (1000);
    digitalWrite(6, HIGH);
    }
    
  if(payload[0] == '2'){
    digitalWrite(7, LOW);
    client.publish("outTopic", "Garage Door 2 Button Pressed");
    delay (1000);
    digitalWrite(7, HIGH);
    }

  if(payload[0] == '3'){
    digitalWrite(8, LOW);
    client.publish("outTopic", "Garage Door 3 Button Pressed");
    delay (1000);
    digitalWrite(8, HIGH);
    }

  if(payload[0] == '4'){
    digitalWrite(9, LOW);
    client.publish("outTopic", "Garage Door 4 Button Pressed");
    delay (1000);
    digitalWrite(9, HIGH);
    }
}

int state_1;
void setup()
{
  pinMode(6, OUTPUT);
  digitalWrite(6, HIGH);
  pinMode(7, OUTPUT);
  digitalWrite(7, HIGH);
  pinMode(8, OUTPUT);
  digitalWrite(8, HIGH);
  pinMode(9, OUTPUT);
  digitalWrite(9, HIGH);

  pinMode(2, INPUT_PULLUP);
  
  Ethernet.begin(mac, ip);
  if (client.connect("arduinoClient")) {
    client.publish("outTopic","Garage door opener conected");
    client.subscribe("inTopic");
  }
  state_1 = digitalRead (2);
  if (state_1 == HIGH){
    client.publish("outTopic","Garage door 1 open");
  }

else{
  client.publish("outTopic", "Garage door 1 Closed");
}

}


void loop()
{

  client.loop();
}

Thanks in advance

My RPi MQTT broker, periodically reads a dB table and sends the fields to a MQTT Broker. A ESP32 WROVER, subscribed to the MQTT broker, gets state topics. The state topics are compared to the last state, stored in a structure, and if any changes are found, the ESP32 reacts.

muellercandd:
I am hoping someone will be willing to point my error. This is only my second program and I am missing something.

Here is the setup:
I am using an Arduino uno, an Ethernet shield, 4 relays and 4 magnetic switches. Mqtt on an raspberry pi.

The plan:
Use Mqtt to send command to cycle the relay, I have this working.
the switch will tell Mqtt if the door is open or closed, partially working.

Save the state of the door as previousState, then when checking for a new state, do something like

if(newState==1 %% previousState==0){
  client.publish("outTopic","Garage door 1 open");

Then save your new state as previousState.

Thanks SteveMann, this helped a ton and now is workkng as I want.

OK, I ran into a new issue. I have one state change working as I thought but I need to add state changes for other pins. Here is what I have tried

int newState1 = digitalRead (2);
  int newState2 = digitalRead (3);
  
  
    if (newState1 != oldState)
  {
    if (newState1 == LOW)
    {
      Serial.println ("GD1 Closed");
      client.publish("GD1", "CLOSED");
    }
  
    else 
    {
      Serial.println ("GD1 Open");
      client.publish("GD1", "OPEN");
    }

    delay (10);
  }
  
  oldState = newState1;

  if (newState2 != oldState2)
  {
    if (newState2 == LOW)
    {
      Serial.println ("GD2 Closed");
      client.publish("GD2", "Closed");
    }

    else
    {
      Serial.println ("GD2 Open");
      client.publish ("GD2", "Open");
    }

    delay (10);
    
  }
  oldState2 = newState2;
  
}

The Ide does not like the oldState2 saying it was not declared.

Adding int oldState2 and #define oldState2 leads to an exit status 1 error. Leaving it as just oldState does not give me the error but continues to print and publish the current state. So I am clearly going about this the wrong way. Any suggestions?

Any suggestions?

Post your complete code.

#include <SPI.h>
#include <Ethernet.h>
#include <PubSubClient.h>
#define oldState2

// Update these with values for your network
byte mac[] = { 0x00, 0xAA, 0xBB, 0xCC, 0xDE, 0x02 };
IPAddress ip(192, 168, 1, 18);      //arduino IP, static
IPAddress server(192, 168, 1 ,7);   //MQTT IP



// Callback function header
void callback(char* topic, byte* payload, unsigned int length);

EthernetClient ethClient;
PubSubClient client(server, 1883, callback, ethClient);

// Callback function
void callback(char* topic, byte* payload, unsigned int length) {
    
  //Cycle relay and publish to the MQTT server a confirmation message
  if(payload[0] == '1'){
    digitalWrite(6, LOW);
    client.publish("outTopic", "Garage Door 1 Button Pressed");
    delay (1000);
    digitalWrite(6, HIGH);
    }
    
  if(payload[0] == '2'){
    digitalWrite(7, LOW);
    client.publish("outTopic", "Garage Door 2 Button Pressed");
    delay (1000);
    digitalWrite(7, HIGH);
    }

  if(payload[0] == '3'){
    digitalWrite(8, LOW);
    client.publish("outTopic", "Garage Door 3 Button Pressed");
    delay (1000);
    digitalWrite(8, HIGH);
    }

  if(payload[0] == '4'){
    digitalWrite(9, LOW);
    client.publish("outTopic", "Garage Door 4 Button Pressed");
    delay (1000);
    digitalWrite(9, HIGH);
    }
}

int oldState;

void setup()
{
  Serial.begin (9600);
  pinMode(6, OUTPUT);
  pinMode(7, OUTPUT);
  pinMode(8, OUTPUT);
  pinMode(9, OUTPUT);
  
  digitalWrite(6, HIGH);
  digitalWrite(7, HIGH);
  digitalWrite(8, HIGH);
  digitalWrite(9, HIGH);

  pinMode(2, INPUT_PULLUP);
  pinMode(3, INPUT_PULLUP);
  
  Ethernet.begin(mac, ip);
  if (client.connect("arduinoClient")) {
    client.publish("outTopic","Garage door opener conected");
    client.subscribe("cmnd/garageDoor/power");
  }
  
}



void loop()
{

  client.loop();

  int newState1 = digitalRead (2);
  int newState2 = digitalRead (3);
  
  
    if (newState1 != oldState)
  {
    if (newState1 == LOW)
    {
      Serial.println ("GD1 Closed");
      client.publish("GD1", "CLOSED");
    }
  
    else 
    {
      Serial.println ("GD1 Open");
      client.publish("GD1", "OPEN");
    }

    delay (10);
  }
  
  oldState = newState1;

  if (newState2 != oldState2)
  {
    if (newState2 == LOW)
    {
      Serial.println ("GD2 Closed");
      client.publish("GD2", "Closed");
    }

    else
    {
      Serial.println ("GD2 Open");
      client.publish ("GD2", "Open");
    }

    delay (10);
    
  }
  oldState2 = newState2;
  
}
#define oldState2

This use of #define makes no sense.
Syntax
#define constantName value

Parameters
constantName: the name of the macro to define.
value: the value to assign to the macro.

https://www.arduino.cc/reference/en/language/structure/further-syntax/define/

For consistency, I would change oldState to oldState1 and declare oldState1 and oldState2 where you declare newState1 and newState2. Initialize to HIGH to be consistent with the INPUT_PULLUP on pins 2 and 3. Since you have made the declarations local in loop() you need to use the static qualifier to preserve the value through each pass of loop() rather than being re-initialized.

int newState1 = digitalRead (2);
int newState2 = digitalRead (3);
static int oldState2 = HIGH;
static int oldState1 = HIGH;

Remove these two pieces of your code

int oldState;
#define oldState2
#include <SPI.h>
#include <Ethernet.h>
#include <PubSubClient.h>
//#define oldState2


// Update these with values for your network
byte mac[] = { 0x00, 0xAA, 0xBB, 0xCC, 0xDE, 0x02 };
IPAddress ip(192, 168, 1, 18);      //arduino IP, static
IPAddress server(192, 168, 1 , 7);  //MQTT IP



// Callback function header
void callback(char* topic, byte* payload, unsigned int length);

EthernetClient ethClient;
PubSubClient client(server, 1883, callback, ethClient);

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

  //Cycle relay and publish to the MQTT server a confirmation message
  if (payload[0] == '1') {
    digitalWrite(6, LOW);
    client.publish("outTopic", "Garage Door 1 Button Pressed");
    delay (1000);
    digitalWrite(6, HIGH);
  }

  if (payload[0] == '2') {
    digitalWrite(7, LOW);
    client.publish("outTopic", "Garage Door 2 Button Pressed");
    delay (1000);
    digitalWrite(7, HIGH);
  }

  if (payload[0] == '3') {
    digitalWrite(8, LOW);
    client.publish("outTopic", "Garage Door 3 Button Pressed");
    delay (1000);
    digitalWrite(8, HIGH);
  }

  if (payload[0] == '4') {
    digitalWrite(9, LOW);
    client.publish("outTopic", "Garage Door 4 Button Pressed");
    delay (1000);
    digitalWrite(9, HIGH);
  }
}

//int oldState;

void setup()
{
  Serial.begin (9600);
  pinMode(6, OUTPUT);
  pinMode(7, OUTPUT);
  pinMode(8, OUTPUT);
  pinMode(9, OUTPUT);

  digitalWrite(6, HIGH);
  digitalWrite(7, HIGH);
  digitalWrite(8, HIGH);
  digitalWrite(9, HIGH);

  pinMode(2, INPUT_PULLUP);
  pinMode(3, INPUT_PULLUP);

  Ethernet.begin(mac, ip);
  if (client.connect("arduinoClient")) {
    client.publish("outTopic", "Garage door opener conected");
    client.subscribe("cmnd/garageDoor/power");
  }

}



void loop()
{

  client.loop();

  int newState1 = digitalRead (2);
  int newState2 = digitalRead (3);
  static int oldState2;
  static int oldState1;


  if (newState1 != oldState1)
  {
    if (newState1 == LOW)
    {
      Serial.println ("GD1 Closed");
      client.publish("GD1", "CLOSED");
    }

    else
    {
      Serial.println ("GD1 Open");
      client.publish("GD1", "OPEN");
    }

    delay (10);
  }

  oldState1 = newState1;

  if (newState2 != oldState2)
  {
    if (newState2 == LOW)
    {
      Serial.println ("GD2 Closed");
      client.publish("GD2", "Closed");
    }

    else
    {
      Serial.println ("GD2 Open");
      client.publish ("GD2", "Open");
    }

    delay (10);

  }
  oldState2 = newState2;

}

Thanks Cattleldog, and sorry for not getting back sooner, to many irons in the fire. This worked perfectly and now that I see it, its pretty obvious.