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

drivers/led: add LEDX_IS_PRESENT defines #20637

Merged
merged 1 commit into from
Apr 30, 2024

Conversation

krzysztof-cabaj
Copy link
Contributor

Contribution description

Currently LEDX_ON, LEDX_OFF, etc. are defined in the board.h (or board_common.h or similar) files. However, various boards could have from 0 to 8 internal LEDs and to write portable code each call of LEDX_... should be protected by #ifdef LEDX_... . Including led.h header file resolve this issue and defines empty LEDX_... if LED is not present for given board. However, after including led.h there is no possibility to check if given board, really posses particular LED.

This PR adds LEDX_IS_PRESENT define for each presented LED at given board.

Testing procedure

This code snippet:

#if defined LED0_IS_PRESENT
 printf("LED0_IS_PRESENT defined.\n");
#endif

#if defined LED1_IS_PRESENT
 printf("LED1_IS_PRESENT defined.\n");
#endif

#if defined LED2_IS_PRESENT
 printf("LED2_IS_PRESENT defined.\n");
#endif

Should show real number of first three LEDs, for example:

LED0_IS_PRESENT defined.
LED1_IS_PRESENT defined.

in native board.

Issues/PRs references

None

@github-actions github-actions bot added the Area: drivers Area: Device drivers label Apr 29, 2024
Copy link
Member

@maribu maribu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, but the CI will insist on the documentation.

I do agree that the name is self-documenting, but the CI doesn't have this intuition and will insistent anyway.

drivers/include/led.h Outdated Show resolved Hide resolved
@maribu maribu enabled auto-merge April 29, 2024 20:28
@maribu maribu disabled auto-merge April 29, 2024 20:28
@maribu
Copy link
Member

maribu commented Apr 29, 2024

Would you mind to squash the two commits into one?

@krzysztof-cabaj
Copy link
Contributor Author

Squashed.

@maribu maribu enabled auto-merge April 29, 2024 20:32
@maribu
Copy link
Member

maribu commented Apr 30, 2024

Hmm, automerge didn't seem to work here: There is no job running on Murdock. Let's turn it off and on again.

@maribu maribu disabled auto-merge April 30, 2024 05:09
@maribu maribu enabled auto-merge April 30, 2024 05:09
@maribu
Copy link
Member

maribu commented Apr 30, 2024

Ahh, my fault 😅

@maribu maribu added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Apr 30, 2024
@riot-ci
Copy link

riot-ci commented Apr 30, 2024

Murdock results

✔️ PASSED

a813e97 drivers/led: add LEDX_IS_PRESENT defines

Success Failures Total Runtime
10065 0 10066 14m:16s

Artifacts

@maribu maribu added this pull request to the merge queue Apr 30, 2024
Merged via the queue into RIOT-OS:master with commit ee8569f Apr 30, 2024
26 checks passed
@maribu
Copy link
Member

maribu commented Apr 30, 2024

Thx for your contribution 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: drivers Area: Device drivers CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants