Enable CO2 auto calibration by default with toggle to disable#97
Enable CO2 auto calibration by default with toggle to disable#97bharvey88 wants to merge 2 commits into
Conversation
|
Warning Review limit reached
More reviews will be available in 21 minutes and 21 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughESPHome configuration upgraded to version 26.6.10.1 with SCD4x CO2 sensor automatic self-calibration enabled by default. New user-facing switch allows runtime toggle of calibration via a dedicated restart-mode script that manages boot delay, measurement state, calibration application, and command sequencing. ChangesCO2 Auto-Calibration Feature
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.
🧹 Nitpick comments (3)
Integrations/ESPHome/Core.yaml (3)
505-516: ⚡ Quick winAdd error checking for start/stop measurement commands.
The script checks the return value of
write_commandfor the calibration setting (line 509) but not for stop (line 505) or start (line 516) periodic measurement. If these commands fail, the sensor could be left in an inconsistent state (e.g., stopped but not restarted).🛡️ Proposed error checking
- lambda: |- - id(scd40).write_command((uint16_t) 0x3F86); //stop periodic measurement + if (!id(scd40).write_command((uint16_t) 0x3F86)) { + ESP_LOGE("Apollo", "Failed to stop CO2 periodic measurement"); + return; // Abort if we can't stop measurement + } - delay: 500ms - lambda: |- id(scd40).set_automatic_self_calibration(enable); if (!id(scd40).write_command((uint16_t) 0x2416, (uint16_t) (enable ? 1 : 0))) { ESP_LOGE("Apollo", "Failed to set CO2 auto calibration"); + // Still try to restart measurement even if calibration failed } else { ESP_LOGI("Apollo", "CO2 auto calibration %s", enable ? "enabled" : "disabled"); } - delay: 10ms - lambda: |- - id(scd40).write_command((uint16_t) 0x21B1); //start periodic measurement + if (!id(scd40).write_command((uint16_t) 0x21B1)) { + ESP_LOGE("Apollo", "Failed to restart CO2 periodic measurement"); + }Note: Even if calibration fails, the measurement should be restarted to avoid leaving the sensor in a stopped state.
🤖 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 505 - 516, The stop/start measurement commands (id(scd40).write_command with 0x3F86 and 0x21B1) currently lack error checking; update the lambdas that call id(scd40).write_command((uint16_t)0x3F86) and id(scd40).write_command((uint16_t)0x21B1) to check their boolean return values and log failures via ESP_LOGE (include a clear message like "Failed to stop periodic measurement" / "Failed to start periodic measurement"); ensure you still attempt to start measurements after attempting calibration even if earlier write_command calls fail so the sensor is not left stopped.
469-485: ⚡ Quick winConsider adding error feedback for calibration failures.
The switch uses
optimistic: true, so it updates immediately when toggled. If thesetCo2AutoCalibrationscript fails (I2C error, sensor not responding), the switch state won't reflect the actual sensor configuration.♻️ Proposed enhancement to sync state on error
Modify the script to return a status value and update the switch handlers to revert state on failure:
- platform: template name: "CO2 Auto Calibration" id: co2_auto_calibration icon: mdi:molecule-co2 restore_mode: RESTORE_DEFAULT_ON - optimistic: true + optimistic: false entity_category: "config" + lambda: |- + // Read actual state from sensor if possible, or use switch state + return id(co2_auto_calibration).state; on_turn_on: then: - script.execute: id: setCo2AutoCalibration enable: true + - if: + condition: + lambda: "return id(setCo2AutoCalibration).is_running();" + then: + - script.wait: setCo2AutoCalibration on_turn_off: then: - script.execute: id: setCo2AutoCalibration enable: false + - if: + condition: + lambda: "return id(setCo2AutoCalibration).is_running();" + then: + - script.wait: setCo2AutoCalibrationNote: ESPHome doesn't provide a direct way to read the ASC setting back from the sensor, so perfect synchronization isn't possible. At minimum, consider logging prominently when the write_command fails so users can investigate.
🤖 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 469 - 485, The CO2 Auto Calibration switch (platform: template, name: "CO2 Auto Calibration", id: co2_auto_calibration) is optimistic and can get out of sync if the script setCo2AutoCalibration fails; update setCo2AutoCalibration to return a status (success/failure) and add explicit error logging inside that script when the sensor write_command fails, then change the switch's on_turn_on/on_turn_off handlers to check the returned status and, on failure, revert the switch state (e.g., call switch.turn_off/turn_on or publish the opposite state) and log a prominent error so the UI reflects the real outcome.
496-500: 💤 Low valueBoot delay could wait longer than necessary.
The current logic checks if
millis() < 20000and then delays for 20s. If the script runs early (e.g., at 5s after boot), the total wait is 25s. If it runs at 19s, the wait is 39s.♻️ More precise delay calculation
- if: condition: lambda: "return millis() < 20000;" then: - - delay: 20s + - lambda: |- + uint32_t wait_ms = 20000 - millis(); + delay(wait_ms);Or use a simpler unconditional delay if the script is only called at boot:
- - if: - condition: - lambda: "return millis() < 20000;" - then: - - delay: 20s + - lambda: |- + if (millis() < 20000) { + delay(20000 - millis()); + }Note: The current approach is conservative and ensures at least 20s has passed, which may be preferred for robustness.
🤖 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 496 - 500, The current conditional uses the lambda "return millis() < 20000;" and then always applies a fixed "delay: 20s", which can overshoot; change the block so it delays only the remaining time until 20s since boot (e.g., compute remaining = 20000 - millis() and delay that many milliseconds/seconds) or, if this action always runs at boot, replace the conditional with a single unconditional "delay: 20s". Update the conditional branch that contains the lambda and the "delay: 20s" to implement the remaining-time calculation or the unconditional delay as appropriate.
🤖 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.
Nitpick comments:
In `@Integrations/ESPHome/Core.yaml`:
- Around line 505-516: The stop/start measurement commands
(id(scd40).write_command with 0x3F86 and 0x21B1) currently lack error checking;
update the lambdas that call id(scd40).write_command((uint16_t)0x3F86) and
id(scd40).write_command((uint16_t)0x21B1) to check their boolean return values
and log failures via ESP_LOGE (include a clear message like "Failed to stop
periodic measurement" / "Failed to start periodic measurement"); ensure you
still attempt to start measurements after attempting calibration even if earlier
write_command calls fail so the sensor is not left stopped.
- Around line 469-485: The CO2 Auto Calibration switch (platform: template,
name: "CO2 Auto Calibration", id: co2_auto_calibration) is optimistic and can
get out of sync if the script setCo2AutoCalibration fails; update
setCo2AutoCalibration to return a status (success/failure) and add explicit
error logging inside that script when the sensor write_command fails, then
change the switch's on_turn_on/on_turn_off handlers to check the returned status
and, on failure, revert the switch state (e.g., call switch.turn_off/turn_on or
publish the opposite state) and log a prominent error so the UI reflects the
real outcome.
- Around line 496-500: The current conditional uses the lambda "return millis()
< 20000;" and then always applies a fixed "delay: 20s", which can overshoot;
change the block so it delays only the remaining time until 20s since boot
(e.g., compute remaining = 20000 - millis() and delay that many
milliseconds/seconds) or, if this action always runs at boot, replace the
conditional with a single unconditional "delay: 20s". Update the conditional
branch that contains the lambda and the "delay: 20s" to implement the
remaining-time calculation or the unconditional delay as appropriate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7118d14d-bb94-4647-a777-24452958a39e
📒 Files selected for processing (1)
Integrations/ESPHome/Core.yaml
Enable the SCD40's automatic self-calibration by default (ESPHome default) so users no longer need to manually recalibrate every 1-2 years. Adds a "CO2 Auto Calibration" switch so users in spaces that never see fresh-air CO2 levels can turn ASC off and keep using the manual 420ppm calibration button instead. The switch re-applies the saved choice after every boot, since the sensor loses the setting on power loss and the scd4x component re-applies the YAML default during setup.
bharvey88
force-pushed
the
co2-asc-toggle
branch
from
01db027 to
ccbfe30
Compare
1 participant
Version: 26.6.10.1
What does this implement/fix?
Enables the SCD40's automatic self-calibration (ASC) by default and adds a "CO2 Auto Calibration" switch so users can turn it off.
write_command), and restarts measurement, the same sequence the component uses for forced calibration. The saved choice is re-applied ~20 seconds after every boot, since the SCD40 loses the setting on power loss and ESPHome re-applies the YAML default during setup.calibrate_co2_valueaction are unchanged and keep working whether ASC is on or off.Why a breaking change: existing devices get ASC turned on when they update. Readings may shift over the first days as the baseline self-corrects. Users whose device sits in a space that rarely gets fresh air (a sealed office, a basement, a grow room) should turn the new switch off and keep calibrating manually. Suggest including this in the release notes.
The same change is rolling out to MSR-2 and MTR-1 (identical), and R_PRO-1 (toggle fix/rename; its ASC was already on by default). A wiki update covering all four products is ready to merge alongside the firmware releases.
Verification:
esphome compilepasses for AIR-1.yaml, AIR-1_BLE.yaml, and AIR-1_Factory.yaml (ESPHome 2026.1.3). Tested on hardware.Types of changes
Checklist / Checklijst:
If user-visible functionality or configuration variables are added/modified:
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Chores