Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Homie MQTT convention ignores unit number #4864

Open
Didier-SimpleCommeDev opened this issue Oct 30, 2023 · 6 comments · May be fixed by #4770
Open

[BUG] Homie MQTT convention ignores unit number #4864

Didier-SimpleCommeDev opened this issue Oct 30, 2023 · 6 comments · May be fixed by #4770
Labels
Category: Controller Related to interaction with other platforms Status: Needs Info Needs more info before action can be taken Type: Enhancement Improve something already present

Comments

@Didier-SimpleCommeDev
Copy link

Didier-SimpleCommeDev commented Oct 30, 2023

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:

  • (ideally) Status messages and data messages are sent to /homie/ESP_Easy_proto_1/#, or
  • Status messages and data messages are sent to /homie/ESP_Easy_proto/#

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):

  • ESP type: ESP8266
  • Build version: mega-20231013 stock
  • Build set collection_A

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
Copy link
Member

TD-er commented Oct 30, 2023

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.

@TD-er TD-er added Type: Enhancement Improve something already present Category: Controller Related to interaction with other platforms Status: Needs Info Needs more info before action can be taken labels Oct 30, 2023
@Didier-SimpleCommeDev
Copy link
Author

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
Copy link
Contributor

tonhuisman commented Oct 30, 2023

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
Copy link
Author

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
Copy link
Contributor

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
Copy link
Contributor

I’ll keep an eye on this PR

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

@tonhuisman tonhuisman linked a pull request Oct 30, 2023 that will close this issue
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Controller Related to interaction with other platforms Status: Needs Info Needs more info before action can be taken Type: Enhancement Improve something already present
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants