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

add Low Power Sensor Fusion (SFLP) support in lsm6dsv16x sensor driver #83094

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

avisconti
Copy link
Collaborator

Add the Sensor Fusion Low Power (SFLP) FIFO streaming capability, using RTIO. The decode function is now aware of parsing following FIFO tags:

    - LSM6DSV16X_SFLP_GAME_ROTATION_VECTOR_TAG
    - LSM6DSV16X_SFLP_GYROSCOPE_BIAS_TAG
    - LSM6DSV16X_SFLP_GRAVITY_VECTOR_TAG

To activate SFLP the user should put in DT the proper configuration. For example, to activate a 60Hz SFLP FIFO batching rate of game rotation vector, gravity vector and gbias components, the user should add in DT the following:

      sflp-odr = <LSM6DSV16X_DT_SFLP_ODR_AT_60Hz>;
      sflp-fifo-enable = <LSM6DSV16X_DT_SFLP_FIFO_GAME_ROTATION_GRAVITY_GBIAS>;

The data are accessed thru three new sensor channels:

    - SENSOR_CHAN_GAME_ROTATION_VECTOR

      The "game rotation vector" reports the four components x/y/z/w
      (quaternions) of the unit vector representing a 3D spatial
      rotation or orientation.

    - SENSOR_CHAN_GRAVITY_VECTOR

      The "gravity vector" reports the three axis x/y/z of the gravity
      vector separated from the linear acceleration components.
      The measurement unit is m/s^2.

    - SENSOR_CHAN_GBIAS_XYZ

      The "gbias" reports the gyroscope offset x/y/z components.
      The measurement unit is radians/s.

Tested on a nucleo_h503rb board plus a x_nucleo_iks4a1 shield, and an improved version of the sample in the #82342

teburd
teburd previously approved these changes Dec 17, 2024
@avisconti avisconti added the DNM This PR should not be merged (Do Not Merge) label Dec 18, 2024
@avisconti
Copy link
Collaborator Author

Added DNM, because I reminded that this PR is dependent to stmemsc module which has not been modified yet.

XenuIsWatching
XenuIsWatching previously approved these changes Dec 18, 2024
@avisconti avisconti dismissed stale reviews from XenuIsWatching and teburd via 8a0009c December 19, 2024 15:28
@avisconti avisconti force-pushed the lsm6dsv16x-fifo-add-sflp branch from 558af5f to 8a0009c Compare December 19, 2024 15:28
@avisconti avisconti removed the DNM This PR should not be merged (Do Not Merge) label Dec 19, 2024
@zephyrbot
Copy link
Collaborator

zephyrbot commented Dec 19, 2024

The following west manifest projects have changed revision in this Pull Request:

Name Old Revision New Revision Diff
hal_st zephyrproject-rtos/hal_st@b2f548f zephyrproject-rtos/hal_st@05fd453 (master) zephyrproject-rtos/[email protected]

All manifest checks OK

Note: This message is automatically posted and updated by the Manifest GitHub Action.

@avisconti
Copy link
Collaborator Author

@teburd @XenuIsWatching
After zephyrproject-rtos/hal_st#24 merge, I rebased this PR and submitted again. Please take a look again if you have time.

@XenuIsWatching
Pls note that I added a patch to #79346, because after rebasing my application was not streaming the FIFO anymore. The patch is in e40f1bd. Take a look at it and maybe try it also at your end, thx!

Comment on lines 180 to 183
if (IS_ENABLED(CONFIG_LSM6DSV16X_STREAM)) {
break;
}

Copy link
Member

@XenuIsWatching XenuIsWatching Dec 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was working for me, but I am using I3C with the IBI payload which will read out the status register in to the IBI TIR payload (possibly already clearing the interrupt event with that payload?)

This change will make it perform a redunant read of the status_reg if it gets the interrupt through I3C. This probably could use some additional if checks to make it work for both

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was working for me, but I am using I3C with the IBI payload which will read out the status register in to the IBI TIR payload (possibly already clearing the interrupt event with that payload?)

Yes, when the status register gets read by any mean, the interrupt event is cleared out. So in I3C seems this is not required. I would try to remove it with some better conditions...

Copy link
Member

@XenuIsWatching XenuIsWatching left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just only 1 comment

@avisconti avisconti force-pushed the lsm6dsv16x-fifo-add-sflp branch from 8a0009c to 015e591 Compare December 27, 2024 15:03
@avisconti
Copy link
Collaborator Author

I addressed the comment...

@avisconti avisconti force-pushed the lsm6dsv16x-fifo-add-sflp branch from 015e591 to 6a3e6ac Compare December 27, 2024 15:51
teburd
teburd previously approved these changes Dec 27, 2024
* read (clearing the interrupt condition), so we can skip the extra bus
* transaction for FIFO stream case.
*/
if (ON_I3C_BUS(cfg) && I3C_INT_PIN(cfg) && IS_ENABLED(CONFIG_LSM6DSV16X_STREAM)) {
Copy link
Member

@XenuIsWatching XenuIsWatching Dec 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will not work if it is using IBI due to the wrong check on I3C_INT_PIN. I3C_INT_PIN is true if it is using a gpio, false if it is using the i3c ibi. Should be

if (ON_I3C_BUS(cfg) && !I3C_INT_PIN(cfg) && IS_ENABLED(CONFIG_LSM6DSV16X_STREAM)) {

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, my bad. :(

Copy link
Member

@XenuIsWatching XenuIsWatching left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see comment

The get_frame_count() API must return the number of frames in FIFO
for a given channel type, and not the total number of generic FIFO
frames. This API may be also re-used in the decoder routine when
the timestamp baseline is adjusted.

Signed-off-by: Armando Visconti <[email protected]>
Add three new channels and relative data:

    - SENSOR_CHAN_GAME_ROTATION_VECTOR

      The "game rotation vector" reports the four components x/y/z/w
      (quaternions) of the unit vector representing a 3D spatial
      rotation or orientation.

    - SENSOR_CHAN_GRAVITY_VECTOR

      The "gravity vector" reports the three axis x/y/z of the gravity
      vector separated from the linear acceleration components.
      The measurement unit is m/s^2.

    - SENSOR_CHAN_GBIAS_XYZ

      The "gbias" reports the gyroscope offset x/y/z components.
      The measurement unit is radians/s.

Signed-off-by: Armando Visconti <[email protected]>
Align all sensor drivers that are using stmemsc (STdC) HAL i/f
to new APIs of stmemsc v2.8

Requires zephyrproject-rtos/hal_st#24

Signed-off-by: Armando Visconti <[email protected]>
@avisconti avisconti force-pushed the lsm6dsv16x-fifo-add-sflp branch from 6a3e6ac to 1ec862d Compare December 29, 2024 18:53
@avisconti
Copy link
Collaborator Author

avisconti commented Dec 29, 2024

CI fixed by #83429
rebased and relaunched!

EDIT:
Fixed also #83094 (comment)

In case I3C IBI interrupt is not used do not skip reading the status
register, otherwise the interrupt event will not get cleared.

Signed-off-by: Armando Visconti <[email protected]>
Add the Sensor Fusion Low Power (SFLP) FIFO streaming capability,
using RTIO. The decode function is now aware of parsing following FIFO
tags:

    - LSM6DSV16X_SFLP_GAME_ROTATION_VECTOR_TAG
    - LSM6DSV16X_SFLP_GYROSCOPE_BIAS_TAG
    - LSM6DSV16X_SFLP_GRAVITY_VECTOR_TAG

To activate SFLP the user should put in DT the proper configuration.
For example, to activate a 60Hz SFLP FIFO batching rate of game rotation
vector, gravity vector and gbias components, the user should add in DT
the following:

  sflp-odr = <LSM6DSV16X_DT_SFLP_ODR_AT_60Hz>;
  sflp-fifo-enable = <LSM6DSV16X_DT_SFLP_FIFO_GAME_ROTATION_GRAVITY_GBIAS>;

Signed-off-by: Armando Visconti <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants