Fixes on 0x69 address detection#10715
Open
oscgonfer wants to merge 19 commits into
Open
Conversation
* Change SEN5X detection method, using class itself * MPU6050 register check * BMX160 default removed
oscgonfer
added
tech debt
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR improves I2C device detection on shared address 0x69, removing fallback assumptions and making detection more explicit—especially for SEN5X air quality sensors and MPU6050 IMUs.
Changes:
- Add a
SEN5XSensor::probe(...)method and use it from the I2C scanner instead of a custom product-name probe. - Add an MPU6050
WHO_AM_Iregister value check. - Remove the previous “default to BMX160/MPU6050” behavior when detection fails.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/modules/Telemetry/Sensor/SEN5XSensor.h |
Exposes a new probe(...) API to support scanner-based detection. |
src/modules/Telemetry/Sensor/SEN5XSensor.cpp |
Implements probe(...) by resetting the sensor and identifying the model. |
src/detect/ScanI2CTwoWire.cpp |
Replaces custom SEN5X probing with SEN5XSensor::probe() and adds MPU6050 WHO_AM_I detection while removing old default fallbacks. |
|
|
||
| bool SEN5XSensor::probe(TwoWire *bus, uint8_t address, ScanI2C::I2CPort port) | ||
| { | ||
| LOG_INFO("SEN5X: probing sensor"); |
Comment on lines
+140
to
145
| #include "../modules/Telemetry/Sensor/SEN5XSensor.h" | ||
| bool probeSEN5X(TwoWire *i2cBus, uint8_t address, ScanI2C::I2CPort port) | ||
| { | ||
| uint8_t cmd[] = {0xD0, 0x14}; | ||
| uint8_t response[48] = {0}; | ||
|
|
||
| i2cBus->beginTransmission(address); | ||
| i2cBus->write(cmd, 2); | ||
| if (i2cBus->endTransmission() != 0) | ||
| return ""; | ||
|
|
||
| delay(20); | ||
| if (i2cBus->requestFrom(address, (uint8_t)48) != 48) | ||
| return ""; | ||
|
|
||
| for (int i = 0; i < 48 && i2cBus->available(); ++i) { | ||
| response[i] = i2cBus->read(); | ||
| } | ||
|
|
||
| char productName[33] = {0}; | ||
| int j = 0; | ||
| for (int i = 0; i < 48 && j < 32; i += 3) { | ||
| if (response[i] >= 32 && response[i] <= 126) | ||
| productName[j++] = response[i]; | ||
| else | ||
| break; | ||
|
|
||
| if (response[i + 1] >= 32 && response[i + 1] <= 126) | ||
| productName[j++] = response[i + 1]; | ||
| else | ||
| break; | ||
| } | ||
|
|
||
| return String(productName); | ||
| SEN5XSensor sen5xsensor = SEN5XSensor(); | ||
| return sen5xsensor.probe(i2cBus, address, port); | ||
| } |
Contributor
Author
There was a problem hiding this comment.
Not quite sure I agree with this. I understand the module coupling, but there are two considerations:
- The sensor has quite a bit of details that are handled in the class directly (changing bus speed when sending commands, for one) that would need to be replicated in a separate class
- The sensor also has an internal firmware versioning, which is also detected in the class. If in the future there are new firmware versions for the sensor, we would only need to update de class, and not the probing on a different function.
Contributor
⚡ Try this PR in the Web Flasher
Warning This is an automated, unreviewed CI test build. Back up your device configuration Supported boards built by this PR (24)
Build artifacts expire on 2026-07-14. Updated for |
* Make ReClockI2C API class * Use new API in AirQualityTelemetry module * Minor changes on some logs
* Define map for sensors to re-scan * Add re-scan on runOnce
Contributor
Firmware Size Report22 targets | vs
Show 17 more target(s)
Updated for f8bb8cb |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR fixes some detection issues on devices with shared address 0x69
BMX160 detection has recently been improved by @caveman99, but there was some leftover code that assumed BMX160 as a default if everything else failed. Similar to MPU6050, which a WHO_AM_I register check was added.
The SEN5X sensor detection process is also improved, avoiding a custom probe for the model, but instead using the class methods, which account for all the I2C speed shebang (#9898 and #10593).
Changes:
🤝 Attestations
Tagging @caveman99 for checks if possible with the BMX160 code.
Caution
Rebased onto #9898 and #10593