feat: add parsed battery storage data with power integration

[it-rec]

What

Adds a small model layer for the storageData endpoint:

• BatteryStorageData / StorageData dataclasses in a new models.py
• SolarEdge.get_parsed_storage_data(...) — a convenience wrapper around the
  existing get_storage_data(...) that returns the parsed, typed objects
• integrate_power(...) helper, exported for reuse/testing

All three are re-exported from the package root.

Why

SolarEdge frequently reports lifeTimeEnergyCharged / lifeTimeEnergyDischarged
as 0 in the storageData response, even while the battery is actively
charging or discharging. The power telemetry, however, is reliably populated.

So instead of reading the (often-zero) lifetime counters, the charge/discharge
energy is derived by trapezoidal integration of the power telemetry over
time (positive power = charging, negative = discharging), returned in Wh.

This was verified against a live site: the power integral gave ~4.84 kWh charged
vs. ~4.47 kWh from SoC × capacity (~3% = charging losses), while the lifetime
counters stayed at 0 the whole time.

Keeping this device-specific parsing in the library (rather than in each
consumer) was requested during review of the Home Assistant integration —
see home-assistant/core#169964 and home-assistant/core#173096. A follow-up HA
PR will switch the integration to get_parsed_storage_data() once this is
released.

Notes

• Batteries without a serial number or without telemetry are skipped.
• StorageData.from_response() raises KeyError if storageData/batteries
  are missing, matching the existing "raise on missing key" expectations of
  consumers.
• New code is fully covered by unit tests; ruff, ruff format and the
  existing test suite pass locally.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[it-rec]

cc @bdraco

[bdraco]

Still have this on my list to get back to, but every day I wake up to a new CVE to solve so it keeps moving down the list. Hopefully that will stop soon

[it-rec]

Thank you!

I can help you if you want if you tell me how you want to go forward.

And can't we simply automerge some of these dependabot PRs?

[bluetoothbot]

@it-rec: Thank you!

Dependabot automerge is doable. Add a workflow keyed on dependabot[bot] PRs, gate on dependabot/fetch-metadata, then call gh pr merge --auto --squash. Requires branch protection with required status checks. Scope it to patch/minor updates to stay safe.

[bluetoothbot]

PR Review — feat: add parsed battery storage data with power integration

Solid, well-scoped additive feature. Merge-ready; one non-blocking accuracy note.

Strengths:

• integrate_power is genuinely robust — unparsable timestamps, non-monotonic intervals (interval <= 0), and missing/None power are all handled and explicitly covered by parametrized tests.
• No circular import: models.py depends only on stdlib, and solaredge.py/__init__.py import from it cleanly.
• Purely additive public surface (new method + re-exports), so no backward-compat risk; from_response raising KeyError matches existing consumer expectations.
• The rationale (integrating power because lifetime counters report 0) is documented in both docstrings and validated against a live site.

Needs attention:

• Per-interval charge/discharge classification misattributes energy when power crosses zero within an interval — minor given telemetry granularity, suggest documenting or splitting at the zero-crossing.

---

🟢 Suggestions

1. Per-interval sign classification misattributes energy across zero-crossings
src/aiosolaredge/models.py:47-57

The energy for each interval is classified entirely as charge or discharge based on the average power over that interval. When power crosses zero within a single interval, this misattributes energy.

Example: previous.power = +3000, current.power = -1000 over 1h. Average is +1000, so the whole 1000 Wh is booked as charge — even though the battery was discharging for part of that interval. The true split is ~1125 Wh charged and ~125 Wh discharged.

Why it matters: this is the PR's core value proposition (accurate charge/discharge split). The error is bounded by telemetry granularity — within a short sample interval the battery rarely reverses sign — and your live-site verification showed only ~3% deviation, so this is not blocking. But it's worth either:

• documenting the limitation in the integrate_power docstring, or
• splitting a sign-crossing interval at its zero point before accumulating (exact for a linear segment).

if energy >= 0:
    charge_energy += energy
else:
    discharge_energy -= energy

---

Checklist

• No circular imports introduced
• Edge cases covered (empty, None, non-monotonic)
• Integration math correct across zero-crossings — suggestion ＃1
• Backward-compatible public API
• New code covered by tests
• No hardcoded secrets

---

Silent Failure Analysis

🟡 **MEDIUM** — fallback value hides missing data
src/aiosolaredge/models.py:48-51

Risk: A missing or null power field is silently coerced to 0 W, so a battery that is actively cycling but has gaps in telemetry will under-report charge/discharge energy with no indication that data was incomplete.

average_power = (
    (previous.get("power") or 0.0) + (current.get("power") or 0.0)
) / 2

Fix: Distinguish a genuine 0 W reading from missing data — skip the interval (like the timestamp case) or surface the gap, rather than treating absent power as zero energy.

🟡 **MEDIUM** — silent data-point skip
src/aiosolaredge/models.py:42-46

Risk: Unparsable timestamps and non-monotonic intervals are dropped with no count or warning, so a malformed batch could silently zero-out large portions of the integrated energy while still returning a plausible-looking total.

if previous_time is None or current_time is None:
    continue
interval = (current_time - previous_time).total_seconds() / 3600
if interval <= 0:
    continue

Fix: Track and expose (or log) the number of skipped intervals so callers can detect when integration was based on degraded telemetry.

🟡 **MEDIUM** — silent null return / dropped record
src/aiosolaredge/models.py:83-93

Risk: Batteries missing a serial number or telemetry are silently dropped in from_response, so a site whose batteries stop reporting telemetry would appear as zero batteries with zero energy rather than signalling the outage.

serial_number = battery.get("serialNumber")
if not serial_number:
    return None
telemetries = battery.get("telemetries") or []
if not telemetries:
    return None

Fix: This filtering is documented, but consider returning/logging the count of skipped batteries so downstream consumers can distinguish 'no batteries' from 'batteries with no usable data'.

---

Automated review by Kōan (Claude) HEAD=b0e45af 2 min 33s

[bluetoothbot]

No blocking issues found.

[Copilot]

Pull request overview

Adds a typed parsing/model layer for the SolarEdge storageData endpoint and a convenience client wrapper to return parsed battery storage data, deriving charge/discharge energy by integrating power telemetry.

Changes:

• Introduces BatteryStorageData and StorageData dataclasses plus integrate_power(...) in a new models.py.
• Adds SolarEdge.get_parsed_storage_data(...) as a wrapper around get_storage_data(...).
• Re-exports the new models/helpers from the package root and adds unit tests.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/aiosolaredge/models.py	Adds dataclasses and power-integration logic to compute charge/discharge energy from telemetry.
src/aiosolaredge/solaredge.py	Adds get_parsed_storage_data(...) returning StorageData instead of raw JSON.
src/aiosolaredge/__init__.py	Re-exports the new models/helper at the package root.
tests/test_models.py	Adds unit tests for parsing and integrate_power(...).
tests/test_init.py	Extends integration-style client test to cover get_parsed_storage_data(...).

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.