Add SEN54/NOX detection to AIR-1 factory test#113
Conversation
The SEN5x component drops the NOX sensor when the chip reports itself as a SEN54, so a mislabeled or wrong-part unit ships with no NOX data and nothing in the factory test catches it. testScript now waits up to 30s for a NOX reading after the DPS310 check and only lights the LED green when NOX reports a value. A unit whose NOX stays NaN flashes red and logs "SEN55 is reporting as SEN54" for the flash station. Bumps version to 26.7.15.1. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
WalkthroughESPHome Core updates its version substitution and extends the device self-test with a NOX sensor validation. The passed LED now requires both DPS310 and NOX checks to succeed. ChangesESPHome self-test
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![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
🤖 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 `@Integrations/ESPHome/Core.yaml`:
- Around line 606-613: Increase the timeout in the SEN54/SEN55 NOX wait_until
block from 30 seconds to at least 45–50 seconds so testScript accommodates the
sensor’s documented warm-up period. Keep the existing
!isnan(id(sen55_nox).state) condition unchanged.
🪄 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
Run ID: eb3aa062-1644-4f1b-871b-ac178e3114c5
📒 Files selected for processing (1)
Integrations/ESPHome/Core.yaml
| # SEN54 detection: when the SEN5x chip reports itself as a SEN54 the | ||
| # component nulls the NOX sensor, so its state stays NaN forever. A | ||
| # healthy SEN55 publishes a NOX index within a couple of update cycles, | ||
| # so wait up to 30s for a value before deciding. | ||
| - wait_until: | ||
| condition: | ||
| - lambda: "return !isnan(id(sen55_nox).state);" | ||
| timeout: 30s |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Increase NOX timeout to account for the sensor's warm-up period.
The SEN55 datasheet specifies that both VOC and NOX index values require a 45-second warm-up period after power-on before they return valid data (outputting 0x7FFF, translated to NaN by ESPHome, in the meantime).
Since testScript runs on boot (via AIR-1_Factory.yaml), the device will have been powered on for just a few seconds. The maximum 5-second wait for the DPS310 plus this 30-second NOX timeout totals around 35 seconds. This falls short of the 45-second hardware warm-up time, meaning the factory test will likely time out and falsely fail healthy SEN55 units.
Please consider increasing the timeout to at least 45-50 seconds.
🕒 Proposed timeout fix
- wait_until:
condition:
- lambda: "return !isnan(id(sen55_nox).state);"
- timeout: 30s
+ timeout: 45s📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # SEN54 detection: when the SEN5x chip reports itself as a SEN54 the | |
| # component nulls the NOX sensor, so its state stays NaN forever. A | |
| # healthy SEN55 publishes a NOX index within a couple of update cycles, | |
| # so wait up to 30s for a value before deciding. | |
| - wait_until: | |
| condition: | |
| - lambda: "return !isnan(id(sen55_nox).state);" | |
| timeout: 30s | |
| # SEN54 detection: when the SEN5x chip reports itself as a SEN54 the | |
| # component nulls the NOX sensor, so its state stays NaN forever. A | |
| # healthy SEN55 publishes a NOX index within a couple of update cycles, | |
| # so wait up to 30s for a value before deciding. | |
| - wait_until: | |
| condition: | |
| - lambda: "return !isnan(id(sen55_nox).state);" | |
| timeout: 45s |
🤖 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 `@Integrations/ESPHome/Core.yaml` around lines 606 - 613, Increase the timeout
in the SEN54/SEN55 NOX wait_until block from 30 seconds to at least 45–50
seconds so testScript accommodates the sensor’s documented warm-up period. Keep
the existing !isnan(id(sen55_nox).state) condition unchanged.
1 participant
Version: 26.7.15.1
What does this implement/fix?
A customer's AIR-1 showed no NOX value. Logs:
The SEN5x component reads the product-name register straight off the chip.
When it reads "SEN54" it nulls the NOX sensor, so NOX never publishes. That
means a mislabeled or wrong-part sensor ships with no NOX and the factory
test never catches it (testScript only checked DPS310 pressure).
testScript now also validates NOX after the DPS310 loop:
reporting itself as one) leaves it NaN forever; a healthy SEN55 publishes
a NOX index within a couple of update cycles. The wait returns as soon as
a value arrives, so a good unit isn't slowed down.
flash station can pull the unit.
AIR-1 is the only Apollo product with a SEN55, so no other repos are affected.
Types of changes
Checklist / Checklijst:
If user-visible functionality or configuration variables are added/modified:
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Maintenance