fmartinou / whats-up-docker

What's up Docker ( aka WUD ) gets you notified when a new version of your Docker Container is available.
https://fmartinou.github.io/whats-up-docker
MIT License
1.02k stars 33 forks source link

Suggestion - change binary_sensor name to host #137

Closed robertalexa closed 2 years ago

robertalexa commented 2 years ago

Hi,

Just set this up and so far I like it a lot :) You did a great job with this project.

I have one suggestion, which if implemented will be a breaking change for users with existing automations in HA, but it could be rolled out in parallel with a grace period so people have time to change them.

My recommendation would be to change the name (and implicitly the ID) of the binary sensor that gets created from wud to host_name. In my setup, i have WUD set up on 2 hosts (instead of 1 single instance accessing the docker socket) and I get 2 binary sensors named wud image

binary_sensor.wud and binary_sensor.wud_2

By using the hostname it will become self explanatory which environment they represent.

Thanks

fmartinou commented 2 years ago

Hi,

Thank you very much for your support!

About names, The default behavior is to give entities the container name as HA displayName, which can actually conflict in case of multi hosts setup. To get around, you can set custom display names by using the wud.display.name label on your containers wud.display.name=Nginx myHost1 wud.display.name=Nginx myHost2 ...

About ids, There is maybe a bug there, but the expected behavior is to get the id computed as ${topic}_${container.watcher}_${containerName}

So in case of multi hosts setup, you should get unique entity ids like wud_container_host1_nginx wud_container_host2_nginx ...

(host1, host2 are the name you give to the watcher (using WUD_WATCHER_{watcher_name}_* env vars))

But the fact you get binary_sensor.wud and binary_sensor.wud_2 ids let me think that HA is not taking the id I provide but a generated one based on the name 🤔 .

I will look at it in more details.

robertalexa commented 2 years ago

Hi @fmartinou

Been a while since I got some time to further look into my setup. But now I have more containers showing in HA so i can provide more data to you showing the bug.

image image

binary_sensor.grafana, binary_sensor.homer, binary_sensor.wud, binary_sensor.wud_2 - all of them follow the same pattern.

Looking at the MQTT information image

So the topic is correct and following the pattern you have suggested above homeassistant/binary_sensor/wud_container_homepi_wud/config

Looking at the documentation for MQTT binary sensors: https://www.home-assistant.io/integrations/binary_sensor.mqtt/ image

The object_id is what we are after. The name could stay as it is (e.g. homer), or it could include the host. Although for single host users this will look bad and should probably be left as it is and multi host users can add it themselves in HA as the entity is editable. As for object_id this would be where the host should be prepended host_containerName

As a result, I have created a PR to fix this available at https://github.com/fmartinou/whats-up-docker/pull/140

I will close this issue but I am happy to continue the conversation if needed either here or on the PR

fmartinou commented 2 years ago

@robertalexa Thanks for your investigations and the PR 👍 I'll give a try to your PR updated version and I get back to you if I have additional questions. Thanks again!