Callback function and MQTT Arduino client

Hi all

I'm using the Arduino MQTT client from knolleary and I'm just trying to get my head around getting multiple variables out of the callback function depending on the topic in question.

void setup() {
client.subscribe("audio/home/bedroom/title");
client.subscribe("audio/home/bedroom/volume");
 [...]
}

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

    payload[length] = '\0';

    msg = (char*)payload;
    writetitle = msg;
    titlelength = length;
    scrollcounter = 100;
    x_pos = 3;

}

// update title on display using writetitle

How would I also get volume out?

I've tried to use an if statement that checks the topic before writing out to the correct variable in the callback, but that didn't really work (it just didn't execute the code within the if, for some reason).

Can anyone help?

How would I also get volume out?

Out of what?

I've tried to use an if statement that checks the topic before writing out to the correct variable in the callback, but that didn't really work (it just didn't execute the code within the if, for some reason).

Given that you have not shown what the input to that function looks like, or what your code looks/looked like, I can't even feign surprise.

@pauls

Out of what?

Pretty obvious given the snippet of code I posted. I want to subscribe to more than one MQTT topic, my code shows I'm trying to subscribe to two - in my case "track" and "volume". As you can see from the code. Obviously whenever the callback function is called - regardless of what topic - it writes out to a temp variable.

Given that you have not shown what the input to that function looks like

Huh? The "input" to the function is part of the MQTT library. But here it is, if it really makes things clearer for you:

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

I can't even feign surprise

/ignores rudeness

Huh? The "input" to the function is part of the MQTT library. But here it is, if it really makes things clearer for you:

No. I mean what is in payload when the callback is called? What is in topic?

The same callback is going to be called for both topics, but the payload should be different. The value in topic will tell you why the callback was called, so that you can make semi-intelligent choices as to where to persist the data in payload.

@pauls Okay I see what you mean.

So the topic is the thing that the function client.subscribe "listens" to. (I say "listens" because they are MQTT messages which are published over the network.)

Thus the contents of the variable topic are, like this: "audio/home/bedroom/title" or "audio/home/bedroom/volume" or "lights/home/bedroom/uplighter", which are char* type.

So the topic is the thing that the function client.subscribe "listens" to.

In that context, yes. Then, when one of those topics happens, the function callback() is called.

You look at what's in topic, in the callback function, to find out why the function was called, in case that has any bearing on how you parse/where you store the data that is in payload.

@pauls Thanks for that comment, since I posted the original thread I've changed my subscribe function to listen one level back (i.e. "audio/home/bedroom/#" instead of "audio/home/bedroom/title"), and changed the callback function to contain two if statements like this:

void callback(char* topic, byte* payload, unsigned int length) {
    payload[length] = '\0';
    String strTopic = String((char*)topic);

    if (strTopic == "audio/home/bedroom/title") {
      msg = (char*)payload;
      writetitle = msg;
      Serial.println("Found a title update");
    }

    if (strTopic == "audio/home/bedroom/vol") {
      msg = (char*)payload;
      writevol = msg;
      Serial.println("Found a volume update");
    }    
}

Problem is, if I do a Serial.println(writetitle) in the loop, it shows payloads from ALL topics under audio/home/bedroom/#

My oled display gets updated with the track ... until I turn the volume up, then it shows "e0"... "e4"... "e8" etc. (the hex representation of the volume string, increases as volume goes up)

Obviously the desired effect is that my writetitle variable only has track titles, and then my writevol variable only has volume updates!

Am I doing it all wrong?

Am I doing it all wrong?

All might be a bit extreme, but you are doing at least two things wrong.

    String strTopic = String((char*)topic);

topic is a char *, so the cast is unnecessary. Creating two String instances, invoking the copy constructor, and then destroying one of the instances is a waste of resources. Using the String class at all is a waste of resources.

if(strcmp(topic, "audio/home/bedroom/title") == 0)
{
   // found a title
}

if(strcmp(topic, "audio/home/bedroom/vol") == 0)
{
   // found a volume
}

The second thing you are doing wrong is expecting help with all of your code while only posting a snippet or two.

@PaulS Here's the whole sketch rather than just snippets. Please don't laugh at my code, coding is not what I do best but I'm trying to learn!

You'll see the callback right at the end of the sketch (I've amended it to account for your previous comments, thank you).

It's still not working, I'm obviously doing something silly but I can't spot it.

/*
Sainsmart ethernet shield and Uno
*/ 

#include <SPI.h>
#include "U8glib.h"

U8GLIB_SH1106_128X64 u8g(U8G_I2C_OPT_NO_ACK);

#include <Ethernet.h>
#include <Wire.h>
#include <PubSubClient.h>
#include <Encoder.h>
#include <ClickButton.h>

const int buttonPin1 = 14;
ClickButton button1(buttonPin1, LOW, CLICKBTN_PULLUP);
byte mac[]    = { 
  0xDE, 0xED, 0xBA, 0xFE, 0xFE, 0xED };
byte server[] = { 
  192, 168, 0, 2 };
byte ip[]     = { 
  192, 168, 0, 100 };
Encoder myEnc(2, 3);
EthernetClient ethClient;
PubSubClient client(server, 1883, callback, ethClient);

char* msg = "";

char* writetitle = "INITIALISING ...  ";
char* writevol = "?";
int titlelength = 20;
int scrollcounter = 100;

void setup()
{
  // Setup button params
  button1.debounceTime   = 20;   // Debounce timer in ms
  button1.multiclickTime = 200;  // Time limit for multi clicks
  button1.longClickTime  = 250; // time until "held-down clicks" register

  Ethernet.begin(mac, ip);
  delay(720);

  // MQTT Startup
  client.connect("arduinoClient");
  delay(50);
  client.publish("audio/home/bedroom/arduinovector", "trackplease");
  delay(50);
  client.subscribe("audio/home/bedroom/#");
  delay(50);


  Serial.begin(9600); 

  // Some display params
  if ( u8g.getMode() == U8G_MODE_R3G3B2 ) {
    u8g.setColorIndex(255);     // white
  }
  else if ( u8g.getMode() == U8G_MODE_GRAY2BIT ) {
    u8g.setColorIndex(3);         // max intensity
  }
  else if ( u8g.getMode() == U8G_MODE_BW ) {
    u8g.setColorIndex(1);         // pixel on
  }
  else if ( u8g.getMode() == U8G_MODE_HICOLOR ) {
    u8g.setHiColorByRGB(255,255,255);
  }
}


int x_pos = 3;  // global variable
int oldPosition = 0;
int trackupdatecycle = 50;


void draw(void) {

  // Draw 3 lines of text on the display. Top 2 are the important ones with volume and track title 
  u8g.setFont(u8g_font_6x10);
  //  u8g.drawStr( 2, 10, "Vol __|__________ 18%");
  u8g.drawStr( 2, 10, writevol);
  //  u8g.setFont(u8g_font_helvB12);
  u8g.drawStr( x_pos, 39, writetitle);
  //  u8g.setFont(u8g_font_6x12);
  u8g.drawStr( 3, 62, "Jazz     R4     Sleep");

}

void loop()
{
  u8g.firstPage();  
  do {
    draw();

    // update track every 1000 cycles
    trackupdatecycle--;

    if (trackupdatecycle == 0) {
      // Request track over MQTT
      client.publish("audio/home/bedroom/arduinovector", "trackplease");  
      trackupdatecycle = 1500;
    }

    // Send volume command
    int newPosition = myEnc.read();
    if (newPosition > oldPosition) {
      oldPosition = newPosition;
      Serial.println("Volume UP");
      client.publish("audio/home/bedroom/arduinovector", "up");
    }
    else if (newPosition < oldPosition) {
      oldPosition = newPosition;
      Serial.println("Volume DOWN");
      client.publish("audio/home/bedroom/arduinovector", "down");
    }

    // Send playpause next prev command
    button1.Update();
    if (button1.clicks != 0) {
      if (button1.clicks == -1) { 
        client.publish("audio/home/bedroom/arduinovector", "playpause"); 
        Serial.println("PlayPause");
      }
      if (button1.clicks == -2) { 
        client.publish("audio/home/bedroom/arduinovector", "next"); 
        Serial.println("Next");
      }
      if (button1.clicks == -3) { 
        client.publish("audio/home/bedroom/arduinovector", "prev"); 
        Serial.println("Prev");
      }
    }

[b]    // Debug my values - this is not showing what I want it to show!
[/b]
    Serial.print(writetitle);
    Serial.print("    ");
    Serial.println(writevol);
 
   // Scroll title if required. Scrollpause used to pause it scrolling when left character is aligned with left of display
    if (titlelength > 14) { // title too long for the page?

      if (scrollcounter < 0) { // scroll pause countdown finished?
        x_pos--; // scroll  
        scrollcounter = 0;
      }
      else{
        scrollcounter--;
      }
    }


    if (x_pos < -256) {
      scrollcounter = 100;
      x_pos = 0;  
    }

    //  delay(5);
    client.loop();
  } 
  while( u8g.nextPage() );

}


void callback(char* topic, byte* payload, unsigned int length) {
  payload[length] = '\0';
  //  String strPayload = String((char*)payload);
  String strTopic = String((char*)topic);


  if(strcmp(topic, "audio/home/bedroom/title") == 0)
  {
    // found a title
    msg = (char*)payload;
    writetitle = msg;
    titlelength = length; // not yet used this
    scrollcounter = 100; // this resets the scroll counter so it pauses before scrolling
    x_pos = 3; // set scroll position to left
    Serial.println("Found a title update");
  }

  if(strcmp(topic, "audio/home/bedroom/vol") == 0)
  {
    // found a volume
    msg = (char*)payload;
    writevol = msg;
    Serial.println("Found a volume command");
  }

}

You have FAR too many global variables that are not needed. Get rid of msg, for instance. You do NOT want to set a global variable to point to a location on the stack (which is what happens when you make msg point to payload. When the function ends, msg is now an invalid pointer.

  //  String strPayload = String((char*)payload);
  String strTopic = String((char*)topic);

Useless.

    writetitle = msg;

This won't work. You must copy the data from payload to writetitle. writetitle should be an array, not a pointer.

    msg = (char*)payload;
    writevol = msg;

Same problem here. writevol points to payload, which goes away when the function ends. Copy the data, into an array.

strcpy() or memcpy() bear investigation.

Edit: And this is why snippets are a bad idea.