-
Notifications
You must be signed in to change notification settings - Fork 6.9k
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: sensors: sensor_shell: fix hang & crash #72815
Conversation
EDIT: this seems to be intended after #61022 The output of the Can be reproduced in this PR with:
I was hoping that I can do a for-loop in the pytest script to test the whole range of channel & attribute |
22f94cb
to
091f2b6
Compare
c046708
to
7c831b0
Compare
7c831b0
to
0fe04ef
Compare
Waiting for #72833 to be merged first, then this PR can update the pytest there to validate the fix. |
0fe04ef
to
fadbdec
Compare
fadbdec
to
d0a2d64
Compare
d0a2d64
to
609dcae
Compare
609dcae
to
5edeefc
Compare
278f524
to
95649ef
Compare
95649ef
to
c984c79
Compare
cc @teburd @MaureenHelm @ubieda this should be ready, PTAL, thanks |
As new channels are added to the `enum sensor_channel`, some of the newer channel aren't updated in the whitelist of rtio decoder. Instead of specifying every channel in the list, do: 1. Verify that the `channel` is valid 2. cherry-pick the channels that require special handling, i.e. 1. `three_axis_data` 2. `byte_data` 3. `uint64_data` 3. handle the remaining `channel` in the default case as `q31_data` to make sure that all channels are handled. Updated the pytest to get channel 32, previously nothing would happen for this channel as there isn't a decoder for it, now it would return: ``` channel type=32((null)) ``` the channel name is NULL because it wasn't added to the channel name look up table in the sensor_shell.c, that is being fixed in zephyrproject-rtos#72815. Signed-off-by: Yong Cong Sin <ycsin@meta.com>
As new channels are added to the `enum sensor_channel`, some of the newer channel aren't updated in the whitelist of rtio decoder. Instead of specifying every channel in the list, do: 1. Verify that the `channel` is valid 2. cherry-pick the channels that require special handling, i.e. 1. `three_axis_data` 2. `byte_data` 3. `uint64_data` 3. handle the remaining `channel` in the default case as `q31_data` to make sure that all channels are handled. Updated the pytest to get channel 32, previously nothing would happen for this channel as there isn't a decoder for it, now it would return: ``` channel type=32((null)) ``` the channel name is NULL because it wasn't added to the channel name look up table in the sensor_shell.c, that is being fixed in #72815. Signed-off-by: Yong Cong Sin <ycsin@meta.com>
The `SENSOR_CHAN_VSHUNT` was added in zephyrproject-rtos#60717 but was never added to the `sensor_channel_name[SENSOR_CHAN_COMMON_COUNT]` table. Since the length of `sensor_channel_name` is fixed to `SENSOR_CHAN_COMMON_COUNT`, this means that the index at `SENSOR_CHAN_VSHUNT` points to `NULL`. When we use the `sensor get` command for anything bigger than `SENSOR_CHAN_VSHUNT`, we will deref that `NULL` pointer when we do `strcmp` in the for-loop of `parse_named_int`. Fix this by defining `SENSOR_CHAN_VSHUNT` in the table. Signed-off-by: Yong Cong Sin <ycsin@meta.com>
If `CONFIG_SENSOR_INFO` is enabled, use the `sensor_info` section to validate that the argument is a sensor before using, otherwise the shell command will hang the application. Signed-off-by: Yong Cong Sin <ycsin@meta.com>
Update the pytest script to exercise the entire `sensor_channel_name` table with `parse_named_int()`. The `gauge_desired_charging_current` is selected because it is the last one in the table before `all`, `all` is not used because `sensor get sensor@0 all` doesn't return anything. Signed-off-by: Yong Cong Sin <ycsin@meta.com>
Instead of specifying 3 axis channels manually, use `SENSOR_CHANNEL_3_AXIS` instead. Signed-off-by: Yong Cong Sin <ycsin@meta.com>
c984c79
to
5ae7f1b
Compare
Rebased to fix merge conflict, added:
Before sensors: add new channel
After:
|
Add new channel: `SENSOR_CHAN_POS_DXYZ`, so that it is consistent with other 3-axis channels. Updated pytest, `sensor_shell` & `fake_sensor` accordingly. Signed-off-by: Yong Cong Sin <ycsin@meta.com>
5ae7f1b
to
fd53bf7
Compare
Doc build is probably broken, the error msg is consistent in a few PRs |
As new channels are added to the `enum sensor_channel`, some of the newer channel aren't updated in the whitelist of rtio decoder. Instead of specifying every channel in the list, do: 1. Verify that the `channel` is valid 2. cherry-pick the channels that require special handling, i.e. 1. `three_axis_data` 2. `byte_data` 3. `uint64_data` 3. handle the remaining `channel` in the default case as `q31_data` to make sure that all channels are handled. Updated the pytest to get channel 32, previously nothing would happen for this channel as there isn't a decoder for it, now it would return: ``` channel type=32((null)) ``` the channel name is NULL because it wasn't added to the channel name look up table in the sensor_shell.c, that is being fixed in zephyrproject-rtos#72815. Signed-off-by: Yong Cong Sin <ycsin@meta.com>
Currently, getting any channel >=
SENSOR_CHAN_VSHUNT
would result in aNULL
pointer deref:and using a non-sensor device in the command will cause a hang:
This PR fixes that:
Fixes #72838