Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 47 additions & 13 deletions src/modules/Telemetry/Sensor/BME280Sensor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,36 +10,70 @@

BME280Sensor::BME280Sensor() : TelemetrySensor(meshtastic_TelemetrySensorType_BME280, "BME280") {}

static void setSensorConfigs(Adafruit_BME280 *bme280)
{
bme280->setSampling(Adafruit_BME280::MODE_FORCED,
Adafruit_BME280::SAMPLING_X1, // Temp. oversampling
Adafruit_BME280::SAMPLING_X1, // Pressure oversampling
Adafruit_BME280::SAMPLING_X1, // Humidity oversampling
Adafruit_BME280::FILTER_OFF, Adafruit_BME280::STANDBY_MS_1000);
}

bool BME280Sensor::initDevice(TwoWire *bus, ScanI2C::FoundDevice *dev)
{
bus->setTimeout(50);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

Does TwoWire::setTimeout on ESP32 and nRF52 (Adafruit nRF52 core / mbed) map to Stream::setTimeout or to an I2C hardware timeout like setWireTimeout?

💡 Result:

The TwoWire::setTimeout method on ESP32 does not map to Stream::setTimeout, nor is it equivalent to setWireTimeout as used in other Arduino cores. On the ESP32 (Arduino ESP32 core), TwoWire::setTimeout is a custom, ESP32-specific API [1]. It is used to set the timeout in milliseconds for I2C transactions, which is then passed directly to the low-level i2cWrite function [2][3]. It is not related to the Stream class's setTimeout, which typically handles serial communication timing. The setWireTimeout method, which is commonly associated with the AVR and certain other cores (like Renesas) to manage I2C hardware bus lockups and timeouts, is generally not implemented in the ESP32 core under that name [4][5][6]. While the goal of the ESP32's setTimeOut is to provide a transaction-level timeout to avoid blocking, it operates independently of the setWireTimeout API found in other ecosystems [1][2]. Regarding the Adafruit nRF52 core (based on mbed), it does not natively implement setWireTimeout or the ESP32's setTimeOut as a standard feature across all boards [7][5]. The Wire library API can vary significantly between different hardware cores in the Arduino ecosystem, as there is no single, unified standard for I2C timeout configuration across all platforms [4][5]. In summary: 1. ESP32's setTimeOut is a unique, platform-specific function that controls I2C transaction duration in milliseconds [1][2]. 2. It does not interface with Stream::setTimeout [2]. 3. It is not equivalent to the setWireTimeout method found in other Arduino cores [4][5]. For cross-platform code, you cannot rely on either setWireTimeout or setTimeout to consistently provide I2C hardware timeout functionality [5].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Candidate file =="
fd -a 'BME280Sensor.cpp' . || true

echo "== File excerpt =="
if [ -f src/modules/Telemetry/Sensor/BME280Sensor.cpp ]; then
  cat -n src/modules/Telemetry/Sensor/BME280Sensor.cpp | sed -n '1,110p'
fi

echo "== Repository Wire/TwoWire declarations =="
rg -n "class TwoWire|struct TwoWire|setTimeout|setWireTimeout|setTimeOut" -S . -g '*.h' -g '*.hpp' -g '*.cpp' | head -200

echo "== Relevant I2C bus initialization / Platform-specific APIs =="
rg -n "Wire\.(setTimeout|setWireTimeout|setTimeOut)|setTimeOut\(|setTimeout\(" . -g '*.cpp' -g '*.h' -g '*.hpp' | head -200

Repository: meshtastic/firmware

Length of output: 7655


🌐 Web query:

nRF52 Adafruit TwoWire setTimeout function definition Stream.hpp inherits setTimeout

💡 Result:

In the Adafruit nRF52 Arduino core, the TwoWire class (defined in Wire.h) inherits from the Stream class [1][2]. Because the Stream class provides the setTimeout method, the TwoWire class technically inherits this function [3][4]. However, there is no specific definition or implementation of setTimeout within the TwoWire class itself in the Adafruit nRF52 library [1][5][6]. Furthermore, the TwoWire implementation for nRF52 does not utilize the Stream-based timeout functionality for its I2C operations (such as endTransmission or requestFrom) [7]. Technical findings: 1. Inheritance: TwoWire inherits from Stream, which makes setTimeout available as part of the public interface [1][2]. 2. Implementation: The TwoWire class does not override or implement its own setTimeout function [1][2]. 3. Functional Status: Users have reported that the nRF52 Wire library lacks timeout handling in its I2C communication methods, meaning that if an I2C device stalls or hangs the bus, calls to functions like endTransmission may block indefinitely because they do not implement or check for the Stream-defined timeout [7].

Citations:


🌐 Web query:

Adafruit nRF52 TwoWire stream setTimeout I2C requestFrom timeout

💡 Result:

In the Adafruit nRF52 Arduino core, the TwoWire implementation does not support a software-based timeout mechanism for I2C transactions [1]. Unlike some other Arduino cores (such as AVR) that implement setWireTimeout to recover from bus hangs [2][3][4], the Adafruit nRF52 Wire library lacks this functionality [1][5][6]. Technical details: 1. Blocking Behavior: The TwoWire::requestFrom and endTransmission functions in the nRF52 core are blocking [1][5][6]. They utilize while loops to poll hardware event registers (e.g., EVENTS_RXSTARTED, EVENTS_LASTRX, EVENTS_STOPPED) [5][6]. 2. Lack of Timeout: If an I2C device fails to respond, NACKs, or holds the bus in a way that prevents these hardware events from triggering, the code will remain stuck in these while loops indefinitely [1][7]. 3. No setWireTimeout: The setWireTimeout function available in other Arduino cores is not part of the standard Adafruit nRF52 Wire library API [8][2]. Users experiencing hangs due to unresponsive I2C devices will find that the system does not automatically recover [1]. Workarounds and Recommendations: * Hardware Reliability: Since software recovery is unavailable, ensure physical I2C bus integrity. This includes using correct pull-up resistors and addressing signal noise, which are common causes of hangs on the nRF52 [1]. * External Watchdog: Using the internal nRF52 Watchdog Timer (WDT) is the most robust way to recover the system if an I2C hang occurs. You can configure the WDT to reset the microcontroller if the main loop stops responding due to an I2C block. * Custom Implementation: While advanced users could theoretically attempt to modify Wire_nRF52.cpp to add timeout checks based on a timer or micros [1], this requires custom firmware builds and is not a standard supported feature.

Citations:


🌐 Web query:

mbed Wire TwoWire setTimeout nRF52

💡 Result:

The class TwoWire does not consistently implement a universal setWireTimeout method across all Mbed-enabled or Arduino-based nRF52 platforms [1][2]. In the context of the Mbed OS nRF52 HAL (Hardware Abstraction Layer), timeout management for I2C (TWI) operations is handled internally by the driver rather than through a public TwoWire method [3]. Historically, issues with I2C "hanging" on nRF52 due to clock stretching or bus errors have been managed by modifying internal timeout values within the driver's C source code (e.g., i2c_api.c), often involving the use of microsecond tickers to monitor transaction duration [4][5][3][6]. If you are using the Arduino framework on an nRF52 (such as the Adafruit nRF52 board support package), the Wire library may lack a setWireTimeout() method [2], which is commonly found in other architectures like AVR [7] or ESP32 [8]. Because implementation varies by the specific board support package (BSP) and Mbed OS version, you should check the following: 1. Check for library-specific methods: Some variants of the Wire library may implement setTimeout(ms) [8], but this is not guaranteed for all nRF52 cores [1][2]. 2. Consult the underlying HAL: If your application hangs during I2C, it is often due to the driver entering an infinite loop while waiting for hardware events (like STOP or RX/TX) [2]. On Mbed-based platforms, ensure you are using a recent version of the OS, as many I2C driver instabilities regarding timeouts and clock stretching have been addressed in later releases [6][9]. 3. Manual Timeout: If the library lacks an API to set a timeout, you may need to implement a "watchdog" approach or use a non-blocking I2C wrapper if available, or verify your hardware pull-up resistors to prevent bus lockup conditions [2]. In summary, there is no standardized setWireTimeout API for TwoWire on nRF52; reliance on driver-level internal timeouts or higher-level application logic is generally required [3][2].

Citations:


🌐 Web query:

Adafruit nRF52 TwoWire setTimeout function definition Stream.hpp inherits setTimeout

💡 Result:

In the Adafruit nRF52 Arduino core, the TwoWire class inherits from the Arduino Stream class [1][2]. The Stream class provides the setTimeout(unsigned long timeout) method [3][4][5], which sets the maximum time, in milliseconds, that the stream will wait for data [6]. However, for the TwoWire class in the Adafruit nRF52 implementation, the setTimeout method from the Stream class is intended for stream-based reading operations (e.g., read, peek) [3][6]. It does not provide I2C bus timeout functionality for methods like endTransmission or requestFrom [7]. Because the TwoWire implementation in this core lacks specific I2C timeout mechanisms, these I2C functions can hang if the bus or target device experiences issues [7]. To clarify: 1. Inheritance: TwoWire inherits from Stream [1][2], so it technically has access to the setTimeout function inherited from the base class [3][6]. 2. Functionality: This setTimeout function affects operations that use the stream-based read/peek logic [6] and does not control the low-level TWI/I2C communication timing [7]. 3. I2C Timeouts: The Adafruit nRF52 TwoWire class does not implement a setWireTimeout function or similar I2C-specific timeout control, which is a known limitation when compared to some other Arduino cores [7][8].

Citations:


Remove the non-portable I2C timeout call.

bus->setTimeout(50) is not a portable I2C hang-protection call. On the nRF54L15 platform in this repository, TwoWire::setTimeout() is an empty stub, so this call does nothing. On core variants where it is inherited from Stream, it affects stream-read helpers instead of requestFrom()/endTransmission(). If I2C timeout behavior is required, gate it behind platform-specific APIs and use the correct timeout type/units for each implementation.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/modules/Telemetry/Sensor/BME280Sensor.cpp` at line 15, Remove the
bus->setTimeout(50) call from the BME280 sensor initialization; do not replace
it with a generic I2C timeout mechanism, and only add platform-specific timeout
handling if an appropriate API and units are available.

LOG_INFO("Init sensor: %s", sensorName);
status = bme280.begin(dev->address.address, bus);
if (!status) {
return status;
}

bme280.setSampling(Adafruit_BME280::MODE_FORCED,
Adafruit_BME280::SAMPLING_X1, // Temp. oversampling
Adafruit_BME280::SAMPLING_X1, // Pressure oversampling
Adafruit_BME280::SAMPLING_X1, // Humidity oversampling
Adafruit_BME280::FILTER_OFF, Adafruit_BME280::STANDBY_MS_1000);
setSensorConfigs(&bme280);

initI2CSensor();
return status;
}

bool BME280Sensor::getMetrics(meshtastic_Telemetry *measurement)
static void getSensorData(meshtastic_Telemetry *measurement, Adafruit_BME280 *bme280)
{
measurement->variant.environment_metrics.temperature = bme280->readTemperature();
measurement->variant.environment_metrics.relative_humidity = bme280->readHumidity();
measurement->variant.environment_metrics.barometric_pressure = bme280->readPressure() / 100.0F;
measurement->variant.environment_metrics.has_temperature = true;
measurement->variant.environment_metrics.has_relative_humidity = true;
measurement->variant.environment_metrics.has_barometric_pressure = true;
}

bool BME280Sensor::getMetrics(meshtastic_Telemetry *measurement)
{
LOG_DEBUG("BME280 getMetrics");
bme280.takeForcedMeasurement();
measurement->variant.environment_metrics.temperature = bme280.readTemperature();
measurement->variant.environment_metrics.relative_humidity = bme280.readHumidity();
measurement->variant.environment_metrics.barometric_pressure = bme280.readPressure() / 100.0F;

return true;
if(bme280.takeForcedMeasurement())
{
getSensorData(measurement, &bme280);
return true;
}
else
{
LOG_WARN("BME280 measurement failed, attempting reset.");
if(bme280.init())
{
setSensorConfigs(&bme280);
LOG_DEBUG("BME280 reset success, getMetrics");
if(bme280.takeForcedMeasurement())
{
getSensorData(measurement, &bme280);
return true;
}
else
{
LOG_WARN("BME280 measurement failed again.");
}
}
else
{
LOG_WARN("BME280 reset/reinit failed.");
}
}
return false;
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
#endif
#endif
Loading