feat: House Consumption Today + correct the load mislabel (modbus 2.1.1)#111
Conversation
… (modbus 2.1.1) givenergy-modbus 2.1.1 (#174) corrected two single-phase mislabels and added the real derived house-consumption figure: - New "House Consumption Today" sensor (e_consumption_today) — the GE-app "Consumption today" value, single-phase only (defensive getattr + skip_if_none). The dashboard's "Consumed" series now points here, fixing the long-standing ~0/cliff that came from the old bogus load sensor. DASHBOARD_VERSION 5 → 6. - e_load_day was never house load (it's AC charge); renamed the sensor to "AC Charge Today" (key e_ac_charge_today). A unique_id migration in __init__.py re-points the existing entity in place, preserving its history/statistics rather than orphaning it. - e_inverter_out_day: entity held (key/name unchanged) pending verification of the matching total, but its value_fn now reads the renamed e_pv_generation_today field directly to avoid a per-poll DeprecationWarning. - Recommended-entities set swaps e_load_day → e_consumption_today. Pin bumped to givenergy-modbus>=2.1.1. Entity-reviewer confirmed the migration is correct on all edge cases (slicing, multi-inverter, collision, ordering). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…e real sensor Two changes to the GivTCP statistics migration, both validated during a live recovery: - Recorder-write resilience: chunk large imports, retry recorder mutations on HA's command-timeout (clear/import are idempotent), and pace entities so the single-threaded recorder can drain. This is the fix for the timeout cascade that previously left entities cleared-but-not-reimported mid-run. - Re-include the house-consumption pair, now targeting house_consumption_today (givenergy-modbus #174's real derived consumption) instead of the old load_energy_today mislabel that read ~0. Catalogue doc updated to match. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 53 minutes and 1 second. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds unique_id migration for renamed sensor suffixes, updates inverter sensor descriptors and recommended keys to match givenergy-modbus v2.1.1, bumps dashboard schema and templates to use house_consumption_today, improves the GivTCP migration script (recorder retries, UTC handling, chunked imports), and adds tests covering the changes. ChangesAlignment with givenergy-modbus v2.1.1
Sequence DiagramsequenceDiagram
participant Setup as async_setup_entry
participant Migration as _migrate_unique_ids
participant Registry as EntityRegistry
participant Platform as PlatformSetup
Setup->>Migration: call with config entry
Migration->>Registry: list entities for config entry
Registry-->>Migration: return entity entries
Migration->>Migration: find entries with old suffix (e_load_day)
Migration->>Registry: update unique_id → e_ac_charge_today (skip if target exists)
Registry-->>Migration: confirm updates
Migration-->>Setup: migration complete
Setup->>Platform: forward to platform setup
Platform-->>Setup: create entities with updated descriptors
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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 |
 |
|
Overall Grade |
Security Reliability Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| Python |  |
Jun 3, 2026 12:05p.m. | Review ↗ |
Important
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
![deepsource-io[bot]](https://avatars.githubusercontent.com/in/16372?s=60&v=4){kind=small}
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the integration to support givenergy-modbus version 2.1.1, introducing a new derived house consumption sensor (e_consumption_today) and renaming the mislabelled e_load_day sensor to e_ac_charge_today. It includes a unique ID migration path to preserve historical data, updates the dashboard template, and enhances the GivTCP migration script with recorder write resilience (chunking, retries, and pacing). A critical issue was identified in the migration script where asyncio is used for pacing and retries but is not imported, which will lead to a NameError at runtime.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![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)
scripts/migrate_from_givtcp.py (1)
123-123: 💤 Low valueOptional: Replace ambiguous minus sign with standard hyphen.
The comment uses the Unicode MINUS SIGN (U+2212 "−") instead of the standard ASCII HYPHEN-MINUS (U+002D "-"). While cosmetic, using ASCII avoids potential copy-paste issues and aligns with Python style.
📝 Suggested fix
- # align; validate the overlap before an --apply. + # align; validate the overlap before an --apply. ("load_energy_today_kwh", "house_consumption_today", "House consumption today", True),(Replace "−" with "-" in "PV + grid-in − grid-out − AC-charge")
🤖 Prompt for 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. In `@scripts/migrate_from_givtcp.py` at line 123, Replace the Unicode MINUS SIGN characters in the comment "PV + grid-in − grid-out − AC-charge" with standard ASCII hyphen-minus characters so it reads "PV + grid-in - grid-out - AC-charge"; update that exact comment string in scripts/migrate_from_givtcp.py to avoid the U+2212 character and use U+002D instead.
🤖 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 `@custom_components/givenergy_local/sensor.py`:
- Around line 493-497: Replace the ambiguous Unicode minus sign (U+2212) used in
the inline comment explaining "Derived house consumption" with a plain ASCII
hyphen '-' so the comment reads e.g. "(PV gen + grid-in - grid-out -
AC-charge)"; search the comment block around the derived house consumption
explanation in sensor.py and update any other occurrences of U+2212 to '-' and
re-run the linter to confirm Ruff RUF003 is resolved.
---
Nitpick comments:
In `@scripts/migrate_from_givtcp.py`:
- Line 123: Replace the Unicode MINUS SIGN characters in the comment "PV +
grid-in − grid-out − AC-charge" with standard ASCII hyphen-minus characters so
it reads "PV + grid-in - grid-out - AC-charge"; update that exact comment string
in scripts/migrate_from_givtcp.py to avoid the U+2212 character and use U+002D
instead.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9b6d98ed-82ba-4e18-8f84-2594f0b1737e
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
custom_components/givenergy_local/__init__.pycustom_components/givenergy_local/const.pycustom_components/givenergy_local/dashboard.pycustom_components/givenergy_local/manifest.jsoncustom_components/givenergy_local/sensor.pydashboard/template.yamldocs/migration-from-givtcp.mdpyproject.tomlscripts/migrate_from_givtcp.pytests/conftest.pytests/test_dashboard.pytests/test_sensor.py
|
I’ve started reviewing this PR. |
Addresses PR #111 review: - DeepSource (run cyclomatic complexity 42): extract the cutover-suggestion print, plan construction, and summary table into helpers — run() drops from ~161 to ~91 lines. The battle-tested execute/write path is untouched. - CodeRabbit/Gemini (RUF003): replace ambiguous U+2212 minus signs with ASCII hyphens in the consumption-formula comments (sensor.py, migrate script). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Addresses PR #111 review: - Codex: the docstring listed "House load today" under "Not migrated", but INVERTER_PAIRS now migrates it (as house consumption). Moved it into the default-migrated list and dropped the stale exclusion. - DeepSource: replace _print_summary's four sum() comprehensions with a single Counter, cutting its cyclomatic complexity below the gate. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
I’ve started re-reviewing this PR. |
dewet22-codex
left a comment
dewet22-codex
left a comment
There was a problem hiding this comment.
Approved. I re-reviewed the current head and do not have any actionable concerns. I checked the updated diff and current GitHub checks; the earlier migration-script help-text finding has been addressed.
2 participants
Summary
Adopts givenergy-modbus 2.1.1 (#174), which corrected two single-phase field mislabels and added the real derived house-consumption figure. This fixes the dashboard's long-standing consumption cliff — givenergy_local showed ~0 consumption because the old "Load Energy Today" sensor read IR35, which is actually AC charge, not house load.
Changes
e_consumption_today) — the GE-app "Consumption today" derived value (PV generation + grid-in − grid-out − AC-charge), single-phase only (defensive getattr +skip_if_none). The dashboard "Consumed" series now points here;DASHBOARD_VERSION5 → 6 so existing users are prompted to regenerate.e_load_day→e_ac_charge_today("AC Charge Today"). A unique_id migration (_migrate_unique_idsin__init__.py) re-points the existing entity in place so its history/statistics survive rather than orphaning it.e_inverter_out_dayentity (key/name) unchanged — the rename is deferred until the matching total (IR45/46) is verified and renamed library-side — but itsvalue_fnreads the renamede_pv_generation_todaydirectly to avoid a per-poll DeprecationWarning.givenergy-modbus>=2.1.1.house_consumption_todaysensor.Notes
ha-entity-revieweragent confirmed the unique_id migration is correct on every edge case (slicing, multi-inverter serials, collision handling, ordering before platform setup).e_inverter_out_total(IR45/46) so today+total move together.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests