[apollo_msr-2] Add configurable DPS310 pressure offset#62
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdded a new ESPHome template sensor Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
|
Beta |
c3ab284 to
796b4f9
Compare
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Integrations/ESPHome/Core.yaml`:
- Around line 361-364: The temperature lambda is incorrectly adding
id(dps310_pressure_offset).state (hPa) to the ESP internal temperature via the
variable current_value; remove that addition from the ESP temperature sensor's
lambda so the pressure offset is not applied to temperature. Locate the code
using current_value and id(dps310_pressure_offset) in the ESP temperature sensor
block and delete the offset application (or wrap it so it only runs in the
DPS310 pressure sensor lambda). Ensure the DPS310 pressure sensor's lambda
explicitly reads id(dps310_pressure_offset).state and applies it to pressure
values only.
- Around line 609-612: The pressure offset (id(dps310_pressure_offset)) is being
added to the LTR390 UV Index sensor via current_value, corrupting UV readings;
remove the addition of id(dps310_pressure_offset).state from the LTR390 sensor
code (the block that references current_value and id(dps310_pressure_offset)) so
the offset is only used in the DPS310 pressure sensor logic, not in the LTR390
UV Index handling.
- Around line 582-585: The LTR390 light sensor handler is incorrectly applying
the DPS310 pressure offset: remove the block that reads
id(dps310_pressure_offset).state and adds it to current_value inside the
LTR390/LTR390 update logic so lux readings are not modified; ensure the
dps310_pressure_offset is only read and applied in the DPS310 pressure sensor
code (look for references to id(dps310_pressure_offset), current_value, and the
LTR390 update function) and keep the offset application limited to the DPS310
pressure processing path.
|
@bharvey88 CodeRabbit found some bugs |
dps310_pressure_offset was incorrectly applied to three sensors that have no relation to barometric pressure: ESP internal temperature, LTR390 light, and LTR390 UV Index. The offset is now applied only in the DPS310 pressure sensor lambda where it belongs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
What does this implement/fix?
Adds a user-configurable DPS310 Pressure Offset (CONFIG, -100 to +100 hPa, 0.1 step, persists across reboots). The offset is applied in the existing pressure filter lambda before the delta-reporting check, with an isnan() guard for safety during startup.
Types of changes
Summary by CodeRabbit