Remove many duplicate samples for temperature sensors, add unified sample #35854
Conversation
nashif
force-pushed
the
sensor_cleanup
branch
2 times, most recently
from
ce705e1 to
6dcb414
Compare
because this is a sample and should be as close as possible to how sensors will be used, this is not a test to only exercise sensor code. Sample start to be useful when they integrate multiple features and subsystems. Also, you can drop the display completely using the configs and just get console output if you wish. |
I like this idea. I had same doubt infact. |
|
The approach seems good to me. I guess it is currently not complete, as for example the triggers are not enabled for all sensors. Moreover I wonder if in documentation we should also list the complete set of boards/shields for a given sensor (e.g. for HTS221 it is just listed disco_l475_iot1, but you can also obtain it from a shield already supported in zephyr, which might makes things too complicated though) |
| void main(void) | ||
| { | ||
|
|
||
| #if defined(CONFIG_HTS221) |
There was a problem hiding this comment.
+1 to remove all the duplicate samples but this PR just shift the issue to the one big sample, look at this #elfi tree below, this will continue to grow. The same for all the "config SAMPLE_SENSOR_*.
There was a problem hiding this comment.
this code was already removed in the latest patch and simplified even more. Still trying to find a more flexible solution with Kconfigs and DTS, but this is a price we have to pay to make this work regardless of board and sensor, we just need to make it suck less. Open for suggestions.
There was a problem hiding this comment.
Let us use labels and device name, and prefix it with device type, like you did it for I2C shell, or as I suggested in #21709, 😄 duck and run away
yes, still need to to do the triggers. |
Opt for a unified sample for temperature sensors and remove this one as it duplicated the same functionality with limited hardware support. Signed-off-by: Anas Nashif <anas.nashif@intel.com>
Opt for a unified sample for temperature sensors and remove this one as it duplicated the same functionality with limited hardware support. Signed-off-by: Anas Nashif <anas.nashif@intel.com>
Opt for a unified sample for temperature sensors and remove this one as it duplicated the same functionality with limited hardware support. Signed-off-by: Anas Nashif <anas.nashif@intel.com>
Opt for a unified sample for temperature sensors and remove this one as it duplicated the same functionality with limited hardware support. Signed-off-by: Anas Nashif <anas.nashif@intel.com>
Opt for a unified sample for temperature sensors and remove this one as it duplicated the same functionality with limited hardware support. Signed-off-by: Anas Nashif <anas.nashif@intel.com>
Opt for a unified sample for temperature sensors and remove this one as it duplicated the same functionality with limited hardware support. Signed-off-by: Anas Nashif <anas.nashif@intel.com>
Opt for a unified sample for temperature sensors and remove this one as it duplicated the same functionality with limited hardware support. Signed-off-by: Anas Nashif <anas.nashif@intel.com>
Opt for a unified sample for temperature sensors and remove this one as it duplicated the same functionality with limited hardware support. Signed-off-by: Anas Nashif <anas.nashif@intel.com>
Opt for a unified sample for temperature sensors and remove this one as it duplicated the same functionality with limited hardware support. Signed-off-by: Anas Nashif <anas.nashif@intel.com>
Opt for a unified sample for temperature sensors and remove this one as it duplicated the same functionality with limited hardware support. Signed-off-by: Anas Nashif <anas.nashif@intel.com>
Opt for a unified sample for temperature sensors and remove this one as it duplicated the same functionality with limited hardware support. Signed-off-by: Anas Nashif <anas.nashif@intel.com>
Merge all sensor samples into one. Signed-off-by: Anas Nashif <anas.nashif@intel.com>
Use NODELABEL and a unified label for all sensors set in overlays, this way we just address the temperature/humidity sensor using env_sensor. Learned that today in Marti's device tree talk :-) Signed-off-by: Anas Nashif <anas.nashif@intel.com>
|
I'm really against this approach (mixing display and sensor code in a single sample). I won't block the PR. |
|
This just says " Comprehensive sample app and tutorial." I have no idea how that applies to this PR.
In my view, that means a bad decision was previously localized to just a few samples, and now will affect the only sample for a large variety of drivers. This is worse than what we had before in my opinion.
I think a small, self-contained sample is best. So a sample for sensor with console only in some directory sounds great. I really appreciate the effort towards that. But then I think the better idea would be a separate sample for displays. Mixing drivers like this in a single sample makes it harder for application developers to copy/paste just what they need for the driver they're trying to get working. But like I said, I'm not blocking this PR. |
|
This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time. |
|
This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time. |
|
This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time. |
The above are so far the samples I removed, more to come. The goal is to have 1 samples for all those temp/humidity/pressure/gas samples and have them all in one single application that support those sensors that had samples already but also support sensors that never had a way to verify and test them.
I also took the opportunity to make this all work with various displays, one being the grove lcd and then also using some of the TFT shields using LVGL. Still work in progress and more sensors to integrate, also trying to find a decent way to support additional sensors without adding more ifdef`ry.
Right now you can build this with display support like this:
or you can skip display completely and just get something on the screen: