Coalesce concurrent polls to prevent burst responses#542
Conversation
When a meter read is delayed — WAIT_FOR_NEXT_MESSAGE awaiting the next push, THROTTLE_INTERVAL sleeping out its window, or any slow source — the battery keeps polling (~1/s) and each UDP datagram spawns its own handler task. Every handler parked in before_send woke on the single fresh reading and sent its own response, so the battery received a burst of instructions milliseconds apart. Because each CT002 instruction is a delta the firmware adds to its current output, the burst wound the battery up by N times the intended correction and stepped the stateful balancer N times per real sample. Track an in-flight consumer set: the first handler owns the read-and- respond section for its consumer; concurrent polls from the same battery are dropped after refreshing per-consumer state, so exactly one response goes out per reading. Keyed per consumer, so distinct batteries stay independent. This sits at the request-handler level, so it fixes both the WAIT_FOR_NEXT_MESSAGE and THROTTLE_INTERVAL causes at once. Python-only: the ESPHome mirror drains UDP synchronously (pump_udp_ -> handle_request_) with no async before_send, so it has no pile-up to coalesce and needs no counterpart change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RaCn4moMmua81wuKo8nKEK
The Shelly Pro 3EM emulator has the same concurrent-handler pile-up as CT002: each UDP datagram spawns its own task, and when the meter read is delayed (WAIT_FOR_NEXT_MESSAGE awaiting a push, THROTTLE_INTERVAL sleeping, or a slow source) the battery keeps polling so handlers pile up parked in the read; one fresh reading wakes them all and each answers. Although a Shelly response carries an absolute grid reading rather than a CT002-style delta, the harm is the same: the battery runs a closed-loop zero-export controller, so the reading is the error signal it integrates against its own output. Feeding it the same pre-adjustment reading several times before the plant can reflect its response makes it over-correct past target — effectively a burst of deltas. Track an in-flight battery set keyed on source IP (matching the existing dedup identity): the first handler owns the read-and-respond section; a concurrent poll from the same battery is dropped after _track_battery_seen has refreshed liveness/poll-interval state, so exactly one response goes out per reading. Keyed per battery, so distinct batteries stay independent; the flag is cleared in finally so a failed/hung read never locks a battery out. Replaces the old test_multiple_requests_with_throttling (which asserted the pre-fix all-answered behavior) with an end-to-end UDP coalescing test plus deterministic in-process tests for single-response coalescing, per-battery independence, no lock-out, and flag-clear on meter failure. Python-only: the Shelly emulator has no ESPHome mirror. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RaCn4moMmua81wuKo8nKEK
|
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 (5)
WalkthroughCT002 and Shelly emulator request handlers now track in-flight requests per consumer/battery using new sets, dropping duplicate concurrent polls while a meter reading is pending. Both handlers use try/finally to clear tracking state. New tests validate coalescing behavior, per-key isolation, and recovery after bursts or failures. ChangesPoll coalescing fix
Estimated code review effort: 4 (Complex) | ~55 minutes Sequence Diagram(s)sequenceDiagram
participant ClientA
participant ClientB
participant Handler
participant Meter
ClientA->>Handler: poll request
Handler->>Handler: mark key as in-flight
Handler->>Meter: read meter value (slow)
ClientB->>Handler: duplicate poll (same key)
Handler-->>ClientB: dropped (already in-flight)
Meter-->>Handler: reading returned
Handler-->>ClientA: single response
Handler->>Handler: clear in-flight key (finally)
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RaCn4moMmua81wuKo8nKEK
Steering evaluation (base vs head)Overall: 0 improved, 0 regressed, 15 unchanged across 15 metrics — mean 0% (unchanged). Priority: priority-weighted 0% (unchanged) — ✅ no do-no-harm guardrail regressions. Lower is better for every metric. See Metrics are the per-scenario mean of 5 seeds. Aggregate — mean across 32 scenarios
📊 Interactive grid-power charts (zoom / hover / toggle series) are in the self-contained What do these metrics mean?
Per-scenario tables (32 scenarios)full_battery_low_pace — settle 0.0→0.0s, overshoot 0.0→0.0W, RMS 29.1→29.1W
mixed_cadence/eff — settle 50.8→50.8s, overshoot 164.5→164.5W, RMS 21.9→21.9W
mixed_cadence/fair — settle 47.7→47.7s, overshoot 70.4→70.4W, RMS 12.9→12.9W
mixed_cadence_solar/eff — settle 52.5→52.5s, overshoot 384.7→384.7W, RMS 28.6→28.6W
mixed_cadence_solar/fair — settle 56.0→56.0s, overshoot 75.4→75.4W, RMS 23.7→23.7W
mixed_venus_b2500/eff — settle 81.2→81.2s, overshoot 221.9→221.9W, RMS 18.6→18.6W
mixed_venus_b2500/fair — settle 75.4→75.4s, overshoot 231.1→231.1W, RMS 22.3→22.3W
phase_imbalance — settle 53.4→53.4s, overshoot 145.2→145.2W, RMS 30.3→30.3W
single_venus_d_solar — settle 24.2→24.2s, overshoot 94.4→94.4W, RMS 15.9→15.9W
single_venus_d_steps — settle 26.3→26.3s, overshoot 90.3→90.3W, RMS 15.5→15.5W
single_venus_d_washer — settle 0.0→0.0s, overshoot 0.0→0.0W, RMS 61.0→61.0W
single_venus_drain — settle 0.0→0.0s, overshoot 0.0→0.0W, RMS 907.3→907.3W
single_venus_fill — settle 360.0→360.0s, overshoot 0.0→0.0W, RMS 953.6→953.6W
single_venus_noisy — settle 0.0→0.0s, overshoot 0.0→0.0W, RMS 94.3→94.3W
single_venus_pv — settle 0.0→0.0s, overshoot 0.0→0.0W, RMS 60.8→60.8W
single_venus_solar — settle 26.8→26.8s, overshoot 80.3→80.3W, RMS 17.8→17.8W
single_venus_solar_slow — settle 33.9→33.9s, overshoot 68.3→68.3W, RMS 22.8→22.8W
single_venus_steps — settle 26.0→26.0s, overshoot 88.0→88.0W, RMS 14.7→14.7W
single_venus_steps_slow — settle 40.5→40.5s, overshoot 98.5→98.5W, RMS 14.8→14.8W
single_venus_trace — settle 0.0→0.0s, overshoot 0.0→0.0W, RMS 278.9→278.9W
single_venus_washer — settle 0.0→0.0s, overshoot 0.0→0.0W, RMS 61.0→61.0W
two_venus/eff — settle 18.1→18.1s, overshoot 126.1→126.1W, RMS 14.0→14.0W
two_venus/fair — settle 18.4→18.4s, overshoot 116.7→116.7W, RMS 13.8→13.8W
two_venus_noisy/eff — settle 0.0→0.0s, overshoot 0.0→0.0W, RMS 94.3→94.3W
two_venus_noisy/fair — settle 0.0→0.0s, overshoot 0.0→0.0W, RMS 94.2→94.2W
two_venus_slow/fair — settle 41.8→41.8s, overshoot 174.5→174.5W, RMS 14.0→14.0W
two_venus_solar/eff — settle 26.0→26.0s, overshoot 396.6→396.6W, RMS 20.4→20.4W
two_venus_solar/fair — settle 25.9→25.9s, overshoot 151.4→151.4W, RMS 20.4→20.4W
two_venus_trace/eff — settle 0.0→0.0s, overshoot 0.0→0.0W, RMS 283.1→283.1W
two_venus_trace/fair — settle 0.0→0.0s, overshoot 0.0→0.0W, RMS 282.1→282.1W
venus_d_plus_c/eff — settle 20.1→20.1s, overshoot 128.9→128.9W, RMS 14.7→14.7W
venus_d_plus_c/fair — settle 21.6→21.6s, overshoot 121.0→121.0W, RMS 14.6→14.6W
📊 Open the interactive report — |
The end-to-end test's two coalesced-away polls are expected to time out, and send_req swallowed that with `except TimeoutError`. But asyncio.wait_for raises asyncio.TimeoutError, which on Python 3.10 is a distinct class from the builtin TimeoutError (they were unified in 3.11). So on 3.10 the expected timeout escaped and failed the test, while 3.11-3.13 passed. Catch both classes so the expected coalescing timeouts are swallowed on every supported Python version. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RaCn4moMmua81wuKo8nKEK
Summary
Fix a critical issue where batteries and the CT002 meter emulator would receive multiple response deltas in quick succession when a meter read was delayed, causing the battery's control loop to overshoot its target. The fix coalesces concurrent polls from the same battery/consumer into a single response per meter reading.
Problem
When a meter read is delayed — either via
WAIT_FOR_NEXT_MESSAGE, a throttled meter, or a slow HTTP source — the battery keeps polling (~1/s) and each datagram spawns its own handler task. Without coalescing, all parked handlers wake on the same fresh reading and each sends a response, creating a burst of deltas that the battery's firmware adds to its output. For Shelly batteries with a zero-export controller, this feeds the same stale error signal multiple times before the plant can respond, causing overshoot. For CT002, it steps the stateful balancer multiple times per real sample, winding up the battery.Solution
Track in-flight handlers per battery/consumer using a set (
_inflight_consumersfor CT002,_inflight_batteriesfor Shelly). When a poll arrives:finallyblockThis ensures exactly one response per meter reading, regardless of how many polls arrive while the read is in flight.
Changes
CT002 (
src/astrameter/ct002/ct002.py)_inflight_consumers: set[str]to track consumers with parked handlersShelly (
src/astrameter/shelly/shelly.py)_inflight_batteries: set[str]to track batteries with parked handlersTests
GatedPowermetertest fixture that parks reads on an asyncio.EventNotes
https://claude.ai/code/session_01RaCn4moMmua81wuKo8nKEK
Summary by CodeRabbit