letscontrolit / ESPEasy

Easy MultiSensor device based on ESP8266/ESP32
http://www.espeasy.com
Other
3.29k stars 2.22k forks source link

[BUG] Homie MQTT convention ignores unit number #4864

Open Didier-SimpleCommeDev opened 1 year ago

Didier-SimpleCommeDev commented 1 year ago

Describe the bug Homie MQTT convention sends auto-discovery messages without the unit number, but cannot be setup to send the actual data without the unit name.

To Reproduce Steps to reproduce the behavior:

  1. Use firware with Collection A
  2. Set the unit name (ESP_Easy_proto), the Unit Number (1), and check "Append Unit Number to hostname"
  3. Configure the MQTT Homie controller
  4. For this controller, set "Controller publish" to homie/%sysname%/%tskname%/%valname%
  5. Set Controller LWT topic to homie/%sysname%/$state
  6. Add any device and configure it to push data to the controller
  7. Monitor MQTT messages sent to homie/#

Expected behavior Either one of the following:

Screenshots Observed behavior from MQTT logs (Mosquitto): anouncement messages are sent to homie/ESP_Easy_proto and device-related messages (data, LWT) are sent to homie/ESP_Easy_proto_1

homie/ESP_Easy_proto/$state init
homie/ESP_Easy_proto/$homie 4.0.0
homie/ESP_Easy_proto/$name ESP_Easy_proto
homie/ESP_Easy_proto/SYSTEM/$name SYSTEM
[...]
homie/ESP_Easy_proto/Mobile_temperature/temperature/$name temperature
homie/ESP_Easy_proto/Mobile_temperature/temperature/$datatype float
homie/ESP_Easy_proto/Mobile_temperature/$name Mobile_temperature
homie/ESP_Easy_proto/Mobile_temperature/$type Environment - 1-Wire Temperature
homie/ESP_Easy_proto/Mobile_temperature/$properties temperature
[...]
homie/ESP_Easy_proto_1/$state ready
homie/ESP_Easy_proto/$state ready
homie/ESP_Easy_proto/$state ready
homie/ESP_Easy_proto_1/Mobile_temperature/temperature 84.5

Used platform (please complete the following information):

Analysis Plugin C014 uses the following source for base topic: pubname.replace(F("%sysname%"), Settings.getName());. Documentation for getName() reads:

Return the name of the unit, without unitnr appended

AFAIK, there is no system variable for "unit name without the unit number", so there is no way to configure the controller "data" and "LWT" topics without the unit number.

Workaround: disable "Append Unit Number to hostname"

TD-er commented 1 year ago

Just taken a screenshot from how the OpenHAB MQTT controller handles the client ID setup: image

Would this be what you like to have? N.B. the variable %sysname% does honor the "append unit to hostname" checkbox.

Is it a good enough fix to add a variable %hostname%, which does NOT append the unit to the hostname?

Edit: Hmm this might be a bit confusing as we label it "Unit Name" on the Config tab and the checkbox mentions "Append Unit Number to Hostname"

So not sure what would be the best name for such a system variable as "%unitname%" would be rather confusing too if it is the hostname without unit nr.

Didier-SimpleCommeDev commented 1 year ago

Hi! %sysname% indeed honors the unit number, so when somebody uses it for the "controller publish topic" (as documented in the plugin page), the unit number is part of the data path (e.g. homie/ESP_Easy_1). The issue is that the controller itself does not honor the unit number, and publishes "announcement" messages (used by consumers to discover new topics) without the unit number (e.g. homie/ESP_Easy/...). This inconsistency prevents the consumer from reading the data (declared as homie/ESP_Easy/.../mySensor, but published as homie/ESP_Easy_1/.../mySensor).

Agreed with the confusing names, even more because the document page https://espeasy.readthedocs.io/en/latest/Controller/C014.html refers to %unitname% which is not a valid system variable.

My 2 cts, I see two solutions:

  1. Either this controller honors the unit number (i.e. sends announcements to e.g. ESP_Easy_1), maybe optionnaly to avoid breaking existing setups?
  2. or we need a system variables which is the plain system name without the unit appended — and we need a self explicit name, like you mentionned.

I’d go with option 1 because unit number really seems to be a thing, so we lose a feature if this controller does not honor it, but that means updating the plugin code and maybe UI – beyond my skills at this point, especially the UI part!

tonhuisman commented 1 year ago

The issue is that the controller itself does not honor the unit number, and publishes "announcement" messages (used by consumers to discover new topics) without the unit number (e.g. homie/ESP_Easy/...).

I have been working on Controller improvements, and will incorporate this as a fix in the pending changes.

Edit: See PR #4770

Didier-SimpleCommeDev commented 1 year ago

I have been working on Controller improvements, and will incorporate this as a fix in the pending changes.

Edit: See PR #4770

Excellent news :) I’ll keep an eye on this PR, maybe I can build/test the branch locally and see how it goes.

Thanks for all the effort!

tonhuisman commented 1 year ago

maybe I can build/test the branch locally and see how it goes.

After a push to a branch the matching GH Actions run will have a complete build for your in ca. 45 minutes (or somewhat longer if another build starts in parallel), so no need to build locally 😃 I'll provide a link to a run to get you going, once I have something for you to test 😉

tonhuisman commented 1 year ago

I’ll keep an eye on this PR

@Didier-SimpleCommeDev A GH Actions run is brewing that uses the correct getHostname() instead of getName().