drivers: sensors: bmi270: Add CRT feature support #63015

peter-axe · 2023-09-23T15:53:51Z

Issue: #62514

Description

This PR integrates enhancements to the existing bmi270 driver by introducing support for the Component Retrimming (CRT) calibration feature. This is achieved through the incorporation of a new module for the CRT calibration process and ensuring its seamless integration with the existing functionalities of the bmi270 driver.

drivers: sensors: bmi270: Add macros and function prototypes for CRT:

adds the necessary macros and function prototypes to the bmi270 driver header in order to add support for the CRT(Component Retrimming) calibration feature.

drivers: sensors: bmi270: add bmi270_soft_reset method public:

Moves the soft reset logic, previously embedded in the init method to a to a new public method:bmi270_soft_reset that is required by the crt module.

drivers: sensors: bmi270: Adds CRT feature module:

Creates a specific kconfig for the feature.
Adds a method to run the crt process.
Adds a method to program the bmi270 nvm.
Adds a method to enable/disable gain compensation.

drivers: sensors: bmi270:Add CRT support to sensor_attr_set:
Allows the bmi270 CRT feature methods to be called through Zephyr's sensor api. Using the sensor_attr_set function, with the following args:
-SENSOR_ATTR_CALIBRATION-run the crt.
-BMI270_SENSOR_ATTR_NVM_PROG-save the CRT results in nvm.
-BMI270_SENSOR_ATTR_GAIN_COMP-enable/disable gain compensation.
drivers: sensors: bmi270:Add CRT support to sensor_attr_get:

allows using sensor_attr_get with SENSOR_ATTR_CALIBRATION to retrieve the compensated user-gain updated data of gyroscope. Also, it allows checking if CRT has ever been performed.

Usage

in your prj.conf, add the feature config:
CONFIG_BMI270_CRT=y
In the dts instance of bmi270, add "bosch,bmi270-base" to the compatible list
This was introduced in a previous PR drivers: sensor: bmi270: Add support for motion, DRDY triggers #55275 in order to download the bmi270 feature blob, that allows using bmi270 features, such as CRT.
e.g.:

 bmi270@69 {
                compatible = "bosch,bmi270", "bosch,bmi270-base";

Use the sensor_attr_set from Zephyr-s sensor API to use the CRT feature.
For additional info, please consult the specific function doxygen documentation.
e.g. test :

#include <zephyr/drivers/sensor/bmi270.h>

/* Run CRT */
sensor_attr_set(dev, SENSOR_CHAN_GYRO_XYZ,SENSOR_ATTR_CALIBRATION, NULL);
/* Program NVM */
sensor_attr_set(dev, SENSOR_CHAN_GYRO_XYZ,BMI270_SENSOR_ATTR_NVM_PROG, NULL);
/*  Enable gyro gain compensation */
SENSOR_ATTR_SET_GAIN_COMPENSATION(dev, BMI270_GYR_GAIN_EN);

(Optional) Use sensor_attr_get with SENSOR_ATTR_CALIBRATION to retrieve the compensated user-gain updated data of gyroscope. And confirm if CRT has ever been performed.

   struct sensor_value usr_gain = {0};
   int ret = sensor_attr_get(dev, SENSOR_CHAN_GYRO_XYZ, SENSOR_ATTR_CALIBRATION, &usr_gain);
   if (ret != 0) {
	printf("sensor_attr_get SENSOR_ATTR_CALIBRATION failed w/ err %d\n", ret);
    }
    uint8_t usr_gain_x = usr_gain.val1 & 0xFF; 
    uint8_t usr_gain_y = (usr_gain.val1 >> 8) & 0xFF;
    uint8_t usr_gain_z = (usr_gain.val1 >> 16) & 0xFF;
    printf("User gain get: x:%d, y:%d, z:%d/n", usr_gain_x, usr_gain_y, usr_gain_z);

Tests

Ensured previous functionalities and provided example remained intact.
Performing CRT and programming the NVM:

*** Booting Zephyr OS build zephyr-v3.4.0-4217-g341590545ac8 ***
Device 0x93e4 name is bmi270@68
<dbg> bmi270: write_config_file: writing config file base
<inf> bmi270: Gyroscope user gain correction, X: 0 Y: 0 Z: 0
<dbg> bmi270: bmi270_set_aps: advance power save mode set to: 0
<dbg> bmi270: bmi270_set_aps: advance power save mode already in the intended state
<dbg> bmi270: bmi270_feature_reg_rmw: Read feature reg[0x32]@1 = 0x0000
<dbg> bmi270: bmi270_feature_reg_rmw: Wrote feature reg[0x32]@1 = 0x0000
<wrn> bmi270: Ensure that the device is at rest during CRT execution!
<dbg> bmi270: bmi270_set_aps: advance power save mode already in the intended state
<dbg> bmi270: bmi270_feature_reg_rmw: Read feature reg[0x32]@1 = 0x0000
<dbg> bmi270: bmi270_feature_reg_rmw: Wrote feature reg[0x32]@1 = 0x0100
<inf> bmi270: CRT running...
<dbg> bmi270: bmi270_set_aps: advance power save mode already in the intended state
<dbg> bmi270: bmi270_get_gyro_gain_update_status: Read feature reg[0x38]@0 = 0x0000
<dbg> bmi270: bmi270_get_gyro_gain_update_status: Status in x-axis: 0 y-axis: 0 z-axis 0, gtrigger: 0
<inf> bmi270: CRT was successful!
<inf> bmi270: Gyroscope user gain correction, X: 3 Y: 114 Z: 125
<dbg> bmi270: bmi270_set_aps: advance power save mode set to: 1
<dbg> bmi270: bmi270_set_aps: advance power save mode set to: 0
<dbg> bmi270: bmi270_set_aps: advance power save mode already in the intended state
<dbg> bmi270: bmi270_feature_reg_rmw: Read feature reg[0x34]@1 = 0x0088
<dbg> bmi270: bmi270_feature_reg_rmw: Wrote feature reg[0x34]@1 = 0x0488
<inf> bmi270: Programming NVM ...
<inf> bmi270: NVM prog Completed!
<dbg> bmi270: write_config_file: writing config file base
<dbg> bmi270: bmi270_set_gyro_gain: Gyro usr gain compensation status:1

Power cycling the bmi270 device and verifying the data was successfully saved, and detect the new calibration was the same:

*** Booting Zephyr OS build zephyr-v3.4.0-4217-g341590545ac8 ***
Device 0x93e4 name is bmi270@68
<dbg> bmi270: write_config_file: writing config file base
<inf> bmi270: Gyroscope user gain correction, X: 3 Y: 114 Z: 125
<dbg> bmi270: bmi270_set_aps: advance power save mode set to: 0
<dbg> bmi270: bmi270_set_aps: advance power save mode already in the intended state
<dbg> bmi270: bmi270_feature_reg_rmw: Read feature reg[0x32]@1 = 0x0000
<dbg> bmi270: bmi270_feature_reg_rmw: Wrote feature reg[0x32]@1 = 0x0000
<wrn> bmi270: Ensure that the device is at rest during CRT execution!
<dbg> bmi270: bmi270_set_aps: advance power save mode already in the intended state
<dbg> bmi270: bmi270_feature_reg_rmw: Read feature reg[0x32]@1 = 0x0000
<dbg> bmi270: bmi270_feature_reg_rmw: Wrote feature reg[0x32]@1 = 0x0100
<inf> bmi270: CRT running.../n
<dbg> bmi270: bmi270_set_aps: advance power save mode already in the intended state
<dbg> bmi270: bmi270_get_gyro_gain_update_status: Read feature reg[0x38]@0 = 0x0000
<dbg> bmi270: bmi270_get_gyro_gain_update_status: Status in x-axis: 0 y-axis: 0 z-axis 0, gtrigger: 0
<inf> bmi270: CRT was successful!
<inf> bmi270: Gyroscope user gain correction, X: 3 Y: 114 Z: 125
<wrn> bmi270: CRT new user-gyro gains remained the same
<dbg> bmi270: bmi270_set_aps: advance power save mode set to: 1

github-actions · 2023-09-23T15:54:36Z

Hello @peter-axe, and thank you very much for your first pull request to the Zephyr project!

A project maintainer just triggered our CI pipeline to run it against your PR and ensure it's compliant and doesn't cause any issues. You might want to take this opportunity to review the project's Contributor Expectations and make any updates to your pull request if necessary. 😊

carlescufi · 2023-09-23T21:26:19Z

Thanks for the PR! Please take a look at our commit message guidelines to find out how to format your commit messages, and at our contribution workflow to understand how to update your Pull Request.

peter-axe · 2023-09-24T14:12:05Z

Hi @carlescufi,
thanks for your input, I think I have fixed the messages according to guidelines and the compliance checks are ok.

However, something broke the pipeline/git action, and seems unrelated with my changes:
twister-build-prep

The self-hosted runner: zephyr-runner-linux-x64-4xlarge-deployment-g2tcl-2lgdq lost communication with the server. Verify the machine is running and has a healthy network connection. Anything in your workflow that terminates the runner process, starves it for CPU/Memory, or blocks its network access can cause this error.

drivers/sensor/bmi270/bmi270.h

drivers/sensor/bmi270/bmi270_crt.c

drivers/sensor/bmi270/bmi270.h

MaureenHelm · 2023-10-17T21:43:45Z

drivers/sensor/bmi270/Kconfig

@@ -62,4 +62,10 @@ config BMI270_THREAD_STACK_SIZE
 	help
 	  Stack size of thread used by the driver to handle interrupts.

+config BMI270_CRT
+	bool "CRT feature support for bmi270"
+	default n


Bools are default n unless specified otherwise, so this line can be removed.

Thanks @MaureenHelm fixed in: f3c4d22

Please amend the commit where it's introduced instead

Fixed @MaureenHelm , last commit was removed and the commits where the feature was introduced where ammended instead.

MaureenHelm · 2023-10-17T21:44:17Z

drivers/sensor/bmi270/Kconfig

+	bool "CRT feature support for bmi270"
+	default n
+	help
+	  Gyro CRT calibration feature support for bmi270


Please expand the CRT acronym

Thanks @MaureenHelm fixed in: f3c4d22

Please amend the commit where it's introduced instead

Fixed @MaureenHelm , last commit was removed and the commits where the feature was introduced where ammended instead.

MaureenHelm · 2023-10-17T21:44:44Z

drivers/sensor/bmi270/bmi270_crt.c

+	BMI270_CRT_TRIGGER_STAT_DL_ERR = 0x02,
+	/*Command is aborted by host or due to motion detection*/
+	BMI270_CRT_TRIGGER_STAT_ABORT_ERR = 0x03
+} GTriggerStatus;


Please no camel case

Thanks @MaureenHelm fixed in: f3c4d22

Please amend the commit where it's introduced instead

Fixed @MaureenHelm , last commit was removed and the commits where the feature was introduced where ammended instead.

drivers/sensor/bmi270/bmi270_crt.c

teburd · 2023-11-09T14:22:08Z

include/zephyr/drivers/sensor/bmi270.h

+#define BMI270_GYR_GAIN_DIS                0x00
+
+/* Helper Macros for setting gyro gain compensation */
+#define SENSOR_VALUE_WITH_GAIN(var_name, gain_value) \


These seem to be BMI270 specific helper macros without any prefix to them which might lead to some confusion

I agree, fixed them in latest rebase, thanks!

teburd · 2023-11-09T14:22:38Z

include/zephyr/drivers/sensor/bmi270.h

+	BMI270_CRT_TRIGGER_STAT_DL_ERR = 0x02,
+	/*Command is aborted by host or due to motion detection*/
+	BMI270_CRT_TRIGGER_STAT_ABORT_ERR = 0x03
+} gtrigger_status;


Is this typedef really needed or can it be used directly as an enum?

The typedef is needed, it's being used in the bmi2_gyr_user_gain_status struct to define the G trigger status data, and as the argument type of the helper method to LOG CRT cmd status.

teburd · 2023-11-09T14:23:32Z

include/zephyr/drivers/sensor/bmi270.h

+ * Describes the saturation status for the gyroscope gain
+ * update and G_TRIGGER command status
+ */
+struct bmi2_gyr_user_gain_status {


How is this struct used from an application, I see it in the private driver bmi270.h header as a member but its unclear how this is used outside of the driver

The structure goes into the device data structure in order for the the driver to keep track of CRT test status and results for a specific device. On the application side, one can only access the compensated user-gain data of gyroscope converted into sensor_value type, through the generic zephyr's sensors API call, e..g
sensor_attr_get(dev, SENSOR_CHAN_GYRO_XYZ, SENSOR_ATTR_CALIBRATION, &usr_gain)

teburd · 2023-11-09T14:24:07Z

include/zephyr/drivers/sensor/bmi270.h

+};
+
+/** @name Structure to store the compensated user-gain data of gyroscope */
+struct bmi2_gyro_user_gain_data {


Same question as above really, its unclear to me how this might be used in an application as its being placed in a public header

asnwered here #63015 (comment)

github-actions · 2024-01-18T00:29:45Z

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.

peter-axe · 2024-01-27T11:55:18Z

Hi @MaureenHelm can you please remove the stale label of this PR?

MaureenHelm

@avisconti please revisit

peter-axe requested a review from MaureenHelm as a code owner September 23, 2023 15:53

zephyrbot added the area: Sensors Sensors label Sep 23, 2023

zephyrbot requested review from avisconti, teburd, tristan-google and yperess September 23, 2023 15:54

zephyrbot assigned MaureenHelm Sep 23, 2023

peter-axe force-pushed the bmi270-crt-62514 branch from b395c53 to 239edf0 Compare September 23, 2023 18:09

peter-axe force-pushed the bmi270-crt-62514 branch 2 times, most recently from 36915e6 to c927cf5 Compare September 24, 2023 10:16

maxmclau reviewed Sep 26, 2023

View reviewed changes

drivers/sensor/bmi270/bmi270.h Outdated Show resolved Hide resolved

maxmclau reviewed Sep 26, 2023

View reviewed changes

drivers/sensor/bmi270/bmi270_crt.c Outdated Show resolved Hide resolved

peter-axe force-pushed the bmi270-crt-62514 branch from c927cf5 to 2d5c60d Compare September 29, 2023 19:12

peter-axe requested a review from maxmclau September 29, 2023 19:13

kartben changed the title ~~drivers:semsors:bmi270: Add CRT feature support~~ drivers: sensors: bmi270: Add CRT feature support Sep 29, 2023

MaureenHelm requested changes Oct 17, 2023

View reviewed changes

peter-axe requested review from nashif, carlescufi and galak as code owners October 21, 2023 12:05

peter-axe requested a review from MaureenHelm October 21, 2023 12:13

teburd reviewed Nov 9, 2023

View reviewed changes

peter-axe force-pushed the bmi270-crt-62514 branch from f3c4d22 to af4ac22 Compare November 18, 2023 13:00

peter-axe requested a review from teburd November 18, 2023 13:03

github-actions bot added the Stale label Jan 18, 2024

github-actions bot removed the Stale label Jan 28, 2024

ycsin removed their request for review August 15, 2024 04:13

MaureenHelm approved these changes Aug 19, 2024

View reviewed changes

peter-axe requested a review from maxmclau September 14, 2024 13:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

drivers: sensors: bmi270: Add CRT feature support #63015

drivers: sensors: bmi270: Add CRT feature support #63015

peter-axe commented Sep 23, 2023 •

edited

Loading

github-actions bot commented Sep 23, 2023

carlescufi commented Sep 23, 2023

peter-axe commented Sep 24, 2023 •

edited

Loading

MaureenHelm Oct 17, 2023

peter-axe Oct 21, 2023

MaureenHelm Nov 8, 2023

peter-axe Nov 18, 2023

MaureenHelm Oct 17, 2023

peter-axe Oct 21, 2023

MaureenHelm Nov 8, 2023

peter-axe Nov 18, 2023

MaureenHelm Oct 17, 2023

peter-axe Oct 21, 2023

MaureenHelm Nov 8, 2023

peter-axe Nov 18, 2023

teburd Nov 9, 2023

peter-axe Nov 18, 2023

teburd Nov 9, 2023

peter-axe Nov 18, 2023

teburd Nov 9, 2023

peter-axe Nov 18, 2023

teburd Nov 9, 2023

peter-axe Nov 18, 2023

github-actions bot commented Jan 18, 2024

peter-axe commented Jan 27, 2024

MaureenHelm left a comment

drivers: sensors: bmi270: Add CRT feature support #63015

Are you sure you want to change the base?

drivers: sensors: bmi270: Add CRT feature support #63015

Conversation

peter-axe commented Sep 23, 2023 • edited Loading

Issue: #62514

Description

Usage

Tests

github-actions bot commented Sep 23, 2023

carlescufi commented Sep 23, 2023

peter-axe commented Sep 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jan 18, 2024

peter-axe commented Jan 27, 2024

MaureenHelm left a comment

Choose a reason for hiding this comment

peter-axe commented Sep 23, 2023 •

edited

Loading

peter-axe commented Sep 24, 2023 •

edited

Loading