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

sensor: adxl345: Decoder-related bug-fixes #83476

Merged
merged 4 commits into from
Jan 20, 2025

Conversation

ubieda
Copy link
Member

@ubieda ubieda commented Dec 31, 2024

Description

This PR applies various fixes to the ADXL345 driver:

  • Decoder:
    • Add adxl345_get_size_info() used, for instance, by the Sensor Shell.
    • Fix adxlXXX_decode_sample() to use sensor_three_axis_data.
    • Fix Q-scale factor scaling tables, and add some description on the calculation rationale.
  • Driver:
    • Disable Streaming by default so users do not need to worry about it unless explicitly interested in such functionality.
    • Only write FIFO_STREAMING config-bit if using FIFO, to suppress delay in sample acquisition for basic read/decode applications.

Fixes #84252

Testing

Run Shell sample with sensor commands on the ADXL345 (tested both for SPI and I2C). The following evidence is for I2C:

west build -b nrf52840dk/nrf52840 samples/subsys/shell/shell_module
  • DTS Overlay (samples/subsys/shell/shell_module/boards/nrf52840dk_nrf52840.overlay)
/ {
	aliases {
		accel0 = &adxl345;
	};
};

&arduino_i2c {
	status = "okay";
	
	adxl345: adxl345@53 {
		compatible = "adi,adxl345";
		reg = <0x53>;
		status = "okay";
		int2-gpios = <&gpio1 3 GPIO_ACTIVE_HIGH>;
	};
};
  • Config Overlay (samples/subsys/shell/shell_module/boards/nrf52840dk_nrf52840.conf)
CONFIG_SENSOR=y
CONFIG_SENSOR_ASYNC_API=y
CONFIG_SENSOR_SHELL=y
  • Sample Output:
uart:~$ sensor get adxl345@53 
channel type=3(accel_xyz) index=0 shift=7 num_samples=1 value=8177551269ns, (0.000000, -0.459686, 8.887274)
uart:~$ sensor get adxl345@53
channel type=3(accel_xyz) index=0 shift=7 num_samples=1 value=8825378417ns, (0.000000, -0.459686, 8.887274)
uart:~$ sensor get adxl345@53
channel type=3(accel_xyz) index=0 shift=7 num_samples=1 value=9273559570ns, (0.000000, -0.459686, 8.887274)
uart:~$ sensor get adxl345@53
channel type=3(accel_xyz) index=0 shift=7 num_samples=1 value=9713378906ns, (0.000000, -0.459686, 8.887274)
uart:~$ sensor get adxl345@53
channel type=3(accel_xyz) index=0 shift=7 num_samples=1 value=10129577636ns, (0.000000, -0.459686, 9.040503)
uart:~$ sensor get adxl345@53
channel type=3(accel_xyz) index=0 shift=7 num_samples=1 value=12177551269ns, (-1.532288, -0.612915, 9.346961)
uart:~$ sensor get adxl345@53
channel type=3(accel_xyz) index=0 shift=7 num_samples=1 value=12641601562ns, (1.685517, -6.129155, 4.137179)
uart:~$ 

@ubieda ubieda added the DNM This PR should not be merged (Do Not Merge) label Dec 31, 2024
@ubieda ubieda force-pushed the croxel/fix-adxl-decoders branch from e6fade9 to a7699d9 Compare January 15, 2025 00:41
@ubieda ubieda marked this pull request as ready for review January 15, 2025 02:05
@zephyrbot zephyrbot added platform: ADI Analog Devices, Inc. area: Sensors Sensors labels Jan 15, 2025
@ubieda ubieda changed the title sensor: adxl: Fix decoders to utilize sensor_three_axis_data sensor: adxl345: Decoder-related bug-fixes Jan 15, 2025
Used by the sensor-shell in order to retrieve values, otherwise
it crashes.

Signed-off-by: Luis Ubieda <luisf@croxel.com>
The following fixes have been applied to this decoder:
- The Q-scale factor was fixed, both for full-scale and non
full-scale modes.
- The data-type decoded is struct sensor_three_axis_data, as
it should for read/decode API.

Signed-off-by: Luis Ubieda <luisf@croxel.com>
Otherwise with its default configuration (25-Hz, 32-level FIFO),
getting individual samples could be up to 1-second old.

Signed-off-by: Luis Ubieda <luisf@croxel.com>
Have the application enable this feature explicitcly, so that
simple applications do not need to disable this to get the
expected behavior.

Signed-off-by: Luis Ubieda <luisf@croxel.com>
@ubieda ubieda force-pushed the croxel/fix-adxl-decoders branch from a7699d9 to 1868211 Compare January 15, 2025 19:07
@ubieda ubieda removed the DNM This PR should not be merged (Do Not Merge) label Jan 15, 2025
@ubieda
Copy link
Member Author

ubieda commented Jan 15, 2025

Just rebased. Need to create a GH issue to get this backported into v4.0-branch

@laurensslats
Copy link

Tested and it works for me using the ADXL345.

@ubieda ubieda added the backport v4.0-branch Backport to the v4.0-branch label Jan 20, 2025
@ubieda
Copy link
Member Author

ubieda commented Jan 20, 2025

Created issue and added backport label

@cfriedt
Copy link
Member

cfriedt commented Jan 20, 2025

I also tested this on the xiao_esp32c3 with adxl343

Copy link
Member

@MaureenHelm MaureenHelm left a comment

Choose a reason for hiding this comment

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

Thanks @ubieda !

I tested this out with the following and it looks good:

$ west build -p -b apard32690//m4 --shield pmod_acl samples/subsys/shell/shell_module/ -- -DCONFIG_SENSOR=y -DCONFIG_SENSOR_ASYNC_API=y -DCONFIG_SENSOR_SHELL=y

@dimitrije-lilic please take a look

@kartben kartben merged commit e453a0c into zephyrproject-rtos:main Jan 20, 2025
28 of 29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Sensors Sensors backport v4.0-branch Backport to the v4.0-branch platform: ADI Analog Devices, Inc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sensor: ADXL345 Decoder does not work
7 participants