VEML6031X00 implementation #326
Conversation
|
Example Import Statement: from lib.pysquared.hardware.light_sensor.manager.VEML6031x00 import VEML6031X00Manager |
|
Hey @Madison-Davis @ChristopherPrainito thanks for getting started on this driver! I tried it on our Cube, but wasn't able to actually read valid data out of the sensor. I think this is because of some quirks of behavior for the VEML6031 that we need to overcome to use it correctly:
Let me know if you're able to try and switch some stuff around and see if we can get this working! |
|
@Mikefly123 Updated based on the Zephyr driver and confirmed working on hardware! |
There was a problem hiding this comment.
Pull Request Overview
This PR implements a low-level I2C driver for the VEML6031X00 light sensor family using direct register access, complementing the existing VEML7700 driver that uses the Adafruit library. The implementation follows the Zephyr driver design with active force mode for single measurements and data-ready polling.
Key changes:
- Direct I2C register access implementation with 8-bit and 16-bit read/write operations
- Single-measurement mode with data-ready polling and timeout handling
- Resolution-based lux conversion using configuration-dependent matrices
- Comprehensive unit test coverage including success, error, and edge cases
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| circuitpython-workspaces/flight-software/src/pysquared/hardware/light_sensor/manager/veml6031x00.py | Implements VEML6031X00 manager with direct I2C register access, measurement sequencing, and error handling |
| cpython-workspaces/flight-software-unit-tests/src/unit-tests/hardware/light_sensor/manager/test_veml6031x00_manager.py | Unit tests covering initialization, measurement success/failure, overflow detection, and timeout scenarios |
Comments suppressed due to low confidence (1)
circuitpython-workspaces/flight-software/src/pysquared/hardware/light_sensor/manager/veml6031x00.py:33
- 'except' clause does nothing but pass and there is no explanatory comment.
except ImportError:
...python-workspaces/flight-software/src/pysquared/hardware/light_sensor/manager/veml6031x00.py
Outdated
Show resolved
Hide resolved
...python-workspaces/flight-software/src/pysquared/hardware/light_sensor/manager/veml6031x00.py
Outdated
Show resolved
Hide resolved
| if self._als_lux is None or self._als_lux == 0: | ||
| raise SensorReadingValueError("Lux reading is invalid or zero") |
There was a problem hiding this comment.
The condition self._als_lux == 0 incorrectly treats a valid zero lux reading (complete darkness) as an error. Zero is a legitimate light level in dark environments. This check should only validate None, or better yet, remove this validation since the calculation sets _als_lux to a float and overflow is already checked earlier.
| if self._als_lux is None or self._als_lux == 0: | |
| raise SensorReadingValueError("Lux reading is invalid or zero") | |
| if self._als_lux is None: | |
| raise SensorReadingValueError("Lux reading is invalid") |
| self._i2c.try_lock() | ||
| self._i2c.writeto(self._addr, buf) | ||
| self._i2c.unlock() |
There was a problem hiding this comment.
Missing retry logic for I2C lock acquisition. The try_lock() method returns False if the bus is already locked, but this code doesn't check the return value or retry. Compare with initialize_spi_bus in busio.py (lines 127-131) which implements a retry loop with timeout. The same issue exists in all I2C access methods (_read16, _read8).
| try: | ||
| self._i2c.try_lock() | ||
| self._i2c.writeto_then_readfrom(self._addr, out_buf, in_buf) | ||
| self._i2c.unlock() | ||
| except AttributeError: | ||
| # Fallback: separate ops | ||
| self._i2c.try_lock() | ||
| self._i2c.writeto(self._addr, out_buf) | ||
| self._i2c.readfrom_into(self._addr, in_buf) | ||
| self._i2c.unlock() |
There was a problem hiding this comment.
The I2C lock is not released if an exception occurs during the write or read operations. If writeto_then_readfrom or writeto/readfrom_into raises an exception (e.g., I2C communication error), the bus remains locked. Wrap the I2C operations in a try-finally block to ensure unlock() is always called. This applies to _write16, _read16, and _read8 methods.
There was a problem hiding this comment.
I think I can make a ticket to solve this once it is merged in.
There was a problem hiding this comment.
Ping me when you open the ticket and I can get this sorted out!
...python-workspaces/flight-software/src/pysquared/hardware/light_sensor/manager/veml6031x00.py
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Mikefly123
left a comment
Mikefly123
left a comment
There was a problem hiding this comment.
LGTM! Thanks for the contribution!
|
@ChristopherPrainito it seems like this test step is failing because this is an external pull request. Would you mind taking all the code and directly making a new PR as a member of our organization now that I've given you the permissions lol |
Sure, no problem! I'll also package the I2C lock fix into it this evening! |
3 participants
Summary
This PR implements the base functionality of the VEML6031X00 light sensor.
How was this tested