Skip to content
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

ISM330DHCX driver: wrong FS bitmask for gyroscope calculated #72617

Open
DeMurlot opened this issue May 11, 2024 · 2 comments
Open

ISM330DHCX driver: wrong FS bitmask for gyroscope calculated #72617

DeMurlot opened this issue May 11, 2024 · 2 comments
Assignees
Labels
area: Sensors Sensors bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug

Comments

@DeMurlot
Copy link

DeMurlot commented May 11, 2024

Describe the bug
The driver for the ism330dhcx incorrectly calculates the bitmap for the fs bits in the CTRL2_G register. The driver uses the ST hal to set the values, but hands over an incorrect bitmask leading to wrong configuration.

HAL values for given FS: https://github.com/zephyrproject-rtos/hal_st/blob/b77157f6bc4395e398d90ab02a7d2cbc01ab2ce7/sensor/stmemsc/ism330dhcx_STdC/driver/ism330dhcx_reg.h#L2894
Zephyr driver logic for getting the value from the devicetree: https://github.com/zephyrproject-rtos/zephyr/blob/be682e22e166c33a67c1db0755f8ca20c37b6576/drivers/sensor/st/ism330dhcx/ism330dhcx.c#L70C2-L70C17

in my opinion, the lines in the zephyr driver should be

static const uint16_t ism330dhcx_gyro_fs_map[] = {250, 4000, 125, 4000, 500, 4000, 125, 4000, 1000, 4000, 125, 4000, 2000};
static const uint16_t ism330dhcx_gyro_fs_sens[] = {2, 32, 1, 32, 4, 32, 1, 32, 8, 32, 1, 32, 16};

In addition the devicetree binding should be adapted to allow a FS value of 4000 (dps).

For convenience, here is a screenshot from the datasheet:
image
And the link to how the ST HAL parses this value: https://github.com/zephyrproject-rtos/hal_st/blob/b77157f6bc4395e398d90ab02a7d2cbc01ab2ce7/sensor/stmemsc/ism330dhcx_STdC/driver/ism330dhcx_reg.h#L357

To Reproduce
Not applicable

Expected behavior
The devicetree full-scale property for gyroscope to be correctly applied.

Impact
Unintended FS selection and wrong gyroscope readings, since raw to dps conversion is intended sensitivity but not the actual one.

@DeMurlot DeMurlot added the bug The issue is a bug, or the PR is fixing a bug label May 11, 2024
Copy link

Hi @DeMurlot! We appreciate you submitting your first issue for our open-source project. 🌟

Even though I'm a bot, I can assure you that the whole community is genuinely grateful for your time and effort. 🤖💙

@MaureenHelm
Copy link
Member

@avisconti please take a look

@aescolar aescolar added the priority: low Low impact/importance bug label May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Sensors Sensors bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug
Projects
None yet
Development

No branches or pull requests

6 participants