Add wait_for_next_message so CT002 serves fresh powermeter data#322
Merged
Conversation
The CT002's before_send callback now awaits wait_for_next_message() before reading the powermeter, ensuring push-based meters (MQTT, SMA, HomeWizard, Home Assistant) deliver a fresh measurement for every battery request instead of re-serving stale cached values.
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 22 minutes and 10 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughA new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/astrameter/powermeter/mqtt_test.py (1)
103-125: Good baseline tests; add a multi-topic cold-start case.Current coverage is single-topic only. Please add a regression test where subscriptions are multi-topic and only one topic updates first, to lock down expected
wait_for_next_messagebehavior beforeget_powermeter_watts().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/astrameter/powermeter/mqtt_test.py` around lines 103 - 125, Add a new async test that creates a multi-topic power meter via _make_pm (simulate multi-topic subscription), starts with a cold start (no pm._message_event set and no initial values), then schedule an update on only one of the subscribed topics (set its value and pm._message_event.set()) after a short sleep; call pm.wait_for_next_message(timeout=...) and assert it unblocks and that pm.get_powermeter_watts() returns the expected value for the topic that updated first. Ensure the test exercises wait_for_next_message and get_powermeter_watts with multi-topic state to lock down the cold-start behavior when only one topic produces the first message.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/astrameter/powermeter/mqtt.py`:
- Around line 176-181: The wait_for_next_message method can return after a
single topic update in multi-topic mode, leaving other phase values None and
causing get_powermeter_watts() to fail; modify wait_for_next_message to not
return until the internal multi-topic state is complete by checking the object's
phase/topic state (e.g. self._phases, self._state or expected topic list) after
the _message_event is set — use a loop that awaits _message_event.wait() with
the remaining timeout, clears the event, then verifies all expected
topics/phases are non-None before returning, and raise TimeoutError if the
overall timeout expires; keep the method name wait_for_next_message and the same
timeout semantics.
---
Nitpick comments:
In `@src/astrameter/powermeter/mqtt_test.py`:
- Around line 103-125: Add a new async test that creates a multi-topic power
meter via _make_pm (simulate multi-topic subscription), starts with a cold start
(no pm._message_event set and no initial values), then schedule an update on
only one of the subscribed topics (set its value and pm._message_event.set())
after a short sleep; call pm.wait_for_next_message(timeout=...) and assert it
unblocks and that pm.get_powermeter_watts() returns the expected value for the
topic that updated first. Ensure the test exercises wait_for_next_message and
get_powermeter_watts with multi-topic state to lock down the cold-start behavior
when only one topic produces the first message.
🪄 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: f5cca408-45d0-4a41-a0d9-3ae795641222
📒 Files selected for processing (17)
src/astrameter/ct002/ct002.pysrc/astrameter/main.pysrc/astrameter/powermeter/base.pysrc/astrameter/powermeter/homeassistant.pysrc/astrameter/powermeter/homeassistant_test.pysrc/astrameter/powermeter/homewizard.pysrc/astrameter/powermeter/homewizard_test.pysrc/astrameter/powermeter/mqtt.pysrc/astrameter/powermeter/mqtt_test.pysrc/astrameter/powermeter/pid.pysrc/astrameter/powermeter/pid_test.pysrc/astrameter/powermeter/sma_energy_meter.pysrc/astrameter/powermeter/sma_energy_meter_test.pysrc/astrameter/powermeter/throttling.pysrc/astrameter/powermeter/throttling_test.pysrc/astrameter/powermeter/transform.pysrc/astrameter/powermeter/transform_test.py
In multi-topic configurations, wait_for_next_message could return after receiving a message on just one topic, leaving other phase values as None and causing get_powermeter_watts() to fail. Now loops with a deadline until all subscribed values are populated, matching wait_for_message semantics but always waiting for a new message first. https://claude.ai/code/session_013EAvo4q396i36rhT45LNkp
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/astrameter/powermeter/mqtt_test.py`:
- Around line 103-117: The test test_wait_for_next_message_blocks_until_new
currently can pass for implementations that return early because it awaits the
setter task before asserting; change the test to assert that the setter task has
already completed immediately after await pm.wait_for_next_message(timeout=2)
(e.g., check task.done() or equivalent) and then assert pm.value == 42.0 to
ensure wait_for_next_message actually blocked until the new message was set;
locate the test and the call to pm.wait_for_next_message and add the post-wait
assertion referencing the existing task variable.
🪄 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: 280d492f-5011-4f99-85a6-5878a0f8b233
📒 Files selected for processing (3)
CHANGELOG.mdsrc/astrameter/powermeter/mqtt.pysrc/astrameter/powermeter/mqtt_test.py
✅ Files skipped from review due to trivial changes (1)
- CHANGELOG.md
Replace `await task` with `assert task.done()` after wait_for_next_message returns, so the test fails if the method returns before the setter task completes. https://claude.ai/code/session_013EAvo4q396i36rhT45LNkp
tomquist
pushed a commit
that referenced
this pull request
Apr 19, 2026
PR #322 wired wait_for_next_message into update_readings so push-based powermeters serve fresh data on every CT002 request. Two regressions followed: 1. Slow event-driven meters (e.g. Home Assistant wrapping a P1 that ticks every 10s, or any source with high THROTTLE_INTERVAL) raise TimeoutError on every request, which surfaces as "CT002 before_send failed" warnings and skips the cached value that would have been perfectly usable (issue #327). 2. MQTT multi-topic kept clearing the event whenever the waker happened to be a phase that had already published, so any topic still at None would force the full 5s timeout even though a fresh message had arrived. Treat wait_for_next_message as best-effort freshness: - update_readings caps the wait at 2s and swallows TimeoutError (cached value is served, which matches behaviour pre-#322). - Add WAIT_FOR_NEXT_MESSAGE (global [GENERAL] + per-section) so users can opt out of the wait entirely for sources that always update slower than the cap. Defaults to true. - MQTT wait_for_next_message returns on any next message instead of re-arming the event when not all topics have ever published. Refactor the closure inside run_device into a module-level read_ct_powermeter helper so the swallow-timeout behaviour is unit-testable.
tomquist
pushed a commit
that referenced
this pull request
Apr 19, 2026
The Shelly UDP handler was ignoring the per-powermeter wait flag, so a Shelly-emulated device would never wait for a fresh push from an event-driven powermeter (behaviour matched pre-#322). Mirror the CT path: read the flag when matching addr to powermeter, then best-effort await ``wait_for_next_message(timeout=2)`` with ``TimeoutError`` swallowed. Also consolidate the duplicate ``[HOMEASSISTANT]`` per-powermeter throttle example in config.ini.example: keep the more informative ``THROTTLE_INTERVAL = 2`` block (with the "2-3 seconds due to network latency" note) and drop the stray ``= 1`` duplicate that predated the ``WAIT_FOR_NEXT_MESSAGE`` example.
tomquist
added a commit
that referenced
this pull request
Apr 19, 2026
…eters (#330) * Stop wait_for_next_message regressions from breaking battery responses PR #322 wired wait_for_next_message into update_readings so push-based powermeters serve fresh data on every CT002 request. Two regressions followed: 1. Slow event-driven meters (e.g. Home Assistant wrapping a P1 that ticks every 10s, or any source with high THROTTLE_INTERVAL) raise TimeoutError on every request, which surfaces as "CT002 before_send failed" warnings and skips the cached value that would have been perfectly usable (issue #327). 2. MQTT multi-topic kept clearing the event whenever the waker happened to be a phase that had already published, so any topic still at None would force the full 5s timeout even though a fresh message had arrived. Treat wait_for_next_message as best-effort freshness: - update_readings caps the wait at 2s and swallows TimeoutError (cached value is served, which matches behaviour pre-#322). - Add WAIT_FOR_NEXT_MESSAGE (global [GENERAL] + per-section) so users can opt out of the wait entirely for sources that always update slower than the cap. Defaults to true. - MQTT wait_for_next_message returns on any next message instead of re-arming the event when not all topics have ever published. Refactor the closure inside run_device into a module-level read_ct_powermeter helper so the swallow-timeout behaviour is unit-testable. * Revert CHANGELOG entry for wait_for_next_message fix * Expose WAIT_FOR_NEXT_MESSAGE in the Home Assistant app config Adds a "Wait For Fresh Home Assistant Reading" toggle (default true) to the HA app so users with slow-updating source sensors (e.g. P1 smart meters) can opt out of the up-to-2s freshness wait and serve the last-known value immediately. * Honor WAIT_FOR_NEXT_MESSAGE in the Shelly emulator path The Shelly UDP handler was ignoring the per-powermeter wait flag, so a Shelly-emulated device would never wait for a fresh push from an event-driven powermeter (behaviour matched pre-#322). Mirror the CT path: read the flag when matching addr to powermeter, then best-effort await ``wait_for_next_message(timeout=2)`` with ``TimeoutError`` swallowed. Also consolidate the duplicate ``[HOMEASSISTANT]`` per-powermeter throttle example in config.ini.example: keep the more informative ``THROTTLE_INTERVAL = 2`` block (with the "2-3 seconds due to network latency" note) and drop the stray ``= 1`` duplicate that predated the ``WAIT_FOR_NEXT_MESSAGE`` example. --------- Co-authored-by: Claude <noreply@anthropic.com>
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.
The CT002's before_send callback now awaits wait_for_next_message() before reading the powermeter, ensuring push-based meters (MQTT, SMA, HomeWizard, Home Assistant) deliver a fresh measurement for every battery request instead of re-serving stale cached values.
Summary by CodeRabbit
New Features
Bug Fixes
Tests