[RFC] introduce common periph led driver#7321
Conversation
smlng
added
Type: cleanup
|
What is the reasoning for the change? What is the overall size benefit/drawback? |
kYc0o
left a comment
kYc0o
left a comment
There was a problem hiding this comment.
I'd also like to have a "easy to use" LED interface, especially for educational purposes this is very useful, however I would like to know, as Martine pointed out, the benefits on code size of this approach.
| #define LED7_INIT do { gpio_init(LED7_PIN, GPIO_OUT); LED7_OFF; } while(0) | ||
| #endif /* LED7_INIT */ | ||
| #endif /* LED7_PIN */ | ||
| /** @} */ |
There was a problem hiding this comment.
Maybe instead of creating a define per "possibly" LEDs present in a board (what if I have 8 LEDs? This won't work), you can propose a definition similar to UART, SPI or (future) I2C, on which you have a general structure and is the board which defines how many "objects" of this kind it has, this way you'll avoid the need to define everything possible.
However, I doubt if such a solution would have a negative impact on code size.
There was a problem hiding this comment.
I thought about this, too ... will try this later on. For now this PR is mostly about avoiding redefinition per board for common function such as LED[0-7]_ON
There was a problem hiding this comment.
btw. 8 LEDs would still work (0-7, it is). However, supporting arbitrary numbers of LEDs is difficult while using macros
drivers/periph_common/led.c
Outdated
| #ifdef LED7_PIN | ||
| LED7_INIT; | ||
| #endif /* LED7_PIN */ | ||
|
|
There was a problem hiding this comment.
Here too, same comment as above.
|
the reasoning behind this is that currently each board redefines macros |
|
btw. @miri64 regarding code size, using master this PR |
|
Why not just use the normal GPIO API and create macros as aliases for the GPIO pins? |
|
@gebart that's already and still possible - though not with |
|
and as I already said, primary goal of this PR is to remove redundant redefinition of LED macros and that way also reduce stuff defined in |
smlng
force-pushed
the
rfc/drivers/periph/led
branch
from
a5fef44 to
7015eb6
Compare
|
I ported this to Still I think its worth discussing and reconsidering current handling of on-board leds - but not the way I proposed here, I'll try something different, too. Will close this soonish as memo, just keep it open if someone wants to had her/his 2 cents |
|
I agree that we should revisit this topic, its been a while since we had this on our list (hello @OlegHahm :-) ). To recab: the reason for having the LED macros was/is simply to be able to control selected pins (i.e. those that have LEDs connected to them) with the least amount of overhead, so they can used for timing analysis. I personally don't think this use is not very pressing (anymore), as most timing analysis are carried out using hardware timers and/or other profiling solutions. Also I think the usefulness of the LED macros for debugging has quite degraded over time, as we have access to decent debuggers on most platforms, that make the use of LED macros for debugging obsolete on these. Bottom line: I think we can deprecate the existing LED macros and move to something easier maintainable. I do however not like the proposed solution:
|
|
@haukepetersen your #7350 is basically what I had in mind with:
so, now its really time to close this and move the discussion to your PR ... |
smlng
removed
the
State: WIP
5 participants
this PR introduces a simplified handling of on-board LEDs by fully utilizing RIOTs hardware independent gpio driver. That way per board redefinition of led macros such as
LEDX_ON,LEDX_OFF, andLEDX_TOGGLEwill be obsolete, it simple requires to define the respective gpio pin for each LED as it is already done.As an example this PR shows this feature on board
remote-revb. However, this is foremost a Request-For-Comments and hence Work-In-Progress.