CT002: relay mode bucket aggregation & adaptive consumer eviction#465
Conversation
…ets, adaptive eviction Three emulator fixes (mirrored in the ESPHome ct002 component per the parity rule), all in the cross-talk aggregation / response path: - #457: relay mode (ACTIVE_CONTROL=False) now aggregates each battery's *reported* power into the per-phase *_chrg/_dchrg buckets instead of reported+grid (the battery's next expected net). Active control keeps the net-instructed-power semantics from #376. - #460: add the x (unassigned/inspection) and ABC (combined, phase "D") buckets. Inspection ('0') reporters now populate x_chrg/x_dchrg and no longer inflate phase A's count (which skewed the relay share split during any peer's inspection window); phase-D reporters populate ABC_chrg/ABC_dchrg and ABC_chrg_nb. The consumer's phase is stored as reported (normalized to "0" for inspection markers) instead of being forced to "A". - #462: consumer eviction now defaults to an adaptive TTL (~2 missed cycles of the battery's observed poll cadence, floored at 5s, 30s while the cadence is unknown), like the real CT, instead of a fixed 120s. Aggregation also skips expired consumers per response so relay counts shrink at poll granularity. An explicit CONSUMER_TTL (Python) / consumer_ttl (ESPHome) restores a fixed window. Eviction and report timestamps now use the injected clock, unifying the timebase with the dedup logic and making eviction deterministically testable. Tests: new relay/x/ABC/eviction unit tests, shared Python-vs-ESPHome e2e scenarios (with new active_control/consumer_ttl test-hook cfg keys), and "0"/"D" phases in the wire-identical fuzzer. https://claude.ai/code/session_01L1fNX5zVsyvfUtVw9yK2Hi
|
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 (1)
✅ Files skipped from review due to trivial changes (1)
WalkthroughPreserve reported phases, add explicit x/A/B/C/ABC aggregation buckets, forward per-phase reported power in relay mode, and change consumer eviction default to adaptive (~2 missed polls) with optional fixed CONSUMER_TTL override. ChangesCT002 relay aggregation with adaptive TTL and phase bucketing
Sequence Diagram(s)sequenceDiagram
participant Battery as Battery (Consumer)
participant Emulator as CT002 Emulator
participant Aggregator as Collector
participant Response as Response Builder
Battery->>Emulator: report(power, phase)
Emulator->>Emulator: normalize phase -> A/B/C/D or 0 (x)
Emulator->>Emulator: compute per-consumer TTL (adaptive or configured)
Emulator->>Aggregator: update consumer store (skip expired)
Aggregator->>Aggregator: map phase -> bucket (x/A/B/C/ABC)
Aggregator->>Aggregator: select value (instructed vs reported)
Aggregator->>Response: aggregate by bucket
Response->>Response: emit x/A/B/C/ABC fields
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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`:
- Around line 362-368: The reporting path still coerces non-ABC phases to "A"
even though you now store canonical "0"/"D" in Consumer.phase; update
reporting_consumer_rows() (and any helper that builds the reporting row type) to
pass through the normalized_phase value (preserve "0" and "D") instead of
remapping to "A", and ensure format_cd4_slave_csv() and the row serialization
accept and emit these canonical phases unchanged so the Python reporting rows
match the ESPHome mirror; inspect any row type/serializer used by
reporting_consumer_rows(), reporting row constructors, and
format_cd4_slave_csv() and remove the forced non-ABC -> "A" conversion.
🪄 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: a8569fce-a9bb-495b-a91f-78022b90181f
📒 Files selected for processing (14)
CHANGELOG.mdREADME.mdconfig.ini.exampledocs/ct002-ct003-protocol.mdesphome.example.yamlesphome/components/ct002/__init__.pyesphome/components/ct002/ct002.cppesphome/components/ct002/ct002.hesphome/components/ct002/test_hooks.cppsrc/astrameter/ct002/ct002.pysrc/astrameter/main.pytests/components/ct002/test_shared_e2e.pytests/test_ct002_protocol.pyweb/ts/schema.ts
Consumer.phase can now hold "0"/"D", but reporting_consumer_rows() still coerced every non-a/b/c phase to "a", so the Python cd=4 slave list claimed phase A for inspecting/combined batteries while the ESPHome mirror returns the stored phase verbatim. Extend ReportingPhase with "d"/"0" and pass the canonical char through, matching the real CT's slv_p=<phase char> semantics. https://claude.ai/code/session_01L1fNX5zVsyvfUtVw9yK2Hi
Summary
Fixes relay-mode cross-talk bucket aggregation to match real CT hardware behavior and implements adaptive consumer eviction based on observed poll cadence (issues #457, #460, #462).
Key Changes
Relay Mode Bucket Aggregation (Issue #457)
Bucket Routing for Inspection & Combined Mode (Issue #460)
"0") now route to thex_*bucket instead of inflating phase A"D") now route to theABC_*bucket andABC_chrg_nbcount instead of phase A_bucket_for_phase()helper to centralize phase→bucket mapping in both Python and C++Adaptive Consumer Eviction (Issue #462)
consumer_ttl=None): consumers expire after missing ~2 of their own observed poll cycles, matching real CT hardwareconsumer_ttlvalue) preserves the old behavior for networks with long polling gapsImplementation Details
"0"for unassigned/inspection states instead of forcing"A"_collect_reports_by_phase()checks consumer expiration before aggregation, mirroring real CT behaviorFiles Modified
src/astrameter/ct002/ct002.py— relay aggregation logic, adaptive TTL, bucket routingesphome/components/ct002/ct002.cpp— C++ mirror of aboveesphome/components/ct002/ct002.h— bucket enum, TTL constants, PhaseReports structtests/test_ct002_protocol.py— unit tests for new behaviortests/components/ct002/test_shared_e2e.py— e2e tests for parityhttps://claude.ai/code/session_01L1fNX5zVsyvfUtVw9yK2Hi
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests