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

Contribution Question: Adding Multiple Decoders #2754

Closed
eshaz opened this issue Dec 15, 2023 · 6 comments · May be fixed by #2764
Closed

Contribution Question: Adding Multiple Decoders #2754

eshaz opened this issue Dec 15, 2023 · 6 comments · May be fixed by #2764
Labels
question Not a bug, but a question. (Belongs in discussions or mailinglist)

Comments

@eshaz
Copy link
Contributor

eshaz commented Dec 15, 2023

I recently purchased a box of 50 or so old car keyfobs, and I also have a few other devices that I plan on using for various home automation things. I quickly tried these devices with rtl_433, and only a handful of them are implemented. I think it would be nice to contribute back to the community by creating decoders in this library that will support the various protocols implemented by these devices.

What would be the best way for me to contribute these additions? Would separate PRs for each protocol be OK, or is there some other way that would be easier for maintainers to review and merge a bunch of new devices?

@zuckschwerdt
Copy link
Collaborator

Usually this type of sender (doorbell style) sends only a fixed code, likely EV1527 protocol. This is best implemented with flex decoders. See https://github.com/merbanan/rtl_433/tree/master/conf
A combined PR is fine for these. You likely want to match on your senders IDs, a commented example as template is still appreciated.

@klohner
Copy link
Contributor

klohner commented Jan 8, 2024

@eshaz Would you mind posting some .cu8 files of these samples with descriptions here?

I took a look through your audiovox_car_remote.c code in the PR and I'm concerned that without any matching of a product ID code or CRC verification in the samples, it will find itself matching other kinds of devices out in the wild too readily. A flex decoder might be more appropriate.

@eshaz
Copy link
Contributor Author

eshaz commented Jan 9, 2024

@klohner I was thinking it would be a good idea to include sample data and docs for these in the rtl_433_tests repo. I can open a PR there with the samples / docs.

Regarding the Nutek remotes, (I renamed audiovox_car_remote.c to nutek_car_remote.c in a later commit to match the actual manufacturer), I'll go ahead and move this one into a flex decoder. I was getting a few false positives with this as I was adding the other devices.

@eshaz
Copy link
Contributor Author

eshaz commented Jan 9, 2024

I've added some samples / docs / pictures here: merbanan/rtl_433_tests#462 if you want to take a look. This has about half of the devices. I plan on adding the rest tomorrow.

@eshaz
Copy link
Contributor Author

eshaz commented Jan 10, 2024

Everything is added now into the tests PR. Also, I've moved that Nutek remote into a flex decoder.

@gdt
Copy link
Collaborator

gdt commented Jun 3, 2024

Seems like this was a question about process, answered, and some progress. Future contributions welcome, but I don't see this issue being open as helpful to anyone. If I got that wrong please tell me.

@gdt gdt closed this as completed Jun 3, 2024
@gdt gdt added the question Not a bug, but a question. (Belongs in discussions or mailinglist) label Jun 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Not a bug, but a question. (Belongs in discussions or mailinglist)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants