-
Notifications
You must be signed in to change notification settings - Fork 2k
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
examples/leds_shell: add example for interactive LEDs/GPIO control #20782
Conversation
78914f5
to
deef944
Compare
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.
Nice idea, just had a brief look at it for now.
Although this is somehow opposed to your intended use-case, aren't there some default shell commands already for this kind of GPIO/LED control?
Maybe we can come up with a better name ? I find it a bit disturbing to have a |
deef944
to
489beaf
Compare
What do you think about |
During my few years with RIOT I did not find any such commands. From similar programs, I only knows |
2d2a5ee
to
d873bd3
Compare
f3af37c
to
fadfe6d
Compare
As @dylad suggested I change the name of application to The code is refactored using inline functions added by @mguetschow in the PR #20783. |
db70901
to
ef0ec52
Compare
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.
Thanks for adapting your code, looks much cleaner now! I've got comments mostly on the Makefile and README below.
4075352
to
8d9af20
Compare
@mguetschow, @dylad thanks for detailed review. I hope all issues are now resolved. |
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.
LGTM now, thanks again!
@mguetschow thanks for supporting during this PR. Can we add PR to the merge queue? |
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.
One last nitpick on my side, feel free to squash directly.
8d9af20
to
1df9170
Compare
1df9170
to
10aac55
Compare
Hmmm ... If I understand CI output well I add Update: Maybe labels |
10aac55
to
9959e62
Compare
@krzysztof-cabaj you should find the required tools for this job locally under |
You may also try |
I run Can we add this PR once again to merge queue? |
Contribution description
Rationale: When I remember my first steps as RIOT newbie, I would like as soon as possible flash the program and
show that it works and could control something, for example LED. However, for RIOT newbie this is not so easy. Of course there is
examples/blink
- but it works automatically. Manual control is something, in my humble opinion, missing.This simple program allows easy control of internal LEDs via RIOT CLI. Additionally, it allows initialization of any GPIO port in output mode and manually set HIGH/LOW state.
From the programming learning perspective, analysis and experiments with this program could teach:
Any comments are welcome.
Testing procedure
Flash
examples/LEDs
and check if number of internal LEDs is properly detected.I tested it with boards from 1 to 4 LEDs, respectively:
nucleo-l496rg
,stm32l47g-disco/native', 'nucleo-f207zg
andstm32f469i-disco
.Read README.md and check if everything is clear.
Issues/PRs references
PR #20637