Add support for Govee water sensors to rtl_433_mqtt_hass

[kcpants]

This PR adds support for auto-discovering Govee water leak detectors and configuring them as binary moisture sensors. Moisture sensor configuration fields are modeled after the existing alarm and tamper binary sensors. This code has been tested with Govee Water Leak Detectors model H5054.

As discussed in #2404, some versions of the these Govee water sensors report a leak_num field while while others do not. This change attempts to address both versions of the sensor by relying on the event field instead. Since the event field is present in many different sensors' data, it is necessary to examine the contents of the field to verify that the mqtt data does indeed pertain to a water sensor. The logic to compare the contents of a field to a predetermined value has been added generically and could be used to aid in the discovery of other sensors in the future.

Note: The logic in bridge_event_to_hass was originally written as follows:

    # detect known attributes
    for key in data.keys():
        if key in mappings:
            if "value" in mappings[key] and mappings[key]["value"] != data[key]: continue

However, this prevents keys associated with a mapping that has a value field whose value field does not match from being added to the skipped_keys list so it was necessary to combine the new conditional logic with the main if statement.

[gdt]

Can you explain why the changed code will do the same thing as before for devices other than this sensor? I find this code to tricky to understand by reading it (not being familiar) and think it could do with a comment explaining the point.

[kcpants]

Sure!

I wasn't able to make the condition as readable as I would've liked for the reason mentioned in the description. I find a helpful way to think about the new condition is by translating it to english as "if the key from the decoded rtl data is present in the hardcoded mappings and either the "value" key is not present in the corresponding hardcoded mapping or, if a "value" key is present and matches the value in the decoded data then publish the config to home assistant (otherwise skip to the next key)"

Here is the condition for reference:

        if key in mappings and ("value" not in mappings[key] or mappings[key]["value"] == data[key]):

The idea of matching against the actual value of the key (and not just the name of the key) is what is being added in this pull request. The reason this does not affect the behavior of the preexisting devices is that none of them define a "value" key in their mappings so "value" not in mappings[key] always evaluates to true for them which simplifies the condition back to if key in mappings.

If you find the condition in the PR description (if "value" in mappings[key] and mappings[key]["value"] != data[key]: continue) more understandable, another way to verify the condition in the code is correct is to note that it's just the negation of that condition according to De Morgan's law !("value" in mappings[key] and mappings[key]["value"] != data[key]) == "value" not in mappings[key] or mappings[key]["value"] == data[key])

In case it helps, here is some sample data from the sensor I'm adding support for:

DEBUG:root:MQTT message: "{\"time\":\"2023-08-15T17:57:28.146836-0400\",\"model\":\"Govee-Water\",\"id\":21723,\"event\":\"Water Leak\",\"leak_num\":0,\"code\":\"54db32004748\",\"mic\":\"CRC\"}"

In the dictionary above, for key in data.keys(): iterates through time, model, id, event, leak_num, code, and mic. I added event to the hardcoded mappings with value: "Water Leak" meaning mappings["event"]["value"] equals "Water Leak". This way any data which contains an event whose value is not Water Leak will still be ignored (as it was prior to this change).

In principle, this approach would allow other sensors to map to the event key by setting a different value. However, it would then be necessary to alter the code to support multiple instances of the same key in the mappings dictionary. Since this would be a more significant change and it's unclear if it will ever actually be needed, I decided to leave it for the future.

Happy to add a comment if you'd like!

[kcpants]

After spending a little more time thinking about how this code could be made more readable, I came up with the following solution by inverting the conditional and reordering the contents of the main if-else statement:

        if key not in mappings or "value" in mappings[key] and mappings[key]["value"] != data[key]:
            if key not in SKIP_KEYS:
                skipped_keys.append(key)
        else:
            # topic = "/".join([topicprefix,"devices",model,instance,key])
            topic = "/".join([base_topic, key])
            if publish_config(mqttc, topic, model, device_id, mappings[key], key):
                published_keys.append(key)

I've included the logic as it's currently written in the pull request below for comparison:

        if key in mappings and ("value" not in mappings[key] or mappings[key]["value"] == data[key]):
            # topic = "/".join([topicprefix,"devices",model,instance,key])
            topic = "/".join([base_topic, key])
            if publish_config(mqttc, topic, model, device_id, mappings[key], key):
                published_keys.append(key)
        else:
            if key not in SKIP_KEYS:
                skipped_keys.append(key)

Let me know if you prefer this version and I will make the change!

[gdt]

        if key not in mappings or "value" in mappings[key] and mappings[key]["value"] != data[key]:
            if key not in SKIP_KEYS:
                skipped_keys.append(key)
        else:
            # topic = "/".join([topicprefix,"devices",model,instance,key])
            topic = "/".join([base_topic, key])
            if publish_config(mqttc, topic, model, device_id, mappings[key], key):
                published_keys.append(key)

I think I do prefer that, as it puts the normal path first.

But the point to me is the reason behind the condition, expressed as a comment in the code -- discussion in PRs does not live on -- not rephrasing code to English, but starrting from the rules about which keys are present and what that means.

But, I get the feeling that this is breaking new ground and establishing these rules, not handling previously-established rules. I don't know where the rules are documented, and I'm not understanding what is going on. That's my real issue with this.

[kcpants]

@gdt I've made the code changes requested and included an attempt at documenting both the preexisting fields in the mappings dictionary as well as the new value field that I am proposing in a comment above the mappings object in the script. Let me know if you think it's sufficient!

I get the feeling that this is breaking new ground and establishing these rules, not handling previously-established rules. I don't know where the rules are documented, and I'm not understanding what is going on. That's my real issue with this.

The value field is new and is needed because the Govee water sensors publish their alarm state to the "event" topic rather than a dedicated leak-sensor-specific topic (e.g. something like "leak" or "leak_detected").

For context, I chose the word "value" because the new field is used to perform a comparison against the value associated with the key from the MQTT rtl message. "value" could also be changed to another term if you think there is a word or phrase that better conveys the purpose of the field ("match", "value_match", "value_equals").

[kcpants]

Since the new conditional logic in this PR seems like it might be a bit of a sticking point, I thought it would be worth searching for other ways to automatically discover these Govee water sensors without altering the matching logic in this script at all.

As alluded to in other comments, the main challenge with these sensors is that they write sensor-specific data in the generic event key. For example, when a water leak is detected the following message is published to the rtl MQTT topic:

{
   "time":"2023-08-15T17:57:28.146836-0400",
   "model":"Govee-Water",
   "id":21723,
   "event":"Water Leak",
   "code":"54db32004748",
   "mic":"CRC"
}

When the button on the sensor is pressed to indicate that the leak has been investigated and resolved the following message is published to the rtl MQTT topic:

{
   "time":"2023-08-15T18:11:48.498135-0400",
   "model":"Govee-Water",
   "id":21723,
   "event":"Button Press",
   "code":"54db30543b5b",
   "mic":"CRC"
}

Given the way this script is currently written, in order to auto-detect these sensors we would add a new mapping entry which maps the event key to a config for a binary moisture sensor that is "on/wet" when a Water Leak event is received and "off/dry" when a Button Press event is received ( "payload_on": "Water Leak", "payload_off": "Button Press").

This simple approach would correctly discover the water sensors. However, searching through the decoders in this codebase reveals that there are at least 6 different devices that publish data in the event key. This wouldn't be a problem if all these devices were water sensors -- unfortunately, they are not (the Honeywell decoder is for a window / door sensor, the Yale decoder is for a home security alarm protocol, etc). Therefore, adding an "event" mapping to the mappings dictionary without any additional code changes to this script would result in all these devices being incorrectly added as moisture sensors.

To prevent the problem described in the paragraph above, it is necessary to add some logic to distinguish messages originating from this water sensor from messages originating from other non-water sensors. The solution proposed in this pull request is to add the ability to compare the data associated with a given key in an MQTT message against a predefined value and ignore otherwise matching keys whose value does not match. Specifically, the script only creates a binary moisture sensor device in home assistant if the value of the event key is "Water Leak". As written, only one value can be specified per key but the code could be extended to support multiple different predefined values in the future (so that the other sensors using the event key could be auto-discovered as well).

Alternate Solution -- Modify govee.c

Now that the details behind why the new conditional logic is needed have been explained, it's clear that mapping the Govee water sensors directly to the "event" key can't be done without some change to the body of the script. The only way to work around this without changing the script logic would be to change the Govee decoder to emit leak information on a new, "moisture sensor specific" key (e.g. something like "leak_status", "water_sensor_status", etc).

I'm not sure if a change to the decoder like this would be allowed or not, perhaps someone who has worked on that layer could comment (maybe @zuckschwerdt?). On the surface, this solution seems fine to me but I admittedly know nothing about how the decoded keys in the MQTT messages are derived so it's possible adding a new key would violate some of the rules or assumptions at that layer. Note that because many peoples' setups today certainly rely on the leak data being present in the event key, it would be necessary to duplicate the data into a new key (without removing it from the existing key).

If changing the decoder is preferred over the solution in this PR I'd be happy to give it a try (although I'd be even happier to test it with my Govee sensors if someone who already has already setup a build environment for the c code made the change).

Looking forward to hearing thoughts/ideas from the repo maintainers on this so auto-discovery for this sensor can be merged soon one way or another!

[zuckschwerdt]

Normalizing keys in decoders is always appreciated. Contributors often start an implementation by matching keys and values to some random display because no prior work is available or visible enough.

Perhaps some _alarm key, maybe water_ or liquid_ -- I don't think leak_ is universal enough.

[gdt]

First, I want to say to @kcpants thank you for taking feedback seriously and revising. We are definitely making progress. And sorry for taking so long; this is complicated and I needed a good chunk of time to page this in.

I agree (and putting my own spin on it) with @zuckschwerdt that in an ideal world we would have a clearer schema for the json output from rtl_433, and adjust that. My perhaps-too-hasty read is that we have key/value for keys that have semantics and values, like temperature, battery. But event: is not really a key in that sense with a value. It is really key event-foo and value "fired".

The biggest question is if it's ok to modify rtl_433_mqtt_hass to deal with how rtl_433 json output is as of now. I think I have to land on "yes, it's ok", because the alternative is an immediate rototill of all drivers that write the event key. But if we are willing to just do that, maybe that's ok.

The next question is about how this code works. I understand now that the general approach is that for each key in the json, an entity is created (unless it is skipped or not in the table), with many being diagnostic. The semantics now are that event: foo is a key, rather than event. The code deals with this by a next-level key where the value subkey is part of the matching, rather than part of the data that is looked up. This is hard to follow because it is semantically messy, and this is exactly what confused me when I started reading the diff without already understanding how this code works. It's now clear enough in the comments.
However, the code does this processing for any mapping table entry with a value subkey, not just event. I am not sure we have a reason to go there, as for now I think we are working around flaws in decoders.

One possible modification that might help clarity and cleanliness is to preprocess the json by saying (with very poor pseudopython):

# Convert "event: foo" into "event_foo: fired", so that keys are semantically distinct.
if key = event:
    event_value = data["value"]
    data["value"] = undef # really, drop this key from data
    data["event_" + "event_value"] = "fired"

and then doing the normal mappings processing. This also leaves a nice path to do this inside each decoder, and then when that's complete, we can drop this stanza.

I am not sure my suggestion is wise but would like to hear what y'all think.

[gdt]

I realize now that my suggestion doesn't work as is, because we would have to also preprocess the json payload we send.

So let me ask: how hard would it be to change the govee decode to emit event_waterleak:ON and event_waterleak:OFF instead? Or to use whatever values that the mqtt entity expects? And, are we creating a binary sensor, or are we creating distinct events? This is really stepping back and asking the bigger-picture "what kind of sensor is this really" question, but I'm now thinking that this path may not take very long at all and be better long term.

I have a zwave leak sensor. It is a binary sensor: water detected vs water not detected. there is no ack button. Does the govee sensor send a "water no longer detected"? I wonder if this should be binary sensor for leak, and then a button for ack. But if there is no 'no longer detected', I'm not sure. Plus it seems like this would need a periodic "i'm ok, no leak, battery is x'" message to have this make sense.

[gdt]

Looking at govee.c, I would say it deserves some updates, and I find it hard to follow.

[zuckschwerdt]

Adding a new output line is easy and compatible, we can keep everything else the same for some time, then drop the "old" event key.
The hard part really is to define a broad enough (naming) scheme so we don't end up with just one special key for a narrow familiy of sensors.

[gdt]

I wrote about the bigger picture on the mailinglist.

[zuckschwerdt]

It might be more efficient to use an int instead of fired (or active as fired implies a thing just now happening).

We can use DATA_COND to output only the 1 and supress the key for any unknown state (hypothetical since we seem to only get messages on alarm).

And to me event sounds like a thing currently happening. But the liquid (water?) would already be there, maybe for days. Is state more appropriate?
I have one such sensor and it does not detect a "leak" but a high "level" (it's in a well). Maybe just "wet" is the most general wording?

"state_wet", "Liquid detected", DATA_COND, wet_alarm, DATA_INT, 1,

[gdt]

Event is the HA term. The fundamental problem is that a leak detector is logically a binary sensor: is it wet or is it not wet. But the govee, while it senses like this, does not report like this. It sends a "wet" message on not-wet to wet transition, and then every 5m thereafter while it is still wet. On a wet to not-wet transition, it just doesn't send any more. At least that's what I understand from the comments. This is IMHO a broken design, and it takes a state machine and timers to turn it back into the binary sensor it ought to be. It is especially tricky to do this when there is packet loss. And, we know rtl_433 doctrine says that time-based processing is improper; we just emit representations of received messages.

I don't think it's always there. There are multiple codepoints in a field, and if that isn't "wet", then that particular message doesn't have a "it is wet" state. That might be because it's a battery report, or because it is reporting something else.

HA seems to use event for things like button press. A 'it is wet" report is sort of like this. But it is sort of not. The right fix is to have the state machine (with timers) in between. The alternative is to kludge it into HA entities via representing "wet" as an event and have another entity (binary sensor) be true if there has been a wet event in the last 17 minutes (as an example).

My conclusion from all this is not to buy any govee stuff!

[ifuchs]

is anyone trying to make this work with HA? I just need to decide whether to return these sensors.

[zuckschwerdt]

You can use the changed rtl_433_mqtt_hass.py from this PR and it will "just work" (given an initial wet trigger I assume?)

[ifuchs]

Thanks! And sorry for asking what probably should be obvious but how do I go about incorporating this change into HA?

[ifuchs]

I finally figured out how to replace the rtl_433_mqtt_hass.py file in the rtl_433 autodiscovery addin in HA. I restarted it and I caused a water leak transmission but although there is a Govee-water-29010 device, there are no entities (e.g., event) from the payload. MQTT explorer shows the mqtt received by HA.

[Jsalas424]

I finally figured out how to replace the rtl_433_mqtt_hass.py file in the rtl_433 autodiscovery addin in HA. I restarted it and I caused a water leak transmission but although there is a Govee-water-29010 device, there are no entities (e.g., event) from the payload. MQTT explorer shows the mqtt received by HA.

Can you please share how you replaced the rtl_433_mqtt_hass.py file in the rtl_433 autodiscovery addin in HA?

[ifuchs]

Sure. In order to access the mqtt_auto_discovery container, I installed Portainer and accessed the container and the console. From there I just did a wget to retrieve the file. If you need more details, just ask. However, I still can't make any use of the Govee sensors which is my objective.

[ifuchs]

Ha. Your question made me go back and check the file, and lo and behold, though I had replaced the file, apparently it gets replaced with the old version when I restart the add-in. How do I replace the addin and have the new file?

[ifuchs]

@Vlad-Star Every time I take a step forward (and think I have it working) I take 1 (or 2) steps backward. I added a second Govee sensor. It works fine but now the first sensor says "unavailable". I can see the MQTT messages from it in Explorer but HA no longer sees it. The only change I made was to mqtt.yaml. I copied the definition of the first sensor and changed the id and name:

%cat mqtt.yaml
binary_sensor:
state_topic: rtl_433/raspberrypi/devices/Govee-Water/29010/event
json_attributes_topic: rtl_433/raspberrypi/devices/Govee-Water/29010
device_class: moisture
unique_id: leak_29010_crawlspace
name: Leak - Crawl Space
payload_on: "Water Leak"
payload_off: "Button Press"

binary_sensor:
state_topic: rtl_433/raspberrypi/devices/Govee-Water/30031/event
json_attributes_topic: rtl_433/raspberrypi/devices/Govee-Water/30031
device_class: moisture
unique_id: leak_30031_utilityroom
name: Leak - Utility Room
payload_on: "Water Leak"
payload_off: "Button Press"

[Jsalas424]

@ifuchs @Jsalas424 This may get a bit tricky when you already have a sensor in your configuration. This is what I normally do in case my sensor is not discovered correctly:

1. Remove batteries from the sensor

2. Remove the sensor from everywhere (entries/devices/MQTT devices) in HA, just search for the number

3. Restart HA core (not just reload). **UPD:** You may lose your all Govee battery readings at this point, they will eventually get back.

4. Make sure rtl_433 and discovery plugins are running. I currently use "next" version of rtl_433 and a "regular" version of rtl_433 MQTT discovery. Enable "debug" level in the discovery configuration, restart this plugin if necessary

5. Insert batteries into the sensor, observe discovery messages in the debug log. At this point I am getting battery + UTC entries for the sensor auto-added by discovery plugin, but not the leak sensor itself

6. Add a code for the leak sensor to mqtt.yaml (or configuration.yaml). Note, your entries should have a slightly different syntax in those files, depending on where you add them

7. Reload HA configuration. At this point I am getting manually added (in the previous step) leak sensor

@Vlad-Star you're a gem, thank you for patiently troubleshooting with all of us! People like you keep our community moving forward.

I didn't have MQTT auto-discovery on, as soon as I turned it on and pressed the button on the sensor, a new entry line pops up in MQTT explorer and a device pops us. I can also see the battery entity, not pictured here:

but I went into entities and deleted the entry regardless to start over and make sure everything gets paired correctly from the get go. I also emptied out mqtt.yaml.

I didn't remove batteries because I was lazy I had to remove the batteries to get it to send the initial battery reading message. It seems to only send this out when it changes or when it power cycles.

I followed the rest of the steps in order. For documentation's sake I will include some logs snippets.

After completing steps 1 through 5, the MQTT Auto Discovery showed the following when I pressed the button:

DEBUG:paho.mqtt.client:Received PINGRESP
DEBUG:paho.mqtt.client:Received PUBLISH (d0, q0, r0, m0), 'rtl_433/9b13b3f4-rtl433-next/events', ...  (209 bytes)
DEBUG:root:MQTT message: "{\"time\":\"1695322311.360389\",\"protocol\":231,\"model\":\"Govee-Water\",\"id\":39787,\"event\":\"Button Press\",\"code\":\"9b6b3054a2ee\",\"mic\":\"CRC\",\"mod\":\"ASK\",\"freq\":433.92634,\"rssi\":-0.0983849,\"snr\":9.295,\"noise\":-9.39339}"
DEBUG:root:homeassistant/sensor/Govee-Water-39787/Govee-Water-39787-UTC/config:{"device_class": "timestamp", "name": "Govee-Water-39787-UTC", "entity_category": "diagnostic", "enabled_by_default": false, "icon": "mdi:clock-in", "state_topic": "rtl_433/9b13b3f4-rtl433-next/devices/Govee-Water/39787/time", "unique_id": "Govee-Water-39787-UTC", "device": {"identifiers": ["Govee-Water-39787"], "name": "Govee-Water-39787", "model": "Govee-Water", "manufacturer": "rtl_433"}}
DEBUG:paho.mqtt.client:Sending PUBLISH (d0, q0, r0, m2), 'b'homeassistant/sensor/Govee-Water-39787/Govee-Water-39787-UTC/config'', ... (394 bytes)
DEBUG:root:homeassistant/sensor/Govee-Water-39787/Govee-Water-39787-rssi/config:{"device_class": "signal_strength", "unit_of_measurement": "dB", "value_template": "{{ value|float|round(2) }}", "state_class": "measurement", "entity_category": "diagnostic", "state_topic": "rtl_433/9b13b3f4-rtl433-next/devices/Govee-Water/39787/rssi", "unique_id": "Govee-Water-39787-rssi", "name": "Govee-Water-39787-rssi", "device": {"identifiers": ["Govee-Water-39787"], "name": "Govee-Water-39787", "model": "Govee-Water", "manufacturer": "rtl_433"}}
DEBUG:paho.mqtt.client:Sending PUBLISH (d0, q0, r0, m3), 'b'homeassistant/sensor/Govee-Water-39787/Govee-Water-39787-rssi/config'', ... (456 bytes)
DEBUG:root:homeassistant/sensor/Govee-Water-39787/Govee-Water-39787-snr/config:{"device_class": "signal_strength", "unit_of_measurement": "dB", "value_template": "{{ value|float|round(2) }}", "state_class": "measurement", "entity_category": "diagnostic", "state_topic": "rtl_433/9b13b3f4-rtl433-next/devices/Govee-Water/39787/snr", "unique_id": "Govee-Water-39787-snr", "name": "Govee-Water-39787-snr", "device": {"identifiers": ["Govee-Water-39787"], "name": "Govee-Water-39787", "model": "Govee-Water", "manufacturer": "rtl_433"}}
DEBUG:paho.mqtt.client:Sending PUBLISH (d0, q0, r0, m4), 'b'homeassistant/sensor/Govee-Water-39787/Govee-Water-39787-snr/config'', ... (453 bytes)
DEBUG:root:homeassistant/sensor/Govee-Water-39787/Govee-Water-39787-noise/config:{"device_class": "signal_strength", "unit_of_measurement": "dB", "value_template": "{{ value|float|round(2) }}", "state_class": "measurement", "entity_category": "diagnostic", "state_topic": "rtl_433/9b13b3f4-rtl433-next/devices/Govee-Water/39787/noise", "unique_id": "Govee-Water-39787-noise", "name": "Govee-Water-39787-noise", "device": {"identifiers": ["Govee-Water-39787"], "name": "Govee-Water-39787", "model": "Govee-Water", "manufacturer": "rtl_433"}}
DEBUG:paho.mqtt.client:Sending PUBLISH (d0, q0, r0, m5), 'b'homeassistant/sensor/Govee-Water-39787/Govee-Water-39787-noise/config'', ... (459 bytes)
INFO:root:Published Govee-Water/39787: time, rssi, snr, noise
INFO:root:Skipped Govee-Water/39787: protocol, event, code

With my rtl_433.conf looking like this:

output mqtt://homeassistant.local:1883,user=<USER>,pass=<PASS>
convert si
frequency 434.000M

report_meta level
report_meta noise
report_meta stats
report_meta time:unix:usec:utc
report_meta protocol

I then added the following to mqtt.yaml

binary_sensor:
  - name: Govee Water Leak Detector 39787
    state_topic: rtl_433/9b13b3f4-rtl433-next/devices/Govee-Water/39787/event
    json_attributes_topic: rtl_433/9b13b3f4-rtl433-next/devices/Govee-Water/39787
    unique_id: govee_water_39787
    device_class: moisture
    payload_on: Water Leak
    payload_off: Button Press

sensor:
  - name: Govee Water Leak Detector Battery 39787
    state_topic: rtl_433/9b13b3f4-rtl433-next/devices/Govee-Water/39787/battery_ok
    json_attributes_topic: rtl_433/9b13b3f4-rtl433-next/devices/Govee-Water/39787
    device_class: battery
    value_template: "{{ (value_json | float * 100) | int }}"
    unique_id: govee_water_39787_b
    unit_of_measurement: "%"
  - name: Govee Water Leak Detector rssi 39787
    state_topic: rtl_433/9b13b3f4-rtl433-next/devices/Govee-Water/39787/rssi
    json_attributes_topic: rtl_433/9b13b3f4-rtl433-next/devices/Govee-Water/39787
    device_class: None
    value_template: "{{value_json}}"
    unique_id: govee_water_39787_rssi
    unit_of_measurement: None

at which point I see the following:

A success!

[ifuchs]

@Jsalas424 that's great! I will be interested to hear what happens if you add a second sensor definition in your mqtt.yaml file. As I just reported I added a second sensor and it works fine but the first one is now unavailable.

[ifuchs]

I found my problem with adding the 2nd sensor. It was a syntax problem in mqtt.yaml. As you can see in my post above, I had binary_sensor twice. The working version looks like this:
binary_sensor:

• name: Leak - Crawl Space
  state_topic: rtl_433/raspberrypi/devices/Govee-Water/29010/event
  json_attributes_topic: rtl_433/raspberrypi/devices/Govee-Water/29010
  device_class: moisture
  unique_id: leak_29010_crawlspace
  payload_on: "Water Leak"
  payload_off: "Button Press"
• name: Leak - Utility Room
  state_topic: rtl_433/raspberrypi/devices/Govee-Water/30031/event
  json_attributes_topic: rtl_433/raspberrypi/devices/Govee-Water/30031
  device_class: moisture
  unique_id: leak_30031_utilityroom
  payload_on: "Water Leak"
  payload_off: "Button Press"
  @Jsalas424 Thanks again for all of your patient assistance.
  Unrelated: I have played with 2 sensors out of 5 and one thing I have observed is that one of them is distinctly louder than the other. Manufacturing variation or perhaps the battery is weaker?

[Vlad-Star]

@ifuchs I am glad I helped you to figure out the problem :) (though this PR wasn't really right place for such a discussion - I apologize to the other folks for so many comments)

[ifuchs]

@Vlad-Star thanks again. It would've taken forever without your help.

[gdt]

I finally got caught up on the comments above. It looks like we are still waiting for the govee driver to be adapted, and then this can be adapted and then we can re-review.

[kcpants]

I finally got caught up on the comments above. It looks like we are still waiting for the govee driver to be adapted, and then this can be adapted and then we can re-review.

Correct. When the other PR is merged the change in this file will be simplified to a new mapping entry and the new value key and associated logic that I added will be removed. Something like:

    "detect_wet": {
        "device_type": "binary_sensor",
        "object_suffix": "moisture",
        "config": {
            "name": "Water Sensor",
            "device_class": "moisture",
            "force_update": "true",
            "payload_on": 1,
            "payload_off": 0
        }
    },

[zuckschwerdt]

For reference the PR to add the normalized key is #2625.

[kcpants]

With the change to govee.c merged, I am returning to the work needed to make rtl_433_mqtt_hass.py automatically discover and add binary moisture sensors to Home Assistant. I believe the change that is now included in the PR will be sufficient but I haven't yet run tests to confirm the new behavior so please refrain from merging until I have posted a follow-up comment with the test results.

The new change is much simpler than the original. I think the main discussion-worthy points are the name and suffix that will be used for binary moisture sensors in HA.

        "object_suffix": "moisture",
        "config": {
            "name": "Water Sensor",

[gdt]

I am not yet using this script, but the change looks good to me -- so I'm +1 on merging after a positive test report. Thanks very much for bearing with us. It was a long slog but I think the end result is much better!

[kcpants]

No worries, the first iteration of the solution in this pull request was hacked together under the assumption that the json keys were unchangeable. I'm glad to have been able to dig into the details and I agree that the solution we all converged on is much cleaner.

Sorry for the slight delay here -- I'm testing this code within the rtl_433 HA add-ons and needed to wait for the nightly build of the rtl_433-next add-on to pick up the detect_wet change.

With the underlying PR merged, the change in this PR works as expected. I've attached screenshots below for reference.

To test I deleted all of my manually added Govee moisture sensors, ran the new version of rtl_433_mqtt_hass.py, pushed the button on each sensor to send a message with detect_wet: 0 (this added a water sensor entity to HA), then removed and reinserted the battery (this added a battery entity to HA). This was expected since the message that includes detect_wet will not include battery_ok or battery_mV.

I also had 2 brand new leak sensors which still contained the battery pull tab. For some reason simply pulling the battery tab to activate the device was enough for a water leak entity and a battery entity to be added to HA. I didn't capture the json but I assume this means the very first message ever sent by the device includes both battery_ and Button Press (maybe as a self-diagnostic).

[Jsalas424]

No worries, the first iteration of the solution in this pull request was hacked together under the assumption that the json keys were fixed. I'm glad to have been able to dig into the details and I agree that the solution we all converged on is much cleaner.

Sorry for the slight delay here -- I'm testing this code within the rtl_433 HA add-ons and needed to wait for the nightly build of the rtl_433-next add-on to pick up the detect_wet change.

With the underlying PR merged, the change in this PR works as expected. I've attached screenshots below for reference.

I'm not having as much luck as you are. I get a battery entity, but no water entity. My MQTT Explorer does reflect a new "detect_wet" field AND my Auto Discovery logs show that being captured correctly but then ignored:

DEBUG:root:MQTT message: "{\"time\":\"1695935192.764993\",\"protocol\":231,\"model\":\"Govee-Water\",\"id\":39497,\"event\":\"Water Leak\",\"detect_wet\":1,\"leak_num\":4,\"code\":\"9a493204006b\",\"mic\":\"CRC\",\"mod\":\"ASK\",\"freq\":433.92758,\"rssi\":-0.123135,\"snr\":12.25757,\"noise\":-12.3807}"

INFO:root:Skipped Govee-Water/39497: protocol, event, detect_wet, code

To test this, I performed the following:

1. Stop & Uninstall rtl_433
2. Removed mqtt.yaml file, rebooted, removed the entities and devices, rebooted
3. Installed rtl_433 (next) rtl_433 version nightly-18-g02724080 branch master at 202309272030
4. Turned on rtl_433 (next) and rtl_433 MQTT Auto Discovery.
5. Take battery out of device and put them back in, I get a battery entity automatically made

but I don't have the water sensor data:

Edit: I proceeded to grab a fresh version of rtl_433 MQTT Auto Discovery (next) but I'm still having the issue here: pbkhrv/rtl_433-hass-addons#155 (comment)

[kcpants]

@Jsalas424 What steps did you take to replace the contents of the rtl_433_mqtt_hass.py script in the rtl_433 MQTT Auto Discovery (next) add-on with the version contained in this pull request? Note that uninstalling and reinstalling rtl_433 MQTT Auto Discovery (next) will not pick up this change since it has not yet been merged.

I have been testing this by logging into the rtl_433 MQTT Auto Discovery (next) docker container, suspending the run.sh script (so that the container will not exit), killing the python script, copying the latest version of the python script into place, setting the MQTT_USERNAME and MQTT_PASSWORD environment variables appropriately, then running the script manually.

[Jsalas424]

@Jsalas424 What steps did you take to replace the contents of the rtl_433_mqtt_hass.py script in the rtl_433 MQTT Auto Discovery (next) add-on with the version contained in this pull request? Note that uninstalling and reinstalling rtl_433 MQTT Auto Discovery (next) will not pick up this change since it has not yet been merged.

I have been testing this by logging into the rtl_433 MQTT Auto Discovery (next) docker container, suspending the run.sh script (so that the container will not exit), killing the python script, copying the latest version of the python script into place, setting the MQTT_USERNAME and MQTT_PASSWORD environment variables appropriately, then running the script manually.

@kcpants I only uninstalled and reinstalled, I thought the change had merged. Sorry!

[kcpants]

I cleaned up the commit history in my clone of the repo to prepare for merging. Given the test results included in the comment above, I don't believe there is any unaddressed feedback on the current version of the fix contained in this PR. @zuckschwerdt I'm happy to make changes if folks have more suggestions, otherwise I think this is ready to go in.

[gdt]

I'm +1 to merge 41bc746 as just pushed by @kcpants yesterday.

[kcpants]

@gdt I think this pull request may have slipped through the cracks. It did get a bit noisy with all the comments pertaining to the underlying problem so perhaps zuckshwerdt muted the thread after leaving this comment and hasn't seen the latest updates?

At this point it's not clear to me what needs to be done to get this merged. Perhaps it would help to try contacting someone with write access to the repo using a channel other than the comments in this particular pull request? I'd really like to see this closed so please let me know if there's anything I can do to help!

[ifuchs]

I don't know if this thread is "dead" but I am having problems (again) getting the Govee leak sensors working. This was working but I had to move from running on a Mac Mini to a Windows system (both running VirtualBox) and it is possible that something was lost along the way. I am running rtl_433 on a Raspberry Pi and sending MQTT messages to HA. I have many Acurite temperature sensors and all of them are working fine via rtl_433 _> MQTT _> HA. When I press the button on the Govee I can see it is received by the rtlsdr and sending mqtt to HA. However, MQTT Browser does not show these messages. I am running MQTT Auto Discovery on HA although these devices already exist and are properly defined in mqtt.yaml. Any ideas about how to figure out where this is failing?

P.S. I was going to delete this comment but I found the problem and just in case anyone makes the same (silly) mistake (unlikely but possible) here is what happened. I was being inundated with new devices being defined in HA so I finally turned off auto discovery and then to reduce the load on the MQTT broker, I added several -R [protocol #] parms to the rtl_433 command running on the RPi in order to accept only certain device types. Unfortunately, I forgot one protocol...the Govee leak sensor. So those messages were being duly ignored at the RPi. I added that protocol back in and voila, HA sees the sensors again.