HASwitchPlate / openHASP

HomeAutomation Switchplate based on lvgl for ESP32
https://www.openhasp.com
MIT License
695 stars 179 forks source link

Custom Group Topic name does not work #697

Closed MrEcosse closed 5 months ago

MrEcosse commented 5 months ago

Perform all steps below and tick them with [x]

Describe the bug

The MQTT group topic is provided to enable sending an MQTT message to multiple selected plates simultaneously. The default name setting is plates and if this is left unchanged the group topic works. However, if you change the group topic name in the web interface it does not work. The GUI shows the new name but the plate still only subscribes to the 'plates' group topic. Effectively this means it is not possible to use the Group Topic function as intended.

To Reproduce

On a plate create an object such as:

{"page":1,"id":1,"obj":"label","x":10,"y":10,"w":120,"h":60,"text":"---","click":0,"text_font":42,"parentid":10}

Then from MQTT Explorer (or Home Assistant) send the command hasp/plates/command/p1b1.text with the value Hello World and the text will change.

Now rename the Group Topic to mygroup. Save and restart the plate.

Then from MQTT Explorer (or Home Assistant) send the command hasp/mygroup/command/p1b1.text with the value Have a Nice Day and the text will not change.

Confirmed by smcgann99 on Discord

openHASP rc11

Expected behavior

The ability to use a custom group name as described in the documentation. Otherwise, the Group Topic has the same functionality as the Broadcast Topic

Screenshots or video

marsman7 commented 5 months ago

I can't understand that. I have copied the jsonl line but deleted "parentid". I can send an MQTT group message to the label without any problems.

Please show more of your "pages.jsonl" and the complete text at Group Topic.

MrEcosse commented 5 months ago

If you send a message with the default group name of pages it all works OK. It is when you change the name of the group it no longer works.

marsman7 commented 5 months ago

it remains "hasp/plates/command/p1b1.text" despite group topic "hasp/mygroup/%topic%" I show later in source code.

marsman7 commented 5 months ago

I have found the place where the problem lies but I have no explanation for the error.

In "src/mqtt/hasp_mqtt_esp.cpp" i have added 2 LOGs

String mqttGetTopic(Preferences preferences, String subtopic, String key, String value, bool add_slash)
{
    String topic = preferences.getString(key.c_str(), value);

    LOG_INFO(TAG_MQTT,"mqttGetTopic 1 key[%s] topic[%s] value[%s]", key.c_str(), topic.c_str(), value.c_str());
    LOG_INFO(TAG_MQTT, "mqttGetTopic 2 key[%s] topic[%s]", FP_CONFIG_GROUP_TOPIC, preferences.getString(FP_CONFIG_GROUP_TOPIC, "notfound").c_str());

    topic.replace(F("%hostname%"), haspDevice.get_hostname());
    topic.replace(F("%hwid%"), haspDevice.get_hardware_id());
    topic.replace(F("%topic%"), subtopic);
    topic.replace(F("%prefix%"), MQTT_PREFIX);

    if(add_slash && !topic.endsWith("/")) {
        topic += "/";
    }
    return topic;
}

This function is called twice in "void mqttStart()". The output of the second log line has exactly the same parameters in both calls but the value of Topic is different. Why?

MQTT: mqttGetTopic 1 key[node_t] topic[hasp/foo/%topic%] value[hasp/%hostname%/%topic%]
MQTT: mqttGetTopic 2 key[group_t] topic[hasp/mygroup/%topic%]
MQTT: mqttGetTopic 1 key[group_t] topic[hasp/plates/%topic%] value[hasp/plates/%topic%]
MQTT: mqttGetTopic 2 key[group_t] topic[notfound]

"preferences.getString" returns different results.

marsman7 commented 5 months ago

On the second call of "mqttGetTopic" "preferences.getString" returns the error INVALID_HANDLE

stefan242 commented 5 months ago

Here the backlink to my similar issue: #686

marsman7 commented 5 months ago

The problem ist multible call of "mqttGetTopic" and in it the call of "preferences.getString".

In first call all ok, in all further calls no longer.

please review my fix.

MrEcosse commented 5 months ago

Thanks all for looking into and fixing this

smcgann99 commented 5 months ago

@MrEcosse Is this working for you now ? If so you can close the issue ?

marsman7 commented 5 months ago

For your information: The fix from above is a workaround but it works well. I have continued to search for the actual cause.

The actual problem is the parameter transfer from "preferences" to the function mqttGetTopic. The function works with a copy of "preferences". This can be fixed if only the pointer to "preferences" is passed to the function "mqttGetTopic".

String mqttGetTopic(Preferences *preferences, String subtopic, String key, String value, bool add_slash)

Only add the asterisk and call the function with

mqttNodeCommandTopic = mqttGetTopic(&preferences, subtopic, FP_CONFIG_NODE_TOPIC, MQTT_DEFAULT_NODE_TOPIC, false);

Only add ampersand. This means that you are not working with a copy of "preferences" but with the same instance. Working with the copy only works with the first call of "mqttGetTopic" from the second call onwards.

MrEcosse commented 5 months ago

Sorry, I have been travelling. Will be able to test tomorrow and report.

MrEcosse commented 5 months ago

All good with RC12