drivers/led: add LED_NUMOF and convenience inline functions#20783
drivers/led: add LED_NUMOF and convenience inline functions#20783benpicco merged 3 commits intoRIOT-OS:masterfrom
Conversation
mguetschow
requested review from
MichelRottleuthner,
aabadie and
leandrolanzieri
as code owners
github-actions
bot
added
Area: tests
mguetschow
added
the
CI: ready for build
drivers/led: add LED_NUMOF and convenience inline functions
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
mguetschow
added
CI: full build
mguetschow
force-pushed
the
led-functions
branch
from
0ca0ab5 to
5403ba5
Compare
| #define BOARD_H | ||
|
|
||
| #include "cpu.h" | ||
| #include "periph/gpio.h" |
There was a problem hiding this comment.
Can we add this to led.h instead to avoid changing all board definitions?
There was a problem hiding this comment.
Well we could, but if we want to follow the include-what-you-use paradigm (see #20570), it doesn't make sense to add it to led.h. That file does not use the content of periph/gpio.h, while the board.h files indeed do (e.g., GPIO_PIN).
There was a problem hiding this comment.
For example, running make -C examples/hello-world BOARD=adafruit-grand-central-m4-express all with the following diff also fails on master due to the missing include in board.h, completely independent of led.h:
diff --git a/examples/hello-world/main.c b/examples/hello-world/main.c
index 213128ac64..a34fcc00bf 100644
--- a/examples/hello-world/main.c
+++ b/examples/hello-world/main.c
@@ -21,6 +21,8 @@
#include <stdio.h>
+#include "board.h"
+
int main(void)
{
puts("Hello World!");
@@ -28,5 +30,7 @@ int main(void)
printf("You are running RIOT on a(n) %s board.\n", RIOT_BOARD);
printf("This board features a(n) %s CPU.\n", RIOT_CPU);
+ printf("board.h defined PIN: %d\n", WS281X_PARAM_PIN);
+
return 0;
}This now only gets apparent because the macros are used in the inline functions.
But anyway, I feel like the board.h include structure could benefit from a bigger overhaul in any case, so I'm not too strictly opposed to put the include in led.h instead for now. What do you think?
mguetschow
force-pushed
the
led-functions
branch
from
5403ba5 to
bb97445
Compare
3 participants
Contribution description
While reviewing #20782, I found some expected convenience functions for simple LED usage to be missing from "led.h". This PR adds the following:
LED_NUMOFthat provides the number of available LEDs at compile-timeled_{on,off,toggle}with the LED id as run-time argument, mirroring theLED_{ON,OFF,TOGGLE}compile-time macrosAlso adapts
tests/ledsto (partly) use the newly provided functionality.Testing procedure
make -C tests/ledsshould still work with the board of your choice.Issues/PRs references
Could enable an easier application code for examples like #20782