You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Adds espp::Vl53l5cx, a driver for the VL53L5CX 4x4 / 8x8 multizone time-of-flight sensor, to the vl53l component next to the existing VL53L4CD/CX driver (both talk I2C with 16-bit register addresses and share BasePeripheral plus the component scaffolding).
The sensor has no firmware in ROM: initialize() (run by the constructor by default) reboots it and uploads ST's ~84 KB firmware, then configures it through the DCI command interface. About two seconds at 400 kHz; the sensor supports 1 MHz.
The logic is a port of ST's Ultra Lite Driver 1.1.2 (BSD-3-Clause, as redistributed by stm32duino/VL53L5CX); the firmware / default configuration / crosstalk blobs are generated from its vl53l5cx_buffers.h and keep the license in their file headers. The component README has a license note.
API: resolution, ranging frequency (validated against the resolution), integration time, sharpener, target order, ranging mode, power mode, I2C address change; start_ranging() / stop_ranging(), is_data_ready() polling, get_ranging_data() into a RangingData (distance, sigma, signal / ambient rates, SPAD count, reflectance, target count and target status per zone, row-major); the motion indicator plugin.
One target per zone (the ULD default). The crosstalk calibration and detection threshold plugins are documented follow-ups.
components/vl53l/vl53l5cx_example (Kconfig: pins, I2C clock, resolution, frequency) prints the distance grid; docs pages, Doxyfile and the CI build matrix are updated.
Notes for review: BasePeripheral::write_many_to_register stages each write on the stack, so the driver chunks writes to 512 bytes; the I2C device timeout has to cover those and the ~1.4 KB frame reads (the example uses 100 ms).
Test plan
components/vl53l/vl53l5cx_example builds for esp32s3 (IDF 6.1), no warnings
components/vl53l/example (VL53L4) still builds
cppcheck with the CI flags is clean on the new sources
Hardware: VL53L5CX breakout on a QtPy ESP32-S3 (init, 8x8 @ 15 Hz, 4x4 @ 60 Hz, motion indicator)
Adds a driver for the VL53L5CX 4x4 / 8x8 multizone time-of-flight sensor to
the vl53l component, next to the VL53L4CD/CX driver (both use 16-bit register
addresses over I2C; they share BasePeripheral and the component scaffolding).
The sensor has no firmware in ROM, so the driver reboots it and uploads ST's
~84 KB firmware at initialize(), then talks to it through the DCI command
interface. The logic is a port of ST's Ultra Lite Driver 1.1.2 (BSD-3-Clause,
as redistributed by stm32duino/VL53L5CX; the firmware / default configuration
blobs are generated from its vl53l5cx_buffers.h, license kept in the file
headers) to espp conventions: error codes, bool returns, a recursive mutex
around the multi-step transactions, chunked writes (BasePeripheral stages a
write on the stack).
- resolution, ranging frequency (validated against the resolution),
integration time, sharpener, target order, ranging mode, power mode, I2C
address change; start / stop ranging, is_data_ready() polling and
get_ranging_data() into a RangingData (distance, sigma, signal / ambient
rates, SPAD count, reflectance, target count, target status per zone)
- motion indicator plugin ported (with the ULD's set_resolution write bug
fixed); one target per zone; xtalk calibration and detection thresholds are
documented follow-ups
- vl53l5cx_example (Kconfig: pins, I2C clock, resolution, frequency) prints
the distance grid; docs page + example page, Doxyfile, CI build entry,
component README / manifest updated
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
The reason will be displayed to describe this comment to others. Learn more.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Adds a new espp::Vl53l5cx multizone ToF driver (VL53L5CX) alongside the existing VL53L4 driver, including an ESP-IDF example and documentation/CI wiring.
- initialize() refuses a device that does not answer with the VL53L5CX ids
(no_such_device) instead of uploading the firmware into it
- send_offset_data: the NVM header is dropped with a bounded memmove of the
480 payload bytes (the ULD shifted 4 bytes past the offset buffer)
- poll_for_answer validates size / pos
- comments on the two ULD conventions kept on purpose (output list length
13, stream count 255 excluded)
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
If either ID read fails after selecting page 0, short-circuit evaluation skips the page-2 restore. Calling this health check on an initialized sensor can leave subsequent frame reads and DCI commands accessing the wrong register page after the bus recovers. Always attempt to restore page 2, preserving the original error if the probe failed.
Restore register page after failed power-register read
components/vl53l/src/vl53l5cx.cpp:490
If selecting page 0 succeeds but reading the power register fails, this return skips the page-2 restoration below. Subsequent DCI commands and frame reads then access the wrong register page even after the I2C fault clears. Attempt the restore on this error path with a separate error code so the original failure is preserved.
Restore register page after xshut-bypass failure
components/vl53l/src/vl53l5cx.cpp:893
If selecting page 0 succeeds but the xshut-bypass write fails, this error path does not restore page 2. Retrying start_ranging() then begins with a DCI resolution read against the wrong page, so a transient I2C failure can prevent recovery. Attempt the page-2 restore before returning, preserving the original error.
Preserve ranging state when stop or cleanup fails
components/vl53l/src/vl53l5cx.cpp:931
If a stop-command write fails or the MCU-stop poll times out, stopping has not been confirmed, but this still clears ranging_. Subsequent frame reads are rejected and configuration setters bypass their while-ranging guard even though the sensor may still be active. Preserve the ranging state on failure; only clear it after a successful stop and cleanup.
- initialized_ / ranging_ are atomics (is_initialized() / is_ranging() read
them without the mutex)
- is_alive(), get_power_mode() and start_ranging() restore the firmware's
register page (0x7fff = 0x02) on their failure paths too, keeping the
original error, so a transient I2C fault never leaves later DCI / frame
reads on page 0
- stop_ranging() clears ranging_ only after a confirmed stop and cleanup
- Vl53l5cx::MAX_TRANSFER_BYTES + i2c_timeout_ms(clock_hz): the example sets
the bus and device timeouts from the clock (the legacy I2C API uses the bus
timeout; 100 ms did not cover an 8x8 frame at 100 kHz)
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
The four "previously missed" items are fixed in f646e5d as well: is_alive(), get_power_mode() and start_ranging() restore register page 2 on their failure paths (original error preserved), and stop_ranging() keeps ranging_ set unless the stop and cleanup were confirmed.
Incoming error prevents recovery after successful retry
components/vl53l/src/vl53l5cx.cpp:107
Clear the incoming error before this read. The separate-write/read fallback selected by this driver's Config leaves ec unchanged on success (components/base_peripheral/include/base_peripheral.hpp:333–344). After a transient read failure, retrying is_data_ready() or get_ranging_data() with the same error-code variable therefore still returns false even when both I2C callbacks succeed. Clearing it here allows recovery while retaining any new transfer error.
Page restoration skipped when cleanup write fails
components/vl53l/src/vl53l5cx.cpp:950
If a cleanup write to 0x14, 0x15 or 0x09 fails after selecting page 0, this short-circuit chain skips restoring page 2. Subsequent frame reads or a stop retry then access the wrong register page, even after a transient I2C failure clears. Attempt the final page restore independently and preserve the first cleanup error.
Address-aware callbacks ignored for configurable I2C addresses
These adapters ignore the callback address and stay bound to 0x29 (components/i2c/include/i2c.hpp:872–892). If a caller uses this setup with set_i2c_address(), the sensor changes address, but the final page-restoration write and later transfers still target 0x29 and fail. Use address-aware bus callbacks here; the bus already has the required timeout. Also document the callback requirement on set_i2c_address() so callers do not use fixed-device adapters for address changes.
- write_multi / read_multi clear the incoming error code first: the base
class's separate write / read path only sets it, so a stale error from an
earlier failure made a successful retry of is_data_ready() /
get_ranging_data() look failed
- stop_ranging() restores the firmware's register page even when the cleanup
writes failed, reporting the first error
- the example uses address-aware bus callbacks (lambdas over I2c::write /
I2c::read) so set_i2c_address() keeps working; the Config / set_i2c_address
docs state that requirement
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
Round 3's three "previously missed" items are fixed in 2c22f41: write_multi / read_multi clear the incoming error code (the base class's separate write / read path only sets it); stop_ranging() restores register page 2 independently of the cleanup writes and reports the first error; the example uses address-aware bus callbacks (lambdas over I2c::write / I2c::read) and the Config / set_i2c_address() docs state that the callbacks must honour their address argument.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
The firmware-driven hardware port still needs device validation and human review, with power-mode reporting and address-change recovery issues unresolved.
0 open findings
Previously missed (2)
In code that hasn't changed since last review
Restore page 2 when address write fails
components/vl53l/src/vl53l5cx.cpp:492
If selecting page 0 succeeds but the address write fails before changing the address, this return leaves the sensor on page 0. Later DCI and frame operations assume page 2, so they access the wrong registers even after the bus recovers. Attempt to restore page 2 on this failure path, using a separate error code to preserve the original failure.
Accept 0x05 as a valid wakeup power mode
components/vl53l/src/vl53l5cx.cpp:514
start_ranging() writes 0x05 to register 0x09 to enable xshut bypass, but this switch accepts only 0x04 and 0x02. Reading back the awake-with-bypass value therefore sets an I/O error in get_power_mode() during normal ranging. Accept 0x05 as WAKEUP too.
- set_i2c_address() restores the firmware's register page on the old address
when the address write failed (the address did not change)
- get_power_mode() accepts 0x05 (awake with the xshut bypass that
start_ranging() sets) as WAKEUP instead of reporting an I/O error while
ranging
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
Round 4's two "previously missed" items are fixed in 1951aad: set_i2c_address() restores register page 2 on the old address when the address write fails, and get_power_mode() accepts 0x05 (awake + xshut bypass during ranging) as WAKEUP.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Firmware initialization, ranging and motion-indicator behavior still need hardware validation and final human review.
0 open findings
Previously missed (1)
In code that hasn't changed since last review
Correct default DCI_INT_TIME from 10 ms to 5 ms
components/vl53l/include/vl53l5cx.hpp:183
The bundled configuration sets DCI_INT_TIME (0x545C) to 10,000 microseconds, and initialize() does not override it. The default integration time is therefore 10 ms, not 5 ms. Please correct this value so callers do not underestimate the integration budget, particularly with four integrations per 8x8 frame.
Round 5's item is fixed in a0d9c38: the bundled default configuration sets DCI_INT_TIME to 10 000 µs, so the initialize() doc now says 10 ms (verified against the blob bytes at 0x545C).
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
The sensor-protocol port needs the planned hardware validation of initialization, ranging, and motion detection, followed by human review.
0 open findings
🧠 Review effort: Balanced
This branch has not been deployed
No deployments
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
espp::Vl53l5cx, a driver for the VL53L5CX 4x4 / 8x8 multizone time-of-flight sensor, to thevl53lcomponent next to the existing VL53L4CD/CX driver (both talk I2C with 16-bit register addresses and shareBasePeripheralplus the component scaffolding).initialize()(run by the constructor by default) reboots it and uploads ST's ~84 KB firmware, then configures it through the DCI command interface. About two seconds at 400 kHz; the sensor supports 1 MHz.vl53l5cx_buffers.hand keep the license in their file headers. The component README has a license note.start_ranging()/stop_ranging(),is_data_ready()polling,get_ranging_data()into aRangingData(distance, sigma, signal / ambient rates, SPAD count, reflectance, target count and target status per zone, row-major); the motion indicator plugin.components/vl53l/vl53l5cx_example(Kconfig: pins, I2C clock, resolution, frequency) prints the distance grid; docs pages, Doxyfile and the CI build matrix are updated.Notes for review:
BasePeripheral::write_many_to_registerstages each write on the stack, so the driver chunks writes to 512 bytes; the I2C device timeout has to cover those and the ~1.4 KB frame reads (the example uses 100 ms).Test plan
components/vl53l/vl53l5cx_examplebuilds for esp32s3 (IDF 6.1), no warningscomponents/vl53l/example(VL53L4) still builds🤖 Generated with Claude Code
https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU