Break/Stop loop with MQTT payload/message

Hi,

after almost two days i give up.
I want to stop a loop with MQTT but can't get it to work.
I tried endless possibilities. Here is one:

int test = 0;
void callback(char* topic, byte* payload, unsigned int length) {

  if (length == 9 && strncmp((char*)payload,"volume-up",9)==0) {
    for (int i = 0; i < 1000; i++) {
      Serial.println("up");
      Serial.println(test);
      delay(50);
      if (test == 1) { 
        break;
      }
    }
  }


  if (length == 11 && strncmp((char*)payload,"volume-stop",11)==0) {
    Serial.println("stop");
    Serial.println(test);
    test = 1;
  }

}

If i call "volume-up" and get my test prints and then call "volume-stop" nothing happens.
If i call "volume-stop" first and then again i see that "test" has a value of "1".
Also "volume-up" is then printed just once.

I also tried it in "void loop()" with:

while (test == 0) {
  Serial.println("its running");
  Serial.println(test);
  // delay(20);
  if (test == 1){
    break;
  }
}

But that also doesn't stop if i call "volume-stop".

I'm out off ideas :frowning:

MrGlasspoole:
I'm out off ideas :frowning:

Me too. But of course I don't have any ideas because I don't have your full code, or a code sample that will compile and show the problem.

I see nothing that can change test. What is the callback all about? How do you get to callback? What is MQTT? is test global?

@lar3ry, you don't know MQTT? Then for sure my code makes no sense to you.
MQTT = Message Queue Telemetry Transport: MQTT - Wikipedia

Callback is part of Arduino Client for MQTT · knolleary

So "volume-up" runs if my MQTT broker on my server sends: "/mytopic/mytopic/IR-SonyTV/volume-up"
And "test" gets changed if i call/send: "/mytopic/mytopic/IR-SonyTV/volume-stop"

And "test" gets changed if i call/send: "/mytopic/mytopic/IR-SonyTV/volume-stop"

Well, that's perfectly obvious. To you.

Callback is part of...

But it sure doesn't have to have the meaningless name "callback".

Well, that's perfectly obvious. To you.

It is cause its the payload.
The topic says MQTT so this is about the Arduino pubsubclient.

But it sure doesn't have to have the meaningless name "callback".

It is not meaningless if you know MQTT in conjunction with Arduino cause it's part of
the pubsubclient (Arduino client for MQTT).

And everything is working great. Switching house lights, TV programs, Doorbell ring through Asterisk over VoIP
to Softphones, 1-Wire sensor logging to MariaDB....
I only can't get this stupid loop stop to work.

I do it with voice commands.
The idea was if i say "TV volume up" that the Arduino sends the IR signal until i say "volume stop".
Volume up works but i can't stop it.

The server sends payload "volume-up" to the Arduino and runs it and it's working.
But if i send then the payload "volume-stop" it does not stop running "volume-up".
If i check my "int test" after sending "volume-stop" it's set to "1" but does not effect the running "volume-up" loop.

  if (length == 9 && strncmp((char*)payload,"volume-up",9)==0) {
    for (int i = 0; i < 1000; i++) {
      Serial.println("up");
      Serial.println(test);
      delay(50);
      if (test == 1) { 
        break;
      }
    }
  }

If the payload IS "volume-up", the for loop starts running. It does NOT end, and is not interrupted, until it completes. During that time, the value of test does not change.

Why do you think that it does/should?

Ok, another 4 hours. I don't get it.
I also tested it with

while(test == 1) { 
    Serial.println("up");
    if (test == 0) {
      goto outer;
    }
 }
outer:;

in the void loop.
What i'm missing or doing not how all the other examples i see are done:
http://forum.arduino.cc/index.php?topic=41706.msg302984#msg302984
First answer: How can I break out of two nested for loops in Objective-C? - Stack Overflow
http://letsmakerobots.com/node/37905

while(test == 1) { 
    Serial.println("up");
    if (test == 0) {
      goto outer;
    }
 }
outer:;

Once the while loop starts, test does NOT ever get a new value, so the %&#%#^% goto statement will never be called.

Why do you persist in thinking that test will get a new value when the while loop is running?

[quote author=MrGlasspoole link=topic=234632.msg1689015#msg1689015 date=1398040799]
I want to stop a loop with MQTT but can't get it to work.

You have not really given us much to go on.
You would be more likely to get meaningful help if you post your entire code or a minimal, compilable example that shows the probem

Here's a thought ot two. My comments in the code.

int test = 0;   // Where is this declared? (where in the code)
void callback(char* topic, byte* payload, unsigned int length) {

  // place Serial.print() statements here, to show length, payload, and test
 
  if (length == 9 && strncmp((char*)payload,"volume-up",9)==0) {
    for (int i = 0; i < 1000; i++) {
      Serial.println("up");
      Serial.println(test);
      delay(50);
      if (test == 1) {
        break;
      }
    }
  }

  if (length == 11 && strncmp((char*)payload,"volume-stop",11)==0) {
    Serial.println("stop");
    Serial.println(test);
    test = 1;
  }
}

If i call "volume-up" and get my test prints and then call "volume-stop" nothing happens.

Insufficient information. What test prints do you get? What happens when you call "volume-stop". "Nothing happens" is meaningless. Does it keep printing "up"? Does it stop printing "up"? If it does stop, is it because of the timeout (ie. "up" is printed 1000 times)? Does it print "stop"?

If i call "volume-stop" first and then again i see that "test" has a value of "1".
Also "volume-up" is then printed just once.

Of course. Once test has been set to 1, you will break out of the for loop after the first time though it.

Why are you testing for length in the two if statements? You are doing strncmp() with the length of the desired compare string. Granted, it's slightly faster to check for length, and not bother with the compare if it's not the right length. Are you worried about speed? If you already know the length of the payload, you don't need strncmp(), and if you don't, your "if length --" may be preventing you from getting into the second if.

Ok first full code:

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

/**************************************************************
*                                               Ethernet Settings                                         *
**************************************************************/

// Ethernet Card Mac Address
byte mac[] = {0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF};
// IPv4 address
byte ip[] = {192, 168, 0, 99};
// Subnet mask
byte subnet[] = {255, 255, 255, 0};
// Default gateway
byte gateway[] = {192, 168, 0, 1};
// MQTT Broker
byte MqttBroker[] = { 192, 168, 0, 12 };
// Preferred DNS sever
// byte dns[] = {192, 168, 0, 1};

/**************************************************************
*                                                       Declare                                                    *
**************************************************************/

IRsend irsend;
int test = 0;

/**************************************************************
*                                                          MQTT                                                    *
**************************************************************/

// MQTT Callback
void callback(char* topic, byte* payload, unsigned int length) {
  
  Serial.print("New message from broker on topic: ");
  Serial.println(topic);

  Serial.print("Payload: ");
  Serial.write(payload, length);
  Serial.println();
  Serial.print("Test: ");
  Serial.println(test);

  // IR Remote Sony TV
  if (strcmp(topic,"skulltronics.net/hive/floor/optimus-prime/IR-SonyTV")==0) { 
    
    // Volume Up Nonstop
    if (length == 10 && strncmp((char*)payload,"volume-ups",10)==0) {
      for (int i = 0; i < 250; i++) {
        Serial.println("running");
        delay(50);
        if (test == 1) {
          break;
        }
      }
    }// End Volume Up Nonstop

    // Stop Volume Up Nonstop
    if (length == 11 && strncmp((char*)payload,"volume-stop",11)==0) {
      test = 1;
    } // End Stop Volume Up Nonstop

  } // End IR Remote Sony TV

} // End MQTT Callback

// Fire up PubSub client
EthernetClient ethClient;
PubSubClient client(MqttBroker, 1883, callback, ethClient);

/**************************************************************
*                                                       Setup                                                       *
**************************************************************/

void setup() {

  // Open serial communications and wait for port to open:
  Serial.begin(9600);
    while (!Serial) {
    ; // wait for serial port to connect. Needed for Leonardo only
  }

  Ethernet.begin(mac, ip, subnet, gateway);
  Serial.print(" IPv4 address: ");
  Serial.println(Ethernet.localIP());
   
  // Connect to Broker, give it Optimus Prime as the name
  if (client.connect("Optimus Prime")) {
    // Publish a message to the status topic
    client.publish("status","Optimus Prime is now online");
    // Listen for messages on the control topic
    client.subscribe("skulltronics.net/hive/floor/optimus-prime/#");
  }

}

void loop() {
  client.loop();
}

If i don't do the "strncmp length test" then my "for loop" is not running.
There is another way with a running for loop:

// MQTT Callback
void callback(char* topic, byte* payload, unsigned int length) {
  
  // ********** IR Remote Sony TV ********** //
  // Volume Up Nonstop
  if (strcmp(topic,"skulltronics.net/hive/floor/optimus-prime/IR-SonyTV/volume-ups")==0) { 
    if (payload[0] == '1') {
      for (int i = 0; i < 250; i++) {
        Serial.println("running");
        irsend.sendSony(0x490, 12);
        delay(50);
        if (test == 1) {
          break;
        }
      }
    }
  } // End Volume Up Nonstop

  // Stop Volume Up Nonstop
  if (strcmp(topic,"skulltronics.net/hive/floor/optimus-prime/IR-SonyTV/volume-stop")==0) {
    if (payload[0] == '1') {
      test = 1;
    }
  } // End Stop Volume Up Nonstop
  // ********** End IR Remote Sony TV ********** //

} // End MQTT Callback

The second example means i have to call every command with his own topic and send "1" as message.
From my point of view it makes more sense to call the remote (IR-SonyTV) and send as message
what to do (volume-up) instead of calling the remote, then the button and tell the button what to do.
It has to do with null- termination and there is more about it here (comment 50 to 60): Arduino Client for MQTT · knolleary
Also i have to do it like in the first example if i want to send the IR-Remote hex code over MQTT.

Ok, what this code now does is printing "running" if i send "volume-ups".
I the send "volume-stop" and "running" is still printed.
After the 250 "running" prints i get my:

New message from broker on topic: skulltronics.net/hive/floor/optimus-prime/IR-SonyTV
Payload: volume-stop
Test: 0

print.

If i send then "volume-ups" as expected "running" is only printed once and Test: 1
So setting "test" works.

I need to send every command minimum 3 times to the IR-LED. That is how Sony remotes work.
You press a button and the IR-LED blinks 3 times.
So for my non-stop volume up i normally need:
3 blinks 50ms pause 3 blinks 50ms pause...
until i say "volume-stop"

OK... I think I'm getting a bit of a handle on this code, but I also think you are making assumptions that are causing you some confusion. Correct me if any of MY assumptions are wrong.

First...

void loop() {
  client.loop();
}

Will be called over and over again, right? Which means that you will be getting something every time you press a button on your remote, and from the sound of it, more than one code being received is normal operation. So this calls the MQTT broker, and that returns something.

Questions:
It is important that you check for the presence of the string "skulltronics.net/hive/floor/optimus-prime/IR-SonyTV" ?
If not, don't even bother with it... just check for the presence of any payload that you are interested in.
If it is important, ie. if it is something you need to check to determine if there is a new payload, try something like this (assuming that the payload contents you are interested in have a maximum length of 20).

char message[ 21 ]; // assuming all message payloads will be 20 characters or fewer

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

  // replace your Serial prints here for debugging

  // IR Remote Sony TV
  if (strcmp(topic,"skulltronics.net/hive/floor/optimus-prime/IR-SonyTV")==0) {
    strncpy(message, (char *)payload, 20);
  }
  // Volume Up
  if (strcmp((char*)payload,"volume-ups")==0) {
    Serial.println("up");
    delay(50);
  }
  // Stop Volume Up
  if (strncmp(message,"volume-stop")==0) {
    // do whatever you need to stop the volume-up
    Serial.println("stop volume-up");
    message[0] = '\0';
  }
} // End MQTT Callback

Now, remembering that client.loop() is called VERY often, you will see that callback will be called often as well.

With this code, you should keep entering the if statement for volume-up until you receive a colume-stop. At that point, you NULL terminate message[], and stop entering either if block.

Now if you want to still set a maximum number of volume-up events, you could use a static variable in callback() and limit the number by incrementing it and making the value of it part of the volume-up if conditions.

What do you think?

Yes,

void loop() {
  client.loop();
}

is lets the MQTT client run. If not you lose connection to the broker (HiveMQ on my Debian server)
http://knolleary.net/arduino-client-for-mqtt/api/#loop

Yes i need to subscript to the whole topic in the callback. I tested also "*/IR-SonyTV" and "#/IR-SonyTV" because i thought
it would work because i already subscribe in the setup.

This part does not compile:

  if (strncmp(message,"volume-stop")==0) {
    // do whatever you need to stop the volume-up
    Serial.println("stop volume-up");
    message[0] = '\0';
  }

c:/programme (zip)/arduino/hardware/tools/avr/lib/gcc/../../avr/include/string.h: In function 'void callback(char*, byte*, unsigned int)':
c:/programme (zip)/arduino/hardware/tools/avr/lib/gcc/../../avr/include/string.h:135: error: too few arguments to function 'int strncmp(const char*, const char*, size_t)'
sketch_apr30a:58: error: at this point in file

If i take it out the "volume-ups" part works. "Up" is printed one time.

If i take it out the "volume-ups" part works. "Up" is printed one time.

What is the "it" that you take out?

Sorry, I was trying to preserve the essential parts of your code, while modifying it to fit more of how I think it will work.

Here's a corrected version...

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

  // replace your Serial prints here for debugging

  // IR Remote Sony TV
  if (strcmp(topic,"skulltronics.net/hive/floor/optimus-prime/IR-SonyTV")==0) {
    strncpy(message, (char *)payload, 20);
  }
  // Volume Up
  if (strcmp(message,"volume-ups")==0) {
    Serial.println("up");
    delay(50);
  }
  // Stop Volume Up
  if (strcmp(message,"volume-stop")==0) {
    // do whatever you need to stop the volume-up
    Serial.println("stop volume-up");
    message[0] = '\0';
  }
} // End MQTT Callback

What do you get if you put a Serial.println(message); before the first if in callback()?

You don't know It?


:smiley:

With it i meant the none compiling part.
And i was now looking for 10 minutes on the screen to find out what was wrong.
strncmp instead of strcmp

Ok the output is if i send my commands in that order:

  1. "volume-ups"
    Payload: volume-ups
    Message:
    Output: Run Code

  2. "volume-ups"
    Payload: volume-ups
    Message: volume-ups
    Output: Run Code

  3. "volume-stop"
    Payload: volume-stop
    Message: volume-ups
    Output: Stop Code

  4. "volume-stop"
    Payload: volume-stop
    Message:
    Output: Stop Code

  5. "volume-ups"
    Payload: volume-ups
    Message:

Payload: volume-ups
Message: volume-upsp

So after "volume-stop" the output (code that needs to be running) is empty on "volume-up".
By other words i can't turn the volume up after stopping it before.
Look at 6. after calling "volume-ups" a second time there is a "p" at the end of the message.
Where does that come from?

And before i forged: THANKS for helping.