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

Add auto discovery for Weewx to HomeAssistant #36

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

anastas78
Copy link

Use HomeAssistant feature to add devices and sensors based on information delivered to MQTT broker.

Resolves #26

The implementation referenced by @ThomDietrich was a very "rough" try to get some data into HA from my weewx. Based on the interest in such feature and the comments of @matthewwall I tried to make a more generic and complete implementation.
Current PR is able to create a device (with some information about weewx - probably could be improved??) and define sensors of this device based on the record data. Missing, badly configured or disabled discovery should not change the standard behavior.

perhaps this could be done as a generic discovery, not specific to ha?

I tried to move to config most of the options, but mqtt autodiscovery is something implemented by HA (a quick googling showed that other home automation systems followed their documentation) and I worked only using their documentation. I would suggest to keep the ha_ prefix at least till someone tests it against other software. Probably then we will have to move the HA_SENSOR_TYPE, HA_SENSOR_UNIT also to config.

i'm a little anxious about sending a discovery message on every loop/archive. perhaps also have a mechanism to indicate how often the discovery info should be sent.

In order in future to implement such a mechanism I've set the retain flag on discovery messages (that helps sensor survive a reboot of HA). In my opinion we can save some resources with it(bandwidth and server load), but really not something impressive as normally weewx has an archive record every 300s and probably a loop record at +-30s. If we have a very long interval for sending configuration it could lead to situation where sensor data is available at broker, but configuration is missing and data is not usable by HA.(imo worse case than "n" more messages)
I was thinking that we could write down last_update_time somewhere (file or broker) and check time delta in process_record(). (Both scenarios will eat some of the aimed positives.) Or send it only on archive record (so use the standard weewx interval). I would be happy to consider any suggestion / opinion about the implementation of such a mechanism.

P.S. The is my first PR and Python is for me far from "native" language. I am really away from programming and have only written some code for my personal use till now. Any suggestions and comments are well welcomed.

# The type of data delivered by a sensor, impacts how it is displayed in the frontend of HomeAssistant.
# Each sensor can have defined device_class and/or unit_of_measurement passed in configuration
# https://www.home-assistant.io/integrations/sensor/
HA_SENSOR_TYPE = {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HA_SENSOR_TYPE needs mm and mm_per_hour. I used a unit customisation to get rain in mm, but did not work until I added mm to thsi lookup. Same for HA_SENSOR_UNIT

@bakerkj
Copy link

bakerkj commented Jan 4, 2024

It would be great to get this merged. Is there anything that I could help with to unblock the merge?

@ThomDietrich
Copy link

@matthewwall did you ever reconsider our discussion regarding moving your weewx repositories to an organization with multiple contributors? It's a shame that valuable additions like this one are left unanswered for months. I would like to nominate myself as one of multiple maintainers in such an organization.
Thanks for consideration

@jshatch
Copy link

jshatch commented Jan 13, 2024

@anastas78 The rainRate sensor is invalid in Home Assistant if the units are set to inches per hour. It identifies the SensorDeviceClass as 'device_class': 'inch_per_hour' and HA barfs on it so the topic never becomes added as an entity.

Also barometer_inHg is doing a similar thing: 'device_class': 'inHg' rather than 'device_class': 'atmospheric_pressure'. There are probably some others, might need to double-check the discovery topics that are in imperial units. Also the value template should be "value_template": "{{ value | float | round(2) }}", instead of round(1) for barometric pressure. I manually published that topic with these modifications and the sensor now shows up correctly.

Decided I should just submit a PR to your fork, it probably doesn't address everything it could but it does address the entities I need for now. anastas78#2

@bdf0506
Copy link

bdf0506 commented Jan 26, 2024

I ran into the same thing with inch_per_hour. I also ran into an issue with lux not being mapped to illumiance and had to manually map that one too.

Overall this PR needs a little work, since it publishes a discovery topic on each and every loop packet which clutters the home assistant logs. Would be nice to see this reworked where that doesn't happen constantly, or maybe moved to a debug message.

@jshatch
Copy link

jshatch commented Jan 26, 2024

I did this to address the frequency of the discovery topic: anastas78#4

@dcapslock
Copy link

Thinking about the discovery chatter and reducing it, a possible solution would be subscribe to the Hone Assistant status MQTT topic (homeassistant/status) and then flag a discovery message publish on the next loop or archive packet. For my own setup I now have Home Assistant as the data master and use the mqtt driver for Weeex so no longer use this extension. Otherwise I would have a go at implementing my suggested method.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Home Assistant discovery metadata
6 participants