-
Notifications
You must be signed in to change notification settings - Fork 7.3k
drivers: input: vs1838b: Add support for VS1838B #88661
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
drivers: input: vs1838b: Add support for VS1838B #88661
Conversation
Hello @bastienjauny, and thank you very much for your first pull request to the Zephyr project! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 2 out of 5 changed files in this pull request and generated 1 comment.
Files not reviewed (3)
- drivers/input/CMakeLists.txt: Language not supported
- drivers/input/Kconfig: Language not supported
- drivers/input/Kconfig.vs1838b: Language not supported
byte += (1 << i); | ||
} else { | ||
LOG_WRN("Failed to identify detected bit at position %u", i); | ||
return false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Returning 'false' in a function that returns a uint8_t is ambiguous because 0 is a valid byte value. Consider refactoring the function to use an out parameter or a dedicated error code to clearly distinguish between a legitimate 0 value and an error condition.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All done.
I'm currently reviewing the needed changes (mainly inquiring with my company about the copyright notice). I'll update the PR when the changes are ready. |
2d400cd
to
fadca6f
Compare
Thanks @fabiobaltieri I was just looking at these. I just ran |
yeah you need to specify a commit range otherwise it only does the last commit, something like |
4bcba9a
to
34b18a7
Compare
drivers/input/input_vs1838b.c
Outdated
return false; | ||
} | ||
if (!detect_last_burst(edges_ticks)) { | ||
LOG_DBG("No command decoded"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
copy paste error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, just pushed the fix.
The VS1838B is one of the most found infrared receiver found in electronic kits and is easy to setup with only a single GPIO used for signal transmission (apart from VCC and GND). This new driver let applications use the VS1838B as an input with events relayed as 0x0000<address><command>. Only the NEC protocol is supported in this version but more can be added later. Link: https://github-wiki-see.page/m/CoreELEC/remotes/wiki/08.-NEC-IR-Protocol-Datasheet This has been tested using the input_dump sample. Signed-off-by: Bastien JAUNY <[email protected]>
34b18a7
to
60bd610
Compare
Hi @bastienjauny! To celebrate this milestone and showcase your contribution, we'd love to award you the Zephyr Technical Contributor badge. If you're interested, please claim your badge by filling out this form: Claim Your Zephyr Badge. Thank you for your valuable input, and we look forward to seeing more of your contributions in the future! 🪁 |
Awesome, many thanks! |
No, thank you! |
The VS1838B is one of the most found infrared receiver in electronic kits and is easy to setup with only a single GPIO used for signal transmission (apart from VCC and GND).
This new driver let applications use the VS1838B as an input with events relayed as
0x0000<address><command>
.Only the NEC protocol is supported in this version but more can be added later.
Link
This has been tested using the input_dump sample.