search

pbkhrv / rtl_433-hass-addons

Collection of Home Assistant add-ons that use rtl_433
217 stars 102 forks source link

Fix "-T option null" issue #162

Closed unverbraucht closed 8 months ago

unverbraucht commented 8 months ago

Summary

When #144 was merged I missed a check to see if device_topic_suffix is null. This causes issues #155 and #159 . This PR adds a simple config check and fixes the issue for me when I don't add this argument.

Alternatives Considered

There are other PRs that also fix this issue but this is a simple one-liner. I also ported the changes to config.json from the next folder over to the main autodiscovery config.json so these options work once next is merged.

Testing Steps

  1. Run next without setting a value for device_topic_suffix
  2. Add-On should start up
catduckgnaf commented 8 months ago

Nah, they want to wait months for "upstream" to fix it instead. mine was a one liner too.

I decided to spend effort on actually making an integration that people want.

unverbraucht commented 8 months ago

Nah, they want to wait months for "upstream" to fix it instead. mine was a one liner too.

I decided to spend effort on actually making an integration that people want.

This has not been my experience with this project.

Since I was the person introducing the bug (sorry about that) I think it's fair that I fix it :)

unverbraucht commented 8 months ago

@catduckgnaf since you also fixed the issue in your branch (along many improvements, cudos!) could you review this PR?

catduckgnaf commented 8 months ago

I can't approve changes here. :( Thanks for the "new" motivation to get my but into gear with the integration. And for the compliment!

I will have 2 repos, one for the "integration" and one for the add-on.

I am waiting to see if the current maintainers want me to take over the add-on....

benklop commented 8 months ago

This is actually caused by the path to the image being incorrect: https://github.com/pbkhrv/rtl_433-hass-addons/pull/164 is probably the right fix

deviantintegral commented 8 months ago

Agreed with going with #164. Closing this one for now, but if someone thinks we'll need both changes then feel free to reopen.

unverbraucht commented 8 months ago

Absolutely fine with the other fix if it fixes things :)