SEN5X: fix version parsing, VOC index reporting, and read-buffer handling (#11114)

* SEN5X: validate read lengths and initialize read buffers

readBuffer() returns the number of data bytes written (0 on error).
Check the return value against the number of bytes each caller
consumes before parsing, and zero-initialize the destination buffers:
findModel (5), getMeasurements (2), readValues (16), readPNValues
(20), and vocStateFromSensor (SEN5X_VOC_STATE_BUFFER_SIZE). This also
resolves maybe-uninitialized compiler warnings.

Small simplifications in the same area:

- Assign the converted measurement values directly; the isnan()
  checks on integer intermediates always took the conversion branch,
  so this preserves behavior.
- Fold a redundant state comparison in wakeUp() that immediately
  followed the assignment of the same value.
- Add an explicit 'return false' to the non-FSCom branches of
  loadState() and saveState().

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* SEN5X: correct version parsing, VOC index gating, and cleaning wait

- getVersion() requested 3 raw I2C bytes (2 data bytes) but parsed
  versionBuffer[0..6], so the hardware and protocol versions came from
  the buffer's initialized-but-unwritten tail. Request the full
  12-byte reply (8 data bytes, the layout used by Sensirion's
  embedded-i2c-sen5x driver) and validate the received length before
  parsing. Also make the error message specific to the version read.
- Use floating-point division when deriving major.minor version
  numbers so minor versions below 10 are preserved (integer division
  reported e.g. firmware 2.2 as 2.00).
- Gate pm_voc_idx on vocIndex rather than noxIndex, so SEN54 devices
  (VOC but no NOx) report their VOC index.
- Widen the millis() snapshot in startCleaning() to uint32_t so the
  10-second fan-cleaning wait always measures elapsed time correctly.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* SEN5X: add size checks to I2C helpers, stage VOC state, handle unavailable readings

- readBuffer(): accept only request sizes that are a multiple of 3
  (2 data bytes + 1 CRC per group), keeping the read loop's size
  arithmetic in bounds for any future caller. Current callers all
  comply.
- sendCommand(): likewise accept only even payload sizes on the write
  side.
- vocStateFromSensor(): read into a staging buffer and copy to
  vocState only after the full transfer verifies, so the stored state
  stays consistent if a read fails partway through.
- readValues()/readPNValues(): the sensor reports unavailable values
  as 0xFFFF (unsigned) / 0x7FFF (signed); map these to the
  UINT16_MAX / UINT32_MAX / FLT_MAX sentinels that getMetrics()
  checks, so unavailable channels are omitted from telemetry rather
  than scaled into numeric readings. Guard the cumulative-to-binned
  PN subtraction so the sentinels are preserved.
- readPNValues(): convert #/cm3 to #/0.1l as raw * 10, retaining the
  0.1-resolution digit that dividing before multiplying discarded.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* SEN5X: size read buffers in data bytes and document I2C helper conventions

readBuffer()'s size parameter is the raw I2C transfer size including
CRC bytes, while only the verified data bytes (2/3 of the request) are
written to the destination. Two call sites sized their buffers in raw
units (findModel: 48 for 32 data bytes; getMeasurements: 3 for 2);
both were safe over-allocations. Size them in data bytes so every call
site reflects the same convention, and document the raw-vs-data
contracts on the readBuffer() and sendCommand() declarations.

No functional change.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* SEN5X: use named defines for I2C reply buffer sizes

Follow the SEN5X_VOC_STATE_BUFFER_SIZE pattern for all reply reads,
per review feedback: define each reply's payload size in data bytes,
size the destination buffer with it, request <size> + <size> / 2 raw
bytes, and compare the received count against the same define.

The version and product-name guards now compare against the full
reply size rather than the bytes parsed (previously 7 and 5);
readBuffer() returns either 0 or the full data count, so the
conditions accept and reject the same transfers.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* SEN5X: document I2C helper size requirements instead of checking at runtime

Per review feedback: drop the runtime even-size and multiple-of-3
checks from sendCommand()/readBuffer() and state the requirements in
@brief/@param documentation on the declarations. All callers pass
sizes derived from the SEN5X_*_BUFFER_SIZE defines, which satisfy
both requirements.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* SEN5X: name the sensor's invalid-value constants

Per review feedback, define SEN5X_UINT_INVALID (0xFFFF) and
SEN5X_INT_INVALID (0x7FFF) for the values the sensor reports when a
reading is unavailable, and use them in the readValues()/readPNValues()
conversions in place of the numeric literals.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: oscgonfer <oscgonfer@users.noreply.github.com>
Co-authored-by: Thomas Göttgens <tgoettgens@gmail.com>
This commit is contained in:
Michael Mohr
2026-08-12 06:56:46 +00:00
committed by GitHub
co-authored by Claude Fable 5 oscgonfer Thomas Göttgens
parent b68de08c6b
commit d5d5bad97c
2 files changed
+92 -51

No files matched your search

+64 -50
View File
@@ -22,16 +22,18 @@ bool SEN5XSensor::getVersion()
}
delay(20); // From Sensirion Datasheet
uint8_t versionBuffer[12]{};
size_t charNumber = readBuffer(&versionBuffer[0], 3);
if (charNumber == 0) {
LOG_ERROR("%s: Error getting data ready flag value", sensorName);
// Version reply layout: fw major/minor, fw debug, hw major/minor,
// protocol major/minor, padding
uint8_t versionBuffer[SEN5X_VERSION_BUFFER_SIZE]{};
size_t charNumber = readBuffer(&versionBuffer[0], SEN5X_VERSION_BUFFER_SIZE + (SEN5X_VERSION_BUFFER_SIZE / 2));
if (charNumber < SEN5X_VERSION_BUFFER_SIZE) {
LOG_ERROR("%s: Error getting device version value", sensorName);
return false;
}
firmwareVer = versionBuffer[0] + (versionBuffer[1] / 10);
hardwareVer = versionBuffer[3] + (versionBuffer[4] / 10);
protocolVer = versionBuffer[5] + (versionBuffer[6] / 10);
firmwareVer = versionBuffer[0] + (versionBuffer[1] / 10.0f);
hardwareVer = versionBuffer[3] + (versionBuffer[4] / 10.0f);
protocolVer = versionBuffer[5] + (versionBuffer[6] / 10.0f);
LOG_INFO("%s: Firmware Version: %0.2f", sensorName, firmwareVer);
LOG_INFO("%s: Hardware Version: %0.2f", sensorName, hardwareVer);
@@ -48,12 +50,11 @@ bool SEN5XSensor::findModel()
}
delay(50); // From Sensirion Datasheet
const uint8_t nameSize = 48;
uint8_t name[nameSize];
size_t charNumber = readBuffer(&name[0], nameSize);
uint8_t name[SEN5X_PRODUCT_NAME_BUFFER_SIZE]{};
size_t charNumber = readBuffer(&name[0], SEN5X_PRODUCT_NAME_BUFFER_SIZE + (SEN5X_PRODUCT_NAME_BUFFER_SIZE / 2));
bool foundModel = false;
if (charNumber == 0) {
if (charNumber < SEN5X_PRODUCT_NAME_BUFFER_SIZE) {
LOG_ERROR("%s: Error getting device name", sensorName);
return foundModel;
}
@@ -361,15 +362,18 @@ bool SEN5XSensor::vocStateFromSensor()
delay(20); // From Sensirion Datasheet
// Retrieve the data
// Allocate buffer to account for CRC
size_t receivedNumber = readBuffer(&vocState[0], SEN5X_VOC_STATE_BUFFER_SIZE + (SEN5X_VOC_STATE_BUFFER_SIZE / 2));
// Retrieve the data into a staging buffer so a partial read (e.g. a CRC
// failure halfway through) cannot corrupt the current vocState.
// The requested size accounts for the CRC bytes
uint8_t stateBuffer[SEN5X_VOC_STATE_BUFFER_SIZE]{};
size_t receivedNumber = readBuffer(&stateBuffer[0], SEN5X_VOC_STATE_BUFFER_SIZE + (SEN5X_VOC_STATE_BUFFER_SIZE / 2));
delay(20); // From Sensirion Datasheet
if (receivedNumber == 0) {
if (receivedNumber < SEN5X_VOC_STATE_BUFFER_SIZE) {
LOG_DEBUG("%s: Error getting VOC's state", sensorName);
return false;
}
memcpy(vocState, stateBuffer, SEN5X_VOC_STATE_BUFFER_SIZE);
// Print the state (if debug is on)
LOG_DEBUG("%s: VOC state from sensor: [%u, %u, %u, %u, %u, %u, %u, %u]", sensorName, vocState[0], vocState[1], vocState[2],
@@ -427,6 +431,7 @@ bool SEN5XSensor::loadState()
return okay;
#else
LOG_ERROR("%s: Filesystem not implemented", sensorName);
return false;
#endif
}
@@ -472,6 +477,7 @@ bool SEN5XSensor::saveState()
return okay;
#else
LOG_ERROR("%s: Filesystem not implemented", sensorName);
return false;
#endif
}
@@ -497,8 +503,7 @@ uint32_t SEN5XSensor::wakeUp()
// keep track of how long it has passed
pmMeasureStarted = getTime();
state = SEN5X_MEASUREMENT;
if (state == SEN5X_MEASUREMENT)
LOG_INFO("%s: Started measurement mode", sensorName);
LOG_INFO("%s: Started measurement mode", sensorName);
return SEN5X_WARMUP_MS_1;
}
@@ -533,7 +538,7 @@ bool SEN5XSensor::startCleaning()
// This message will be always printed so the user knows the device it's not hung
LOG_INFO("%s: Started fan cleaning (10 sec)", sensorName);
uint16_t started = millis();
uint32_t started = millis();
while (millis() - started < 10500) {
delay(500);
}
@@ -658,9 +663,9 @@ bool SEN5XSensor::readValues()
LOG_TRACE("%s: Reading PM Values", sensorName);
delay(20); // From Sensirion Datasheet
uint8_t dataBuffer[16]{};
size_t receivedNumber = readBuffer(&dataBuffer[0], 24);
if (receivedNumber == 0) {
uint8_t dataBuffer[SEN5X_READ_VALUES_BUFFER_SIZE]{};
size_t receivedNumber = readBuffer(&dataBuffer[0], SEN5X_READ_VALUES_BUFFER_SIZE + (SEN5X_READ_VALUES_BUFFER_SIZE / 2));
if (receivedNumber < SEN5X_READ_VALUES_BUFFER_SIZE) {
LOG_ERROR("%s: Error getting values", sensorName);
return false;
}
@@ -676,15 +681,17 @@ bool SEN5XSensor::readValues()
int16_t int_vocIndex = static_cast<int16_t>((dataBuffer[12] << 8) | dataBuffer[13]);
int16_t int_noxIndex = static_cast<int16_t>((dataBuffer[14] << 8) | dataBuffer[15]);
// Convert values based on Sensirion Arduino lib
sen5xmeasurement.pM1p0 = !isnan(uint_pM1p0) ? uint_pM1p0 / 10 : UINT16_MAX;
sen5xmeasurement.pM2p5 = !isnan(uint_pM2p5) ? uint_pM2p5 / 10 : UINT16_MAX;
sen5xmeasurement.pM4p0 = !isnan(uint_pM4p0) ? uint_pM4p0 / 10 : UINT16_MAX;
sen5xmeasurement.pM10p0 = !isnan(uint_pM10p0) ? uint_pM10p0 / 10 : UINT16_MAX;
sen5xmeasurement.humidity = !isnan(int_humidity) ? int_humidity / 100.0f : FLT_MAX;
sen5xmeasurement.temperature = !isnan(int_temperature) ? int_temperature / 200.0f : FLT_MAX;
sen5xmeasurement.vocIndex = !isnan(int_vocIndex) ? int_vocIndex / 10.0f : FLT_MAX;
sen5xmeasurement.noxIndex = !isnan(int_noxIndex) ? int_noxIndex / 10.0f : FLT_MAX;
// Convert values based on Sensirion Arduino lib.
// Map values the sensor reports as unavailable (SEN5X_UINT_INVALID /
// SEN5X_INT_INVALID) to the sentinels getMetrics() checks for
sen5xmeasurement.pM1p0 = (uint_pM1p0 != SEN5X_UINT_INVALID) ? (uint_pM1p0 / 10) : UINT16_MAX;
sen5xmeasurement.pM2p5 = (uint_pM2p5 != SEN5X_UINT_INVALID) ? (uint_pM2p5 / 10) : UINT16_MAX;
sen5xmeasurement.pM4p0 = (uint_pM4p0 != SEN5X_UINT_INVALID) ? (uint_pM4p0 / 10) : UINT16_MAX;
sen5xmeasurement.pM10p0 = (uint_pM10p0 != SEN5X_UINT_INVALID) ? (uint_pM10p0 / 10) : UINT16_MAX;
sen5xmeasurement.humidity = (int_humidity != SEN5X_INT_INVALID) ? (int_humidity / 100.0f) : FLT_MAX;
sen5xmeasurement.temperature = (int_temperature != SEN5X_INT_INVALID) ? (int_temperature / 200.0f) : FLT_MAX;
sen5xmeasurement.vocIndex = (int_vocIndex != SEN5X_INT_INVALID) ? (int_vocIndex / 10.0f) : FLT_MAX;
sen5xmeasurement.noxIndex = (int_noxIndex != SEN5X_INT_INVALID) ? (int_noxIndex / 10.0f) : FLT_MAX;
LOG_TRACE("%s: Got readings: pM1p0=%u, pM2p5=%u, pM4p0=%u, pM10p0=%u", sensorName, sen5xmeasurement.pM1p0,
sen5xmeasurement.pM2p5, sen5xmeasurement.pM4p0, sen5xmeasurement.pM10p0);
@@ -711,9 +718,9 @@ bool SEN5XSensor::readPNValues(bool cumulative)
LOG_TRACE("%s: Reading PN Values", sensorName);
delay(20); // From Sensirion Datasheet
uint8_t dataBuffer[20]{};
size_t receivedNumber = readBuffer(&dataBuffer[0], 30);
if (receivedNumber == 0) {
uint8_t dataBuffer[SEN5X_READ_PM_BUFFER_SIZE]{};
size_t receivedNumber = readBuffer(&dataBuffer[0], SEN5X_READ_PM_BUFFER_SIZE + (SEN5X_READ_PM_BUFFER_SIZE / 2));
if (receivedNumber < SEN5X_READ_PM_BUFFER_SIZE) {
LOG_ERROR("%s: Error getting PN values", sensorName);
return false;
}
@@ -730,22 +737,29 @@ bool SEN5XSensor::readPNValues(bool cumulative)
uint16_t uint_pN10p0 = static_cast<uint16_t>((dataBuffer[16] << 8) | dataBuffer[17]);
uint16_t uint_tSize = static_cast<uint16_t>((dataBuffer[18] << 8) | dataBuffer[19]);
// Convert values based on Sensirion Arduino lib
// Multiply by 100 for converting from #/cm3 to #/0.1l for PN values
sen5xmeasurement.pN0p5 = !isnan(uint_pN0p5) ? uint_pN0p5 / 10 * 100 : UINT32_MAX;
sen5xmeasurement.pN1p0 = !isnan(uint_pN1p0) ? uint_pN1p0 / 10 * 100 : UINT32_MAX;
sen5xmeasurement.pN2p5 = !isnan(uint_pN2p5) ? uint_pN2p5 / 10 * 100 : UINT32_MAX;
sen5xmeasurement.pN4p0 = !isnan(uint_pN4p0) ? uint_pN4p0 / 10 * 100 : UINT32_MAX;
sen5xmeasurement.pN10p0 = !isnan(uint_pN10p0) ? uint_pN10p0 / 10 * 100 : UINT32_MAX;
sen5xmeasurement.tSize = !isnan(uint_tSize) ? uint_tSize / 1000.0f : FLT_MAX;
// Convert values based on Sensirion Arduino lib.
// Raw PN values are #/cm3 with 0.1 resolution; multiplying by 10
// converts to #/0.1l without the truncation of dividing first.
// Map values the sensor reports as unavailable (SEN5X_UINT_INVALID) to the
// sentinels getMetrics() checks for
sen5xmeasurement.pN0p5 = (uint_pN0p5 != SEN5X_UINT_INVALID) ? ((uint32_t)uint_pN0p5 * 10) : UINT32_MAX;
sen5xmeasurement.pN1p0 = (uint_pN1p0 != SEN5X_UINT_INVALID) ? ((uint32_t)uint_pN1p0 * 10) : UINT32_MAX;
sen5xmeasurement.pN2p5 = (uint_pN2p5 != SEN5X_UINT_INVALID) ? ((uint32_t)uint_pN2p5 * 10) : UINT32_MAX;
sen5xmeasurement.pN4p0 = (uint_pN4p0 != SEN5X_UINT_INVALID) ? ((uint32_t)uint_pN4p0 * 10) : UINT32_MAX;
sen5xmeasurement.pN10p0 = (uint_pN10p0 != SEN5X_UINT_INVALID) ? ((uint32_t)uint_pN10p0 * 10) : UINT32_MAX;
sen5xmeasurement.tSize = (uint_tSize != SEN5X_UINT_INVALID) ? (uint_tSize / 1000.0f) : FLT_MAX;
// Remove accumuluative values:
// https://github.com/fablabbcn/smartcitizen-kit-2x/issues/85
if (!cumulative) {
sen5xmeasurement.pN10p0 -= sen5xmeasurement.pN4p0;
sen5xmeasurement.pN4p0 -= sen5xmeasurement.pN2p5;
sen5xmeasurement.pN2p5 -= sen5xmeasurement.pN1p0;
sen5xmeasurement.pN1p0 -= sen5xmeasurement.pN0p5;
if (sen5xmeasurement.pN10p0 != UINT32_MAX && sen5xmeasurement.pN4p0 != UINT32_MAX)
sen5xmeasurement.pN10p0 -= sen5xmeasurement.pN4p0;
if (sen5xmeasurement.pN4p0 != UINT32_MAX && sen5xmeasurement.pN2p5 != UINT32_MAX)
sen5xmeasurement.pN4p0 -= sen5xmeasurement.pN2p5;
if (sen5xmeasurement.pN2p5 != UINT32_MAX && sen5xmeasurement.pN1p0 != UINT32_MAX)
sen5xmeasurement.pN2p5 -= sen5xmeasurement.pN1p0;
if (sen5xmeasurement.pN1p0 != UINT32_MAX && sen5xmeasurement.pN0p5 != UINT32_MAX)
sen5xmeasurement.pN1p0 -= sen5xmeasurement.pN0p5;
}
LOG_TRACE("%s: Got readings: pN0p5=%u, pN1p0=%u, pN2p5=%u, pN4p0=%u, pN10p0=%u, tSize=%.2f", sensorName,
@@ -767,10 +781,10 @@ uint8_t SEN5XSensor::getMeasurements()
}
delay(20); // From Sensirion Datasheet
uint8_t dataReadyBuffer[3];
size_t charNumber = readBuffer(&dataReadyBuffer[0], 3);
if (charNumber == 0) {
LOG_ERROR("%s: Error getting device version value", sensorName);
uint8_t dataReadyBuffer[SEN5X_DATA_READY_BUFFER_SIZE]{};
size_t charNumber = readBuffer(&dataReadyBuffer[0], SEN5X_DATA_READY_BUFFER_SIZE + (SEN5X_DATA_READY_BUFFER_SIZE / 2));
if (charNumber < SEN5X_DATA_READY_BUFFER_SIZE) {
LOG_ERROR("%s: Error getting data ready flag value", sensorName);
return 2;
}
@@ -909,7 +923,7 @@ bool SEN5XSensor::getMetrics(meshtastic_Telemetry *measurement)
measurement->variant.air_quality_metrics.has_pm_temperature = true;
measurement->variant.air_quality_metrics.pm_temperature = sen5xmeasurement.temperature;
}
if (sen5xmeasurement.noxIndex != FLT_MAX) {
if (sen5xmeasurement.vocIndex != FLT_MAX) {
measurement->variant.air_quality_metrics.has_pm_voc_idx = true;
measurement->variant.air_quality_metrics.pm_voc_idx = sen5xmeasurement.vocIndex;
}
+28 -1
View File
@@ -86,6 +86,18 @@ class SEN5XSensor : public TelemetrySensor
#define SEN5X_READ_RAW_VALUES 0x03D2
#define SEN5X_READ_PM_VALUES 0x0413
// Values the sensor reports when a reading is unavailable
#define SEN5X_UINT_INVALID 0xFFFF
#define SEN5X_INT_INVALID 0x7FFF
// Reply payload sizes in data bytes; the raw I2C transfer adds one CRC byte
// per 2-byte group, so requests are <size> + <size> / 2 raw bytes
#define SEN5X_VERSION_BUFFER_SIZE 8
#define SEN5X_PRODUCT_NAME_BUFFER_SIZE 32
#define SEN5X_DATA_READY_BUFFER_SIZE 2
#define SEN5X_READ_VALUES_BUFFER_SIZE 16
#define SEN5X_READ_PM_BUFFER_SIZE 20
#define SEN5X_VOC_VALID_TIME 600
#define SEN5X_VOC_VALID_DATE 1514764800
@@ -114,8 +126,23 @@ See: https://sensirion.com/resource/application_note/low_power_mode/sen5x
#define SEN5X_PN4P0_CONC_THD 100
bool sendCommand(uint16_t command);
/**
* @brief Send a command word followed by a data payload; a CRC byte is
* computed and inserted on the wire after every 2-byte pair.
* @param command 16-bit command code, sent big-endian
* @param buffer payload data bytes, without CRCs
* @param byteNumber payload size in data bytes; must be even
* @return true when the full transfer is written and acknowledged
*/
bool sendCommand(uint16_t command, uint8_t *buffer, uint8_t byteNumber = 0);
uint8_t readBuffer(uint8_t *buffer, uint8_t byteNumber); // Return number of bytes received
/**
* @brief Read a reply, verifying and stripping the interleaved CRC bytes.
* @param buffer destination for the data bytes (byteNumber * 2 / 3 of them)
* @param byteNumber raw transfer size including CRCs; must be a multiple
* of 3 (2 data bytes + 1 CRC per group)
* @return the number of data bytes written to buffer, or 0 on any error
*/
uint8_t readBuffer(uint8_t *buffer, uint8_t byteNumber);
uint8_t sen5xCRC(const uint8_t *buffer);
bool startCleaning();
uint8_t getMeasurements();