add device_topic_suffix arg

[unverbraucht]

Summary

First, thanks for a great add-on :)

rtl_433 supports custom topic name configurations and the HA MQTT helper script recently got support for it. This PR makes this topic name configurable also in the HA add-on and maps it through to rtl_433_mqtt_autodiscovery script invocation.

Alternatives Considered

Since this feature has been added downstream and the add-on wraps down-stream I think this is the most sane way of achieving this. Alternatives would be to run rtl_433_mqtt_home_assistant manually on a different node.

Testing Steps

1. Set up rtl_433 to use a different device topic path. My use case was to remove id from the path since they change with every battery change. Example line in rtl_433.conf:

output mqtt://IP,user=USERNAME,pass=PASS,devices=rtl_433/localhost/devices[/type][/model][/subtype]/C[channel:0],events=rtl_433/localhost/events

3. Now observe that the rtl433 HA MQTT add-on stops making working autoconfig topics because it assembles the wrong topic path
4. With this PR, set device_topic_suffix to the same value (rtl_433/localhost/devices[/type][/model][/subtype]/C[channel:0]) and it should work again.

[touste]

I've tested this and was also able to remove the ID from the device name, which also randomly changes from time to time on my device. Very useful, thanks!

[deviantintegral]

This looks great! I will be setting up my rain meter which changes ids in the next week or so. That will give me a good opportunity to test this.

[deviantintegral]

I pushed a few documentation updates to your branch.

It looks like there's some typos in the run script. I will commit the fix eventually if I don't hear back, but I wanted to double check because others have said this PR worked fine for them... but then I don't see how that's possible.

[unverbraucht]

@deviantintegral thanks for pointing that out and sorry for dropping the ball on this. It seems strange that worked with that typo, I'll try to build it locally again and run it.

[unverbraucht]

@deviantintegral there was a fix merged into RTL-433 and I updated the README for this PR.
I tested the add-on locally and it finds devices with a custom topic suffix, so I think this is good for merging.

[deviantintegral]

Mind linking to the fix?

I’m wondering if we need to document that this only works in the “next” addon, until rtl_433 creates a new release tag.

[unverbraucht]

The fix is here: <merbanan/rtl_433#2593> And then another PR was merged that fixes a deprecation and upcoming breakage in HA: <merbanan/rtl_433#2594> That's sounds good. I'm travelling currently but should have access to my GH account later today or tomorrow, I'll look into the changes.

…

On Mo, Aug 14 2023 at 14:18:22 -07:00:00, Andrew Berry ***@***.***> wrote: Mind linking to the fix? I’m wondering if we need to document that this only works in the “next” addon, until rtl_433 creates a new release tag. — Reply to this email directly, view it on GitHub <#144 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAOKAX6BZBP76Z7XDO6IANTXVKIZ5ANCNFSM6AAAAAAWORZD2E>. You are receiving this because you authored the thread.Message ID: ***@***.***>

[unverbraucht]

I’m wondering if we need to document that this only works in the “next” addon, until rtl_433 creates a new release tag.

@deviantintegral the README already mentions that this config flag only works in next.

[deviantintegral]

I tested this and it works as advertised. It's too bad more devices don't implement a stable ID. My WH51 soil moisture sensor has a random ID that it transmits that's printed on the label. Thinking through this, I think there's a chance that some users will get unpredictable data with this type of configuration due to multiple sensors transmitting on the same channel. But, what else can you do?

Thanks for all of the updates!

[deviantintegral]

FYI this is merged but not working. For some reason, when running in GitHub Actions wget is returning a stale version of the upstream script. I get the correct version on my local, so I think this might be an issue with the Fastly CDN layer. We're likely hitting different CDN POPs. I found https://github.com/orgs/community/discussions/46691#discussioncomment-4953226 confirming this was once an issue, but it's several months old so it may be fixed.

I will likely change this to be a git clone instead of a wget to sidestep the issue entirely.

[SiGmAX666]

FYI this is merged but not working. For some reason, when running in GitHub Actions wget is returning a stale version of the upstream script. I get the correct version on my local, so I think this might be an issue with the Fastly CDN layer. We're likely hitting different CDN POPs. I found https://github.com/orgs/community/discussions/46691#discussioncomment-4953226 confirming this was once an issue, but it's several months old so it may be fixed.

I will likely change this to be a git clone instead of a wget to sidestep the issue entirely.

I probably should make a new issue, but I'm going to just comment here for now: is the suffix functionality working currently, in either 0.7.1-merge or next? I am getting results that make me think it's broken in the rtl_433_mqtt_hass.py script, and then when comparing the script in the next container here vs what is in merbanan/rtl_433:master it's quite a bit different, too.

After spending a -lot- of hours screwing with it, I just modified the script to make it work for me, but...I must be missing something!

[deviantintegral]

This should be working in the stable release 0.7.0. If not, please open a new issue. Thanks!