Add HM330X PM Sensor#11026
Conversation
📝 WalkthroughWalkthroughAdds Seeed HM330X air-quality sensor support, including its dependency, I2C discovery, telemetry registration, warmup lifecycle, checksum validation, and PM10/PM2.5/PM100 metric reporting. ChangesHM330X sensor integration
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant I2CScanner
participant HM330XSensor
participant TelemetryModule
I2CScanner->>I2CScanner: Probe HM330X at 0x40
I2CScanner->>TelemetryModule: Register detected HM330X
TelemetryModule->>HM330XSensor: Initialize and wake sensor
HM330XSensor-->>TelemetryModule: Return PM10, PM2.5, and PM100 metrics
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
oscgonfer
added
the
hardware-support
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/modules/Telemetry/Sensor/HM330XSensor.h (1)
27-27: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove unimplemented declaration.
This
checksummethod is declared but never implemented or used. The implementation useshm330x.checksum_calcfrom the library instead.♻️ Proposed fix
- HM330XErrorCode checksum(uint8_t* data);🤖 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/HM330XSensor.h` at line 27, Remove the unused, unimplemented checksum declaration from the HM330X sensor interface. Keep checksum handling through the existing hm330x.checksum_calc library call and do not add a replacement method.src/modules/Telemetry/Sensor/HM330XSensor.cpp (2)
48-52: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
Throttlefor the warmup interval.Replace the
getTime()subtraction and unit conversion with the repository’sThrottlehelper to avoid duplicated elapsed-time logic and unit ambiguity.As per coding guidelines, “Use
Throttlefromsrc/mesh/Throttle.hfor time-based rate limiting and elapsed-time checks.”Also applies to: 71-80
🤖 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/HM330XSensor.cpp` around lines 48 - 52, Update HM330XSensor::wakeUp and the related warmup timing logic to use the repository’s Throttle helper from src/mesh/Throttle.h instead of manual getTime subtraction and unit conversion. Preserve the existing warmup interval behavior while routing elapsed-time checks through Throttle.Source: Coding guidelines
117-119: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCorrect and shorten this particulate-size comment.
For cumulative mass concentrations, PM1.0 is normally less than or equal to PM2.5, which is less than or equal to PM10. The current comment states the inverse and exceeds the two-line comment limit.
As per coding guidelines, “Keep comments minimal—one or two lines maximum.”
Proposed change
- // TODO - This is suspiciously not in terms of PM1, PM2.5 and PM10.0 relationship - // Normally, PM1.0 > PM2.5 > PM10, but in this case it's not. Are the buckets cumulative? - // It's not documented in the datasheet, so safely assuming that it's correct as in the datasheet + // HM330X reports cumulative PM1.0, PM2.5, and PM10 standard concentrations.🤖 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/HM330XSensor.cpp` around lines 117 - 119, Replace the three-line comment near the HM330X particulate readings with a concise one- or two-line comment stating that cumulative mass concentrations should satisfy PM1.0 ≤ PM2.5 ≤ PM10 and that the datasheet-defined bucket behavior is being followed.Source: Coding guidelines
🤖 Prompt for all review comments with 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.
Inline comments:
In `@src/modules/Telemetry/Sensor/HM330XSensor.cpp`:
- Around line 60-64: Update HM330XSensor::canSleep() to return true so
AirQualityTelemetryModule applies wake-up and pendingForReadyMs readiness
tracking before reading the sensor. Keep sleep() as a no-op if necessary, but do
not change the readiness flow.
---
Nitpick comments:
In `@src/modules/Telemetry/Sensor/HM330XSensor.cpp`:
- Around line 48-52: Update HM330XSensor::wakeUp and the related warmup timing
logic to use the repository’s Throttle helper from src/mesh/Throttle.h instead
of manual getTime subtraction and unit conversion. Preserve the existing warmup
interval behavior while routing elapsed-time checks through Throttle.
- Around line 117-119: Replace the three-line comment near the HM330X
particulate readings with a concise one- or two-line comment stating that
cumulative mass concentrations should satisfy PM1.0 ≤ PM2.5 ≤ PM10 and that the
datasheet-defined bucket behavior is being followed.
In `@src/modules/Telemetry/Sensor/HM330XSensor.h`:
- Line 27: Remove the unused, unimplemented checksum declaration from the HM330X
sensor interface. Keep checksum handling through the existing
hm330x.checksum_calc library call and do not add a replacement method.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5320e741-8f0a-4b68-8d73-dfa8396c3fe7
📒 Files selected for processing (7)
platformio.inisrc/configuration.hsrc/detect/ScanI2C.hsrc/detect/ScanI2CTwoWire.cppsrc/modules/Telemetry/AirQualityTelemetry.cppsrc/modules/Telemetry/Sensor/HM330XSensor.cppsrc/modules/Telemetry/Sensor/HM330XSensor.h
| bool HM330XSensor::canSleep() | ||
| { | ||
| // Sleep not available in this sensor | ||
| return false; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the base lifecycle contract and comparable air-quality sensors.
fd -t f 'TelemetrySensor|Sensor' src/modules/Telemetry |
xargs rg -n -C3 '\b(canSleep|wakeUp|pendingForReadyMs|isActive)\s*\('Repository: meshtastic/firmware
Length of output: 20253
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- HM330XSensor.cpp ---'
sed -n '1,220p' src/modules/Telemetry/Sensor/HM330XSensor.cpp
echo
echo '--- TelemetrySensor.h ---'
sed -n '1,140p' src/modules/Telemetry/Sensor/TelemetrySensor.h
echo
echo '--- AirQualityTelemetryModule.cpp ---'
sed -n '1,260p' src/modules/Telemetry/AirQualityTelemetryModule.cppRepository: meshtastic/firmware
Length of output: 8147
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline src/modules/Telemetry/AirQualityTelemetryModule.cpp --view expanded || true
echo
rg -n -C4 'canSleep|wakeUp|pendingForReadyMs|isActive|getMetrics' src/modules/Telemetry/AirQualityTelemetryModule.cpp src/modules/Telemetry/Sensor/*.cppRepository: meshtastic/firmware
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -t f 'AirQualityTelemetryModule.cpp' src/modules/Telemetry
sed -n '1,260p' src/modules/Telemetry/AirQualityTelemetryModule.cppRepository: meshtastic/firmware
Length of output: 252
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C4 'pendingForReadyMs\(|wakeUpTimeMs\(|canSleep\(\)|isActive\(\)|sleep\(\)|wakeUp\(\)' src/modules/Telemetry src/modules | sed -n '1,260p'Repository: meshtastic/firmware
Length of output: 19542
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- HM330XSensor.h ---'
sed -n '1,160p' src/modules/Telemetry/Sensor/HM330XSensor.h
echo
echo '--- AirQualityTelemetry.cpp (top/middle) ---'
sed -n '1,260p' src/modules/Telemetry/AirQualityTelemetry.cpp
echo
echo '--- AirQualityTelemetry.cpp (rest) ---'
sed -n '261,520p' src/modules/Telemetry/AirQualityTelemetry.cppRepository: meshtastic/firmware
Length of output: 22122
Keep canSleep() true for HM330X AirQualityTelemetryModule only calls wakeUp() and pendingForReadyMs() when this returns true, so returning false skips the warmup gate and the sensor is read immediately. Leave sleep() as a no-op if needed, but don’t block readiness tracking.
🤖 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/HM330XSensor.cpp` around lines 60 - 64, Update
HM330XSensor::canSleep() to return true so AirQualityTelemetryModule applies
wake-up and pendingForReadyMs readiness tracking before reading the sensor. Keep
sleep() as a no-op if necessary, but do not change the readiness flow.
⚡ Try this PR in the Web FlasherNote Building this pull request… the flash button, badges and supported-board |
|
Caution Review failedAn error occurred during the review process. Please try again later. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
1 participant
This is a PR for the FAB26 event in Boston using a HM330X (datasheet).
Tested locally with a XIAO ESP32-S3 and working properly.
Caution
I'd wait until tomorrow (17/07/2026) for confirming that it works well with "at home testing".
Summary by CodeRabbit