Don't fail the refresh when every entity has its own scan_interval - #276
Don't fail the refresh when every entity has its own scan_interval#276bprus wants to merge 2 commits into
scan_interval#276Conversation
`async_update` polls only entities without a `scan_interval`. A device
config that sets one on every entity leaves that list empty,
`_update_device` returns {}, and the falsy check raised `UpdateFailed`
on every single refresh. That sets last_update_success False, which
marks ALL of the integration's entities unavailable - including the ones
their own timers are polling perfectly well.
The symptom is confusing: entities come up fine and then drop together
about one refresh interval later, with nothing wrong on the bus.
An empty poll list is not a failure. Return the data the per-entity
timers have already stored instead of discarding it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughSummary by CodeRabbit
Walkthrough
ChangesCoordinator polling
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The PR changes empty coordinator polls to remain valid without marking entities unavailable, while preserving failures for non-empty polls. No actionable merge-blocking risk remains. Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
bprus
changed the title
scan_interval
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
1 participant
Hi! So this is my first PR here in a small series that will follow. The end goal is to add read-modify-write feature to this integration. I needed it to integrate my Midea-based heat pump.
I want to be upfront. I wrote the code with the assistance of Claude Code. I'm myself a programmer, so I tried to check all the PRs myself to the best of my capabilities and reduce the slop to minimum.
Problem
ModbusCoordinator.async_update()raisesUpdateFailedwhen the device returns no data. But thecoordinator only polls entities that have no
scan_intervalof their own — entities with oneare polled by their own timer.
So if every entity in a device config sets
scan_interval, the coordinator's poll list is empty,update_device()legitimately returns{}, andUpdateFailedis raised on every refresh. HomeAssistant then marks all the device's entities unavailable — including the ones whose own timers
are polling perfectly well.
The failure is confusing because the entities are being read successfully; only the coordinator's
own (empty) poll fails.
Fix
Treat an empty poll list as "nothing to do" rather than an error: return the existing data
(
self.data or {}) instead of raising.An empty result from a non-empty poll list still raises, so a genuinely failing device is still
reported.
Notes
Found on a device config where every entity had an explicit
scan_interval; it cost a hardware testrun before the cause was clear. Tests cover both cases — empty poll list (no raise) and a real
failure (still raises).