Feat: Support graceful fallback target (TIMEOUT_FALLBACK_W) - #560
Open
thkrmr wants to merge 2 commits into
Open
Conversation
…eter failure - Added TIMEOUT_FALLBACK_W config for CT002 to gracefully step down battery output during meter outages rather than freezing on stale data. - Updated ThrottledPowermeter to conditionally bypass its cache on failure when a fallback is configured, allowing the CT002 control loop to activate. - Improved test_powermeter to not fail daemon startup if a fallback is configured, enabling AstraMeter to boot during an ongoing outage. - Silenced expected traceback spam in fallback mode caused by the custom _AutoExcInfoFilter. - Increased Tibber Pulse HTTP timeout from 2s to 5s to avoid false-positive fallback triggers during momentary WiFi slowdowns.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe PR adds configurable CT002 fallback output, propagates powermeter failures through wrappers, allows startup with fallback configured, returns fallback readings during cloud gathering, publishes fallback health state, and lengthens Tibber Pulse connection timeouts. ChangesPowermeter fallback handling
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CloudGather
participant ThrottledPowermeter
participant CT002
participant MQTTHealth
CloudGather->>ThrottledPowermeter: read raw powermeter watts
ThrottledPowermeter-->>CloudGather: propagate read failure
CloudGather->>CT002: use configured fallback target
CT002-->>CloudGather: provide fallback phase values
CloudGather->>MQTTHealth: publish fallback_active state
Possibly related PRs
Suggested reviewers: 🚥 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: 6
🤖 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/astrameter/ct002/ct002.py`:
- Line 1071: Change the per-request fallback log in the meter polling path to
use debug level instead of warning, while preserving its existing message and
arguments. Leave the rate-limited fallback activation warning in
_call_before_send unchanged.
- Around line 1062-1074: Update the ESPHome CT002 meter-timeout handling in the
relevant control/update method to support timeout_fallback_w like the Python
CT002 implementation: when configured, calculate the delta from the reported
power to the fallback target and apply it across all three phase values;
otherwise preserve the existing zero-adjustment behavior. Keep the fallback
target and reported-power semantics consistent with the Python path.
- Around line 1062-1074: Update the timeout fallback logic in the CT002 handling
block to use reported_phase when constructing values: place the full delta_w in
the battery’s reported phase field and set the other phase fields to zero for
per-phase batteries, while preserving the existing combined (“D”) behavior that
distributes the delta across all three fields. Keep the fallback target and
warning values unchanged.
In `@src/astrameter/main.py`:
- Around line 557-588: The control-loop fallback state is not reaching the MQTT
health publisher when cloud reporting is disabled. Update the state propagation
around the visible powermeter read/failure handling and the MQTT health
publisher so `fallback_active` uses the control-loop’s `chosen.in_fallback_mode`
state, preserving true during CT002 fallback and false after a successful read.
- Around line 694-702: The startup test currently applies one global
has_fallback value to every powermeter. Derive the TIMEOUT_FALLBACK_W setting
for each powermeter’s associated configuration, carry that per-meter fallback
flag through the powermeters iteration, and pass it as allow_failure to
test_powermeter so unrelated meters retain strict failure behavior and accurate
logging.
- Around line 263-280: The bypass_cache_on_error update in run_device must only
affect meter wrappers belonging to the current device, not every entry in the
shared powermeters collection. Filter powermeters to this device’s matching
client or construct its per-device wrapper chain, then set bypass_cache_on_error
only on wrappers in that chain while preserving the existing timeout_fallback_w
condition.
🪄 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: 4bcc3553-7418-458d-99c0-be181a116d5f
📒 Files selected for processing (5)
src/astrameter/ct002/ct002.pysrc/astrameter/main.pysrc/astrameter/mqtt_insights/service.pysrc/astrameter/powermeter/tibber_pulse.pysrc/astrameter/powermeter/wrappers/throttling.py
… delta, improve fallback logging - Demote per-poll fallback delta log to debug; keep rate-limited ENGAGED/DISENGAGED warnings at WARNING level for alerting. - Fix per-phase battery delta: place full delta_w in the correct phase field instead of splitting across all three. - Scope bypass_cache_on_error lazily in read_ct_powermeter instead of eagerly on all wrappers, so unrelated powermeters keep caching. - Clarify intentional broad scope of has_fallback for startup test.
Owner
|
@thkrmr Can you provide more context on what problem you're solving with this change? |
Author
|
@tomquist My Pulse IR loses connection frequently as of recent (closed distribution box, in a separate room), at times it also fails to deliver a valid SML (I pasted the logs in an issue) but doesn't lose its connection. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Feat: Support graceful fallback target (TIMEOUT_FALLBACK_W) on powermeter failure
Summary by CodeRabbit