Skip to content

Conversation

@fabocode
Copy link
Contributor

@fabocode fabocode commented Aug 7, 2025

Overview

This PR adds driver support for the Allegro Microsystems ALS31300 3-axis linear Hall Effect sensor with I2C interface.

Features Implemented

  • 3-axis magnetic field measurement (X, Y, Z axes)
  • I2C communication interface
  • Device temperature readings
  • Standard Zephyr sensor API (sample_fetch/channel_get)
  • Add driver support for one-shot sensor reads (Read/Decode Sensor API)
  • Device tree integration with proper bindings

Hardware Details

The ALS31300 is a 3D linear Hall Effect sensor that provides 12-bit digital output proportional to the magnetic field strength in each axis. The sensor communicates via I2C and is commonly used in:

  • 3D position sensing applications
  • Joystick implementations
  • Linear motion detection
  • Angle sensing applications

Testing

  • Driver builds successfully
  • Follows Zephyr sensor API conventions
  • Device tree bindings validated

Files Modified/Added

  • drivers/sensor/als31300/ - Driver implementation
  • dts/bindings/sensor/allegromicro,als31300.yaml - Device tree bindings
  • drivers/sensor/CMakeLists.txt - Build integration
  • drivers/sensor/Kconfig - Configuration options

References

@fabocode fabocode force-pushed the sensor/add-als31300 branch from 56c4273 to 04ceaf7 Compare August 7, 2025 23:04
@sonarqubecloud
Copy link

sonarqubecloud bot commented Aug 7, 2025

@ubieda
Copy link
Member

ubieda commented Aug 8, 2025

Also, please address the Issues raised by SonarQube

@kartben kartben requested a review from Copilot September 1, 2025 13:00
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds driver support for the Allegro Microsystems ALS31300 3-axis linear Hall Effect sensor with I2C interface. The implementation provides both synchronous sensor API and asynchronous RTIO API support for magnetic field measurements on X, Y, and Z axes along with device temperature readings.

  • Complete driver implementation for ALS31300 3D Hall Effect sensor with I2C communication
  • Standard Zephyr sensor API support for sample_fetch/channel_get operations
  • RTIO-based asynchronous API implementation with proper encoding/decoding functions

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tests/drivers/build_all/sensor/i2c.dtsi Adds test device tree node for build verification
dts/bindings/sensor/allegro,als31300.yaml Device tree binding definition for the sensor
drivers/sensor/als31300/als31300.h Header file with register definitions, bit masks, and data structures
drivers/sensor/als31300/als31300.c Main driver implementation with sensor API and RTIO support
drivers/sensor/als31300/Kconfig Configuration options for the driver
drivers/sensor/als31300/CMakeLists.txt Build configuration for the driver
drivers/sensor/Kconfig Integration of driver configuration into main sensor Kconfig
drivers/sensor/CMakeLists.txt Integration of driver build into main sensor CMakeLists

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.


description: Allegro ALS31300 3D Linear Hall Effect Sensor

compatible: "allegro,als31300"
Copy link

Copilot AI Sep 1, 2025

Choose a reason for hiding this comment

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

The compatible string uses 'allegro' vendor prefix, but the expected format should be 'allegromicro' to match the company name Allegro Microsystems. This should be consistent with other Allegro device bindings.

Suggested change
compatible: "allegro,als31300"
compatible: "allegromicro,als31300"

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@fabocode fabocode Sep 24, 2025

Choose a reason for hiding this comment

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

This is not applicable, after checking this will break build

* Bits [6:4] = 0 → Low-power count = 0.5ms (doesn't matter in Active Mode)
* Bits [31:7] = 0 → Reserved (should be 0)
*/
ret = i2c_reg_write_byte_dt(&cfg->i2c, ALS31300_REG_VOLATILE_27, reg27_value);
Copy link

Copilot AI Sep 1, 2025

Choose a reason for hiding this comment

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

The function i2c_reg_write_byte_dt() expects a uint8_t value but reg27_value is declared as uint32_t. This will only write the lowest byte and ignore the upper 24 bits, which may not configure the device correctly.

Suggested change
ret = i2c_reg_write_byte_dt(&cfg->i2c, ALS31300_REG_VOLATILE_27, reg27_value);
ret = i2c_reg_write_dt(&cfg->i2c, ALS31300_REG_VOLATILE_27, (uint8_t *)&reg27_value, sizeof(reg27_value));

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@fabocode fabocode Sep 24, 2025

Choose a reason for hiding this comment

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

Is required i2c_reg_write_byte_dt

Comment on lines 527 to 529
uint8_t test_val[4];

ret = i2c_reg_read_byte_dt(&cfg->i2c, ALS31300_REG_VOLATILE_27, test_val);
Copy link

Copilot AI Sep 1, 2025

Choose a reason for hiding this comment

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

The function i2c_reg_read_byte_dt() expects a uint8_t* parameter but test_val is declared as uint8_t[4]. This function only reads a single byte, not 4 bytes as the array suggests.

Suggested change
uint8_t test_val[4];
ret = i2c_reg_read_byte_dt(&cfg->i2c, ALS31300_REG_VOLATILE_27, test_val);
uint8_t test_val;
ret = i2c_reg_read_byte_dt(&cfg->i2c, ALS31300_REG_VOLATILE_27, &test_val);

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it, now I am following your suggestion

@fabocode fabocode force-pushed the sensor/add-als31300 branch 7 times, most recently from cbd86fa to eb0c088 Compare September 24, 2025 00:10
@fabocode fabocode requested review from kartben and ubieda September 24, 2025 00:54
@fabocode fabocode force-pushed the sensor/add-als31300 branch 3 times, most recently from 9aa1e02 to e58b74c Compare September 24, 2025 18:15
@fabocode fabocode force-pushed the sensor/add-als31300 branch 2 times, most recently from f73cde1 to 518883a Compare September 25, 2025 01:47
@fabocode fabocode requested a review from ubieda September 25, 2025 02:25
ubieda
ubieda previously approved these changes Sep 25, 2025
Copy link
Member

@ubieda ubieda left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks!

* @param raw_value Signed 12-bit magnetic field value
* @return Magnetic field in microgauss
*/
int32_t als31300_convert_to_gauss(const struct device *dev, int16_t raw_value)
Copy link
Member

Choose a reason for hiding this comment

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

Why does this function take a device pointer argument?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will delete it, I thought it might be used in the future, but this will simplify it

Comment on lines 172 to 180
/* Convert microcelsius to sensor_value format */
val->val1 = converted_val / 1000000;
val->val2 = converted_val % 1000000;

/* Handle negative temperatures properly */
if (converted_val < 0 && val->val2 != 0) {
val->val1 -= 1;
val->val2 = 1000000 + val->val2;
}
Copy link
Member

Choose a reason for hiding this comment

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

Use sensor_value_from_micro

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I added this as well

Comment on lines 189 to 196
/* Convert microgauss to sensor_value format */
val->val1 = converted_val / 1000000;
val->val2 = converted_val % 1000000;
/* Handle negative values properly */
if (converted_val < 0 && val->val2 != 0) {
val->val1 -= 1;
val->val2 = 1000000 + val->val2;
}
Copy link
Member

Choose a reason for hiding this comment

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

ditto

return 0;
}

static const struct sensor_driver_api als31300_api = {
Copy link
Member

Choose a reason for hiding this comment

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

Use DEVICE_API

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I included it

Comment on lines 238 to 239
/* Small delay to ensure the mode change takes effect */
k_msleep(10);
Copy link
Member

Choose a reason for hiding this comment

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

Why this particular value for the delay?

Copy link
Contributor Author

@fabocode fabocode Oct 6, 2025

Choose a reason for hiding this comment

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

It was meant for testing at first. I tested and functionality is not affected, so I will delete it.

Add driver for Allegro Microsystems ALS31300 3-axis linear Hall Effect
sensor. The driver supports:
- I2C communication interface
- X, Y, Z magnetic field measurements
- Device temperature readings

Signed-off-by: Fabian Barraez <fabianbarraez@gmail.com>
@fabocode fabocode force-pushed the sensor/add-als31300 branch from 518883a to 0e14a50 Compare October 5, 2025 00:03
@zephyrbot zephyrbot added area: Boards/SoCs area: Tests Issues related to a particular existing or missing test area: Devicetree Bindings labels Oct 5, 2025
@zephyrbot zephyrbot requested a review from nashif October 5, 2025 00:05
@sonarqubecloud
Copy link

sonarqubecloud bot commented Oct 5, 2025

@fabocode fabocode requested review from MaureenHelm and ubieda October 6, 2025 01:14
@jhedberg jhedberg merged commit 6b8fcae into zephyrproject-rtos:main Oct 17, 2025
32 checks passed
@fabocode fabocode deleted the sensor/add-als31300 branch October 17, 2025 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: Boards/SoCs area: Devicetree Bindings area: Sensors Sensors area: Tests Issues related to a particular existing or missing test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants