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

Update Dockerfile #160

Closed
wants to merge 1 commit into from
Closed

Update Dockerfile #160

wants to merge 1 commit into from

Conversation

catduckgnaf
Copy link

This is a link to a separate contained discovery script, with more additions. It should work with next and non next.

Summary

Alternatives Considered

Testing Steps

  1. First...
  2. Then...
  3. You should see... (screenshots or console text are helpful)

This is a link to a separate contained discovery script, with more additions. It should work with next and non next.
@deviantintegral
Copy link
Collaborator

I'm a little hesitant to completely diverge the autodiscovery script from the upstream rtl_433 repository and pull in "with more additions" without clarity on what those are. The upstream rtl_433 has many more eyes and overall activity, and I think we benefit from their work. On diffing the current master of your version of the autodiscovery script addon, I didn't see exactly where this is fixed. If this is fixed by changing one of our files, I'd like to just get it committed here. And if it's in the python script, it would be good if we can file an issue or PR upstream.

@Ninja1283
Copy link

Ninja1283 commented Oct 22, 2023

@deviantintegral reviewing against the upstream rtl_433_mqtt_hass.py file, there was some documentation cleanup/clarification and a few minor mapping edits, but the main edits were at line 868 reversing the if-else logic, line 966 adding the discovery_ids , and line 996, removing run().

Agreed that this should be a PR upstream, if they fix the issues.

@catduckgnaf
Copy link
Author

Since "upstream" doesn't care about breaking things for us, and I didn't see the problem with cloning the script, checking changes, and making sure it doesn't break things.

My head hurts looking at their requirements for a PR with such a big project. The script has so many issues most never use it.

Anyway, I'm gonna close my request. I also think its silly to leave a non working add-on in place for months, you could always submit it back when/if they fix it.

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.

None yet

3 participants