samples: led: consolidate LED samples under same directory #77840
Conversation
zephyrbot
added
area: Samples
zephyrbot
requested review from
avisconti,
erwango,
gmarull,
jfischer-no,
MaureenHelm,
nashif and
stephanosio
kartben
force-pushed
the
move_led_samples
branch
from
e8da175
to
784c4b6
Compare
|
I'd like to see a |
Ya, good catch, but different topic :) |
There was a problem hiding this comment.
Thanks for this patch @kartben !
Please see below for some comments and questions.
| @@ -68,7 +68,7 @@ modified by changing the :kconfig:option:`CONFIG_SAMPLE_LED_UPDATE_DELAY`. | |||
| Then build and flash the application: | |||
|
|
|||
| .. zephyr-app-commands:: | |||
| :zephyr-app: samples/drivers/led_strip | |||
| :zephyr-app: samples/drivers/led/led_strip | |||
There was a problem hiding this comment.
Don't you think we should put led_strip at the same level than led ?
samples/drivers/led
samples/drivers/led_strip
After all led_strip is a distinct subsystem, with its own API. Also, this would mirror the drivers tree:
drivers/led
drivers/led_strip
There was a problem hiding this comment.
Maybe, yes. My reasoning was based on how things are presented in: https://docs.zephyrproject.org/latest/hardware/peripherals/led.html , i.e. everything under LED.
There was a problem hiding this comment.
I am happy to do a follow-up PR but hopefully this is good enough for a first pass at cleaning things up.
I just rebased to address a conflict in redirects.py
There was a problem hiding this comment.
Would vote for led_strip being at the same level as led not inside of it, they are different drivers
There was a problem hiding this comment.
And eventually it is the opportunity to remove some of the led_ prefixes now that we have a leading led directory ?
For example:
samples/drivers/led/led_is31fl3194 -> samples/drivers/led/is31fl3194
It would be nice to have the same name for a sample and its drivers.
There was a problem hiding this comment.
Or should we keep the led_ prefix when it is used in the driver name ?
drivers/led/led_pwm -> samples/drivers/led/led_pwm
drivers/led/is31fl3194 -> samples/drivers/led/is31fl3194
There is maybe some value to allow a script to retrieve the sample name from the exact driver name ? Or am I overthinking this ? :)
There was a problem hiding this comment.
define "driver_name" :) I would hope that the name of the C file implementing a particular driver is mostly irrelevant. The names of the source files are a bit all over the place, and I am not sure they go through as much scrutiny as the compatibles do.
If anything, the samples could maybe be named after the compatibles, but this is probably overthinking (at this point at least), yes :)
There was a problem hiding this comment.
Note that some drivers and samples support several compatibles. We have the lp50xx LED driver in that case. This doesn't help for a generic naming...
kartben
force-pushed
the
move_led_samples
branch
from
784c4b6
to
86619ac
Compare
With over a dozen LED samples, grouping them in a common drivers/led/ folder helps keep things tidy and lays the groundwork for further improvements in the way samples are showing up in the docs. Signed-off-by: Benjamin Cabé <[email protected]>
kartben
force-pushed
the
move_led_samples
branch
from
86619ac
to
bb91720
Compare
With over a dozen LED samples, grouping them in a common
samples/drivers/led/folder helps keep things tidy and lays the groundwork for further improvements in the way samples are showing up in the docs.