examples/leds_shell: add example for interactive LEDs/GPIO control by krzysztof-cabaj · Pull Request #20782 · RIOT-OS/RIOT

krzysztof-cabaj · 2024-07-10T10:04:21Z

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:

RIOT CLI,
Adding own functions,
Basic GPIO.

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 and stm32f469i-disco.

Read README.md and check if everything is clear.

Issues/PRs references

PR #20637

mguetschow

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?

examples/LEDs/main.c

riot-ci · 2024-07-10T11:50:11Z

Murdock results

✔️ PASSED

9959e62 examples/leds_shell: code refactoring-one led and gpio commands

Success	Failures	Total	Runtime
18	0	19	01m:21s

Artifacts

Documentation preview

dylad · 2024-07-10T12:11:01Z

Maybe we can come up with a better name ? I find it a bit disturbing to have a tests/leds and examples/LEDs.

krzysztof-cabaj · 2024-07-10T12:34:57Z

Maybe we can come up with a better name ? I find it a bit disturbing to have a tests/leds and examples/LEDs.

What do you think about examples/leds_cli or examples/cli_leds?

krzysztof-cabaj · 2024-07-10T12:44:25Z

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?

During my few years with RIOT I did not find any such commands. From similar programs, I only knows examples/blinky and mentioned by @dylad tests/leds but they lacks interactivity.

krzysztof-cabaj · 2024-07-12T21:02:57Z

As @dylad suggested I change the name of application to leds_shell - I hope that current name will be clear for everybody.

The code is refactored using inline functions added by @mguetschow in the PR #20783.
The code is know simpler and looks better.

mguetschow

Thanks for adapting your code, looks much cleaner now! I've got comments mostly on the Makefile and README below.

examples/leds_shell/Makefile

examples/leds_shell/README.md

examples/leds_shell/main.c

examples/leds_shell/README.md

examples/leds_shell/main.c

krzysztof-cabaj · 2024-07-16T10:25:12Z

@mguetschow, @dylad thanks for detailed review.

I hope all issues are now resolved.

mguetschow

LGTM now, thanks again!

krzysztof-cabaj · 2024-07-17T07:27:03Z

@mguetschow thanks for supporting during this PR.

Can we add PR to the merge queue?

dylad

One last nitpick on my side, feel free to squash directly.

examples/leds_shell/main.c

krzysztof-cabaj · 2024-07-17T18:18:54Z

Hmmm ... If I understand CI output well examples/leds_shell did not fit to atmega8 flash.

I add Makefile.ci, but I suspect that more boards with small amount of flash could rise this same error.
Is in RIOT a tool which could I use on my machine to check this automatically? Or can we set some CI flag to not stop after first error?

Update: Maybe labels CI: full build and CI: no fast fail could be used to solve this problem?

dylad · 2024-07-18T06:03:26Z

@krzysztof-cabaj you should find the required tools for this job locally under dist/tools/insufficient_memory

dylad · 2024-07-18T07:36:23Z

You may also try dist/tools/compile_and_test_for_board/compile_and_test_for_board.py for your board, but to run compile only (no test) for your board. It will try to compile your board for all available examples and tests. A list of all failed compilation should be printed at the end.

krzysztof-cabaj · 2024-07-18T09:34:58Z

I run dist/tools/insufficient_memory/create_makefile.ci.sh and only atmega8 did not fit to flash.

Can we add this PR once again to merge queue?

krzysztof-cabaj requested a review from jia200x as a code owner July 10, 2024 10:04

github-actions bot added Area: doc Area: Documentation Area: examples Area: Example Applications labels Jul 10, 2024

krzysztof-cabaj force-pushed the example-LEDs branch from 78914f5 to deef944 Compare July 10, 2024 10:11

mguetschow reviewed Jul 10, 2024

View reviewed changes

examples/LEDs/main.c Outdated Show resolved Hide resolved

mguetschow added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jul 10, 2024

krzysztof-cabaj force-pushed the example-LEDs branch from deef944 to 489beaf Compare July 10, 2024 12:30

mguetschow mentioned this pull request Jul 11, 2024

drivers/led: add LED_NUMOF and convenience inline functions #20783

Merged

krzysztof-cabaj force-pushed the example-LEDs branch from 2d2a5ee to d873bd3 Compare July 12, 2024 16:16

krzysztof-cabaj changed the title ~~examples/LEDs: add example for interactive LEDs/GPIO control~~ examples/leds_shell: add example for interactive LEDs/GPIO control Jul 12, 2024

krzysztof-cabaj added 3 commits July 12, 2024 22:41

examples/LEDs: addition of new example appplication

1113be9

examples/LEDs: add README.md file

9aea4bd

examples/leds_shell: change application name

4273f97

krzysztof-cabaj force-pushed the example-LEDs branch 3 times, most recently from f3af37c to fadfe6d Compare July 12, 2024 20:56

krzysztof-cabaj force-pushed the example-LEDs branch 2 times, most recently from db70901 to ef0ec52 Compare July 12, 2024 21:15

mguetschow requested changes Jul 15, 2024

View reviewed changes

dylad reviewed Jul 15, 2024

View reviewed changes

examples/leds_shell/README.md Outdated Show resolved Hide resolved

dylad reviewed Jul 15, 2024

View reviewed changes

examples/leds_shell/main.c Outdated Show resolved Hide resolved

krzysztof-cabaj force-pushed the example-LEDs branch 2 times, most recently from 4075352 to 8d9af20 Compare July 16, 2024 10:24

mguetschow approved these changes Jul 16, 2024

View reviewed changes

examples/leds_shell: code refactoring-one led and gpio commands

9959e62

dylad reviewed Jul 17, 2024

View reviewed changes

examples/leds_shell/main.c Show resolved Hide resolved

krzysztof-cabaj force-pushed the example-LEDs branch from 8d9af20 to 1df9170 Compare July 17, 2024 08:00

dylad added this pull request to the merge queue Jul 17, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 17, 2024

krzysztof-cabaj force-pushed the example-LEDs branch from 1df9170 to 10aac55 Compare July 17, 2024 18:13

krzysztof-cabaj force-pushed the example-LEDs branch from 10aac55 to 9959e62 Compare July 17, 2024 18:30

mguetschow added this pull request to the merge queue Jul 18, 2024

Merged via the queue into RIOT-OS:master with commit 4612cc2 Jul 18, 2024

benpicco added this to the Release 2024.10 milestone Nov 27, 2024

Conversation

krzysztof-cabaj commented Jul 10, 2024

Contribution description

Testing procedure

Issues/PRs references

Uh oh!

mguetschow left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

riot-ci commented Jul 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Murdock results

Artifacts

Uh oh!

dylad commented Jul 10, 2024

Uh oh!

krzysztof-cabaj commented Jul 10, 2024

Uh oh!

krzysztof-cabaj commented Jul 10, 2024

Uh oh!

krzysztof-cabaj commented Jul 12, 2024

Uh oh!

mguetschow left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

krzysztof-cabaj commented Jul 16, 2024

Uh oh!

mguetschow left a comment

Choose a reason for hiding this comment

Uh oh!

krzysztof-cabaj commented Jul 17, 2024

Uh oh!

dylad left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

krzysztof-cabaj commented Jul 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dylad commented Jul 18, 2024

Uh oh!

dylad commented Jul 18, 2024

Uh oh!

krzysztof-cabaj commented Jul 18, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

riot-ci commented Jul 10, 2024 •

edited

Loading

krzysztof-cabaj commented Jul 17, 2024 •

edited

Loading