feat(dashboard): add cross-pack Battery Health view with bundled heatmap card#79
Conversation
|
Warning Review limit reached
More reviews will be available in 44 minutes and 27 seconds. 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 (6)
📝 WalkthroughWalkthroughThe PR adds a bundled cell-heatmap frontend card, registers it automatically on setup, detects missing required Lovelace resources, and generates a new cross-battery Battery Health dashboard view with a custom heatmap table and aggregated voltage/temperature/power charts. ChangesBattery Health Dashboard with Bundled Heatmap Card
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
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)
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 |
![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 introduces a new 'Battery Health' view to the GivEnergy Local integration dashboard, featuring a bundled cell-balance heatmap card (ge-cell-heatmap.js) and comprehensive time-series charts for cell voltages, temperatures, power, and state of charge. It also implements pre-flight checks for required HACS cards and removes the cluttered cell voltage list from the main batteries view. Feedback on these changes highlights three key improvements: caching battery states in the custom card to prevent DOM thrashing on frequent updates, using a unique index-based tag for battery series to avoid naming collisions in the charts when multiple identical battery models are used, and replacing Number(x) with parseFloat(x) in the JavaScript filters to ensure null or empty readings are correctly rendered as gaps rather than plotted as zero.
dewet22
force-pushed
the
feat/dashboard-battery-health
branch
from
6b95241 to
891e9c4
Compare
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
custom_components/givenergy_local/dashboard.py (1)
87-95:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSkip the Battery Health view when no packs are present.
handle_generate_dashboard(...)buildsbatsfrom the coordinator's battery list, so this can be[]. In that case this still emits_battery_health_view(...), which in turn generatescustom:ge-cell-heatmapwithbatteries: [];GeCellHeatmap.setConfig()rejects that config and the generated dashboard breaks for inverter-only installs.Suggested fix
views = "\n".join( [ _overview_view(inv, max_power_kw), _energy_view(inv), _battery_view(bats), - _battery_health_view(inv, bats), + *([_battery_health_view(inv, bats)] if bats else []), _controls_view(inv), _diagnostics_view(inv, max_power_kw), ] )🤖 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 `@custom_components/givenergy_local/dashboard.py` around lines 87 - 95, handle_generate_dashboard currently always includes _battery_health_view(inv, bats) even when bats is empty, which causes GeCellHeatmap to reject the config; update handle_generate_dashboard (where views is built) to only append/call _battery_health_view when bats is non-empty (e.g., if bats and len(bats) > 0) so the battery health card is skipped for inverter-only installs and the rest of the dashboard remains valid.
🧹 Nitpick comments (1)
tests/test_dashboard.py (1)
56-67: ⚡ Quick winAdd the zero-battery case here.
This loop only exercises 1+ pack inputs, so it misses the
bats=[]path that currently produces an invalid heatmap config. Adding that case will lock in the fix for inverter-only systems.🤖 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 `@tests/test_dashboard.py` around lines 56 - 67, The test test_series_scale_with_battery_count currently iterates only over non-empty battery lists and misses the bats=[] path that exposes an invalid heatmap config; update the iteration in test_series_scale_with_battery_count to include an empty list (bats=[]) when calling _health_cards so cell_chart and power_chart are exercised for zero-battery systems, then assert expected counts for n = len(bats) (volt and temp series lengths should be 16*n and 4*n respectively) and verify power_chart["series"] equals 1 when n == 0 (otherwise 1 + n) to lock in the inverter-only behavior.
🤖 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/www/ge-cell-heatmap.js`:
- Line 87: The file currently calls customElements.define("ge-cell-heatmap",
GeCellHeatmap) unconditionally which throws if the element is already
registered; change it to first check customElements.get("ge-cell-heatmap") and
only call customElements.define("ge-cell-heatmap", GeCellHeatmap) when that
returns undefined (i.e., if not already registered) so duplicate loads won't
throw; locate the define call for GeCellHeatmap and wrap it with the get-check.
---
Outside diff comments:
In `@custom_components/givenergy_local/dashboard.py`:
- Around line 87-95: handle_generate_dashboard currently always includes
_battery_health_view(inv, bats) even when bats is empty, which causes
GeCellHeatmap to reject the config; update handle_generate_dashboard (where
views is built) to only append/call _battery_health_view when bats is non-empty
(e.g., if bats and len(bats) > 0) so the battery health card is skipped for
inverter-only installs and the rest of the dashboard remains valid.
---
Nitpick comments:
In `@tests/test_dashboard.py`:
- Around line 56-67: The test test_series_scale_with_battery_count currently
iterates only over non-empty battery lists and misses the bats=[] path that
exposes an invalid heatmap config; update the iteration in
test_series_scale_with_battery_count to include an empty list (bats=[]) when
calling _health_cards so cell_chart and power_chart are exercised for
zero-battery systems, then assert expected counts for n = len(bats) (volt and
temp series lengths should be 16*n and 4*n respectively) and verify
power_chart["series"] equals 1 when n == 0 (otherwise 1 + n) to lock in the
inverter-only behavior.
🪄 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: cacb6d92-197d-46eb-8b74-416d8606b443
📒 Files selected for processing (5)
custom_components/givenergy_local/__init__.pycustom_components/givenergy_local/dashboard.pycustom_components/givenergy_local/www/ge-cell-heatmap.jstests/test_dashboard.pytests/test_init.py
|
I reviewed the current PR head and do not have any actionable concerns. I ran |
dewet22
force-pushed
the
feat/dashboard-battery-health
branch
2 times, most recently
from
cb1d6c1 to
f1fb53d
Compare
…map card Replaces the hard-to-scan per-battery list of 16 cell voltages with a plant-level "Battery Health" view in the generated dashboard: - custom:ge-cell-heatmap cell-balance card, bundled with the integration and auto-served (async_register_static_paths + add_extra_js_url) — no HACS or manual resource install needed - a cell-voltage + cell-group-temperature chart whose right axis is scaled so the LFP warn bands align across both metrics, plus a battery-power + per-pack-SoC chart; both generalised to N packs with cycling palettes - removes the 16-row Cell Voltages list from each per-battery section - pre-flight check warns (never blocks) when apexcharts-card or power-flow-card-plus aren't registered before generating - bumps DASHBOARD_VERSION to 4 to raise the regenerate Repairs prompt The chart bad-read filters are a readability stopgap until the dongle garbage is filtered library-side (givenergy-modbus#78). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
dewet22
force-pushed
the
feat/dashboard-battery-health
branch
from
f1fb53d to
4bfe9c4
Compare
dewet22-codex
left a comment
dewet22-codex
left a comment
There was a problem hiding this comment.
Approved. I reviewed the current head and do not have any actionable concerns.
2 participants
Battery Health dashboard view
Replaces the per-battery list of 16 cell voltages (hard to scan for outliers, worse with multiple packs) with a dedicated, cross-pack Battery Health view.
What's in it
custom:ge-cell-heatmap) — each cell coloured by its mV deviation from its own pack's mean (imbalance shows at any charge level), plus per-pack mean and spread. Bundled with the integration and served automatically (static path +add_extra_js_url); users install nothing.Supporting changes
apexcharts-card/power-flow-card-plusaren't registered, so you get a clear note rather than a broken card. Advisory only (storage-mode resources are all it can enumerate).DASHBOARD_VERSION→ 4 raises the existing "dashboard outdated" Repairs prompt.Notes
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Tests