Storing MQTT topics in an array not working

Hello,

I'm using MQTT to make a remote for my home automation and at the beginning I had:

TOPIC1= /remote/control/parking
TOPIC2= /remote/control/visiteurs
TOPIC3= /remote/control/portail

which were working fine, I tried to replace with an array:
char* command_topics[5];

generated with:

//Channels
const char* Channels[] = {"parking","visiteurs","portail","immeuble","lumiere"}; //name channels
const int numChannels = sizeof(Channels) / sizeof(Channels[0]); //amount of Channels

//MQTT
const char* COMMAND_TOPIC_PATH = "remote/command/"; //Commands Topic Path
const char* STATE_TOPIC_PATH = "remote/state/"; //State of Command Topic Path

using the following code called during the setup and in case of a reconnection to the MQTT broker:

void mqtt_topics_initialization() {
  for (int i = 0; i < numChannels; i++) {
    //Command Topic Generation & subscribe to MQTT
    char buf[50];
    strcpy(buf, COMMAND_TOPIC_PATH);
    strcat(buf, Channels[i]);
    if (!client.subscribe(buf)) {
      Serial.print("Subscribe to ");
      Serial.print(buf);
      Serial.println(" failed");
      client.disconnect();
      return;
    }
    Serial.print("Subscribed to ");
    Serial.println(buf);
    command_topics[i] = buf;
    Serial.println(command_topics[i]); //check command topic stored

    //State Topic Generation & subscribe to MQTT
    char buf2[50];
    strcpy(buf2, STATE_TOPIC_PATH);
    strcat(buf2, Channels[i]);
    
    if (!client.subscribe(buf2)) {
      Serial.print("Subscribe to ");
      Serial.print(buf2);
      Serial.println(" failed");
      client.disconnect();
      return;
    }
    Serial.print("Subscribed to ");
    Serial.println(buf2);
    state_topics[i] = buf2;
    Serial.println(state_topics[i]); //check state topic stored
    client.publish(state_topics[i], "OFF");
  }
  //Debug check size of array
  numCommandTopics = sizeof(command_topics) / sizeof(command_topics[0]);
  numStateTopics = sizeof(state_topics) / sizeof(state_topics[0]);
  Serial.println(numCommandTopics); //Debug check size of array
  Serial.println(numStateTopics);
}

In the terminal it shows the right topics when it gets initialized and subscribed in the void_setup:

Attempting MQTT connection...
Subscribed to /remote/command/parking
remote/command/parking
Subscribed to /remote/state/parking
remote/state/parking

but it seems something is wrong as when it is in the loop it, printing the content of the array shows:

L-�?�'�?�
L-�?�'�?�
L-�?�'�?�
L-�?�'�?�
L-�?�'�?�

and it also makes a second problem, when it receives an MQTT message it is supposed to compare the topic of the message to the topics contained in the array
but all the values of the array gets replaced by the content of the message received. (if I send HELLO with MQTT the values of the array will show HELLO for all)

// Handle incoming messages from the broker
void callback(char* topic, byte* payload, unsigned int length) {
        for (int i = 0; i < numCommandTopics; i++) {
      Serial.println(command_topics[i]);
      }
  String response;
  for (int i = 0; i < length; i++) {
    response += (char)payload[i];
  }
      Serial.println(topic);
      Serial.println(response);
      Serial.println("--test begin--");
      for (int i = 0; i < numCommandTopics; i++) {
      Serial.println(command_topics[i]);
      }
       Serial.println("--test end--");

  if (strcmp(topic, command_topics[0]) == 0) {
    Serial.print("Message arrived [");
    Serial.print(topic);
    Serial.print("] ");
    Serial.println(response);
    if (response == "ON")  // Turn the light on
    {
    mqtt_lora_channel_1();
    }
    else if (response == "OFF")  // Turn the light off
    {
      client.publish(state_topics[0], "OFF");
    }
  }

I guess this is not what it is supposed to do as there is nothing in the code to put the response in the command_topics and at some point with the code generating the topics URL it was causing reboots.

How could I solve this problem? Is there possible improvements also?
Thank you

Type of each topic is char* (pointer to char), so the type of the entire array should be char** (pointer to pointer to char).

Not if you want to store the char*. That array could store 5 c-strings pointers for example


Just an idea: The issue might be that the MQTT library expects the topics you subscribe to to be long lived.

You do this

for (int i = 0; i < numChannels; i++) {
    //Command Topic Generation & subscribe to MQTT
    char buf[50];
    strcpy(buf, COMMAND_TOPIC_PATH);
    strcat(buf, Channels[i]);
    if (!client.subscribe(buf)) {

It would be interesting to look in the library if they duplicate the string or just memorize the pointer. If it’s the latter then subscribing using your temporary buffer won’t cut it.

I tried the char** and it displayed an error:

Compilation error: no matching function for call to 'PubSubClient::publish(char**&, const char [4])'

Then I checked the library and all the references to the topic are:
const char* topic

thanks

Please post the complete code all at once rather than snippets. That will make it easier to drop into the IDE and test compile.

Here is a link to the project as I splitted it in multiple tabs:
https://file.io/WsCty1lsqx86
(new user I can't upload files here)

The folder should contain the libraries needed.
Thanks

To post images etc you need trust level 1, you can get there by:

  • Entering at least 5 topics
  • Reading at least 30 posts
  • Spend a total of 10 minutes reading posts

Users at trust level 1 can…

  • Use all core Discourse functions; all new user restrictions are removed
  • Send PMs
  • Upload images and attachments

But new users can post code using Code Tags from the beginning, right? That's what I requested, not an uploaded file or a link of unknown provenance.

sorry, it was a wrong advice, as @J-M-L said

Read the post #4 carefully. I think the problem is that you are using a temporary buffer.
Try to create an array just like you did in the first post, but fill it with full topic names, not just the last component

but fill it with full topic names, not just the last component

isn't it what i am doing ?
The buf before being stored in the array contains the full topic: remote/command/parking

or I'm missing something as I am expecting that it produces an array like:
char* command_topics[]= {remote/command/parking,remote/command/visiteurs}

Thanks

try something like this

typed here so don't know if that compiles but the idea is that you actually create c-strings which stay in memory for your commandTopics and stateTopics (that's why they are global (or you can make them static in the function), so they will stick around)

//Channels
const char* channels[] = {"parking", "visiteurs", "portail", "immeuble", "lumiere"}; //name channels
const size_t numChannels = sizeof channels / sizeof *channels; // number of channels

//MQTT
const char* COMMAND_TOPIC_PATH = "remote/command/"; //Commands Topic Path
const char* STATE_TOPIC_PATH = "remote/state/"; //State of Command Topic Path

const size_t maxTopicSize = 35; // make sure all the topic fits
char commandTopics[numChannels][maxTopicSize + 1];
char stateTopics[numChannels][maxTopicSize + 1];


bool mqtt_topics_initialization() {

  for (int i = 0; i < numChannels; i++) {
    //Command Topic Generation & subscribe to MQTT
    strncpy(commandTopics[i], COMMAND_TOPIC_PATH, maxTopicSize);
    strncat(commandTopics[i], channels[i], maxTopicSize);
    if (!client.subscribe(commandTopics[i])) {
      Serial.print("Subscribing to ");
      Serial.print(commandTopics[i]);
      Serial.println(" failed");
      client.disconnect();
      return false;
    }
    Serial.print("Subscribed to ");
    Serial.println(commandTopics[i]);

    //State Topic Generation & subscribe to MQTT
    strncpy(stateTopics[i], STATE_TOPIC_PATH, maxTopicSize);
    strncat(stateTopics[i], channels[i], maxTopicSize);

    if (!client.subscribe(stateTopics[i])) {
      Serial.print("Subscribing to ");
      Serial.print(stateTopics[i]);
      Serial.println(" failed");
      client.disconnect();
      return false;
    }
    Serial.print("Subscribed to ");
    Serial.println(stateTopics[i]);

    client.publish(state_topics[i], "OFF");
  }
  return true;
}

You tried, but done it wrong.
You created an array to store a full topic names in it. But than you stored the same pointer to the same buf in every array element. Additionally, your buf was a local, so it was destroyed after returning from the function.

I tried the code provided by J-M-L but it makes the system reboot with the following error:

IP address: 192.168.0.176
LoRa Initialization
Attempting MQTT connection...
Subscribed to remote/command/parking
Subscribed to remote/state/parking
Guru Meditation Error: Core 1 panic'ed (LoadProhibited). Exception was unhandled.

edit: I found the problem, the code by J-M-L was using commandTopics and I had some of my code written command_topics left everywhere.

good - only the two arrays

char commandTopics[numChannels][maxTopicSize + 1];
char stateTopics[numChannels][maxTopicSize + 1];

are needed indeed

does it work now?

if it does it means the library does not duplicate the content of the topics and only keeps the pointers around

Yes it works perfectly now =)

Thank you.

Looking at the source code for PubSubClient, that appears not to be the case. The 'subscribe()' function copies the char array's contents into a buffer, adds the MQTT header and other info, then sends it off to the broker. Neither the pointer nor the topics array's characters are saved in the library. The internal buffer is reused for the next subscription. I imagine the library doesn't need any of the topic names after they're given to the broker.

Don't know what OP's original problem was, but it's not the need for the application code to save the topic names for the library.

thanks for looking at this. Interesting to know. indeed as you say it makes sense that the information is server side as that's where the notifications are coming from

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