[stefanodecillis]

Summary

Add a read-only client for SolarEdge's Basic Monitoring API V2 alongside the existing V1 client. The existing SolarEdge API and package version are unchanged.

• SolarEdgeV2: site metadata, onboard lifetime-energy counters and timestamped 15-minute average power samples.
• BasicMonitoring: site-timezone day/month/year energy, directional daily energy, device inventory and per-battery first-to-last observed daily counter deltas.
• Caller-managed OAuth tokens and an optional asynchronous before_request hook for reserving a shared request budget before I/O.
• Typed authentication, permission, throttling and invalid-response errors. Preserve allowlisted quota headers and Retry-After without retrying automatically or exposing response bodies or tokens.
• Normalize units to Wh/W; preserve null values; reject invalid numeric values, counter resets and inconsistent storage membership.

Only Basic read endpoints are used. No paid Advanced endpoints, device controls, historical-TOTAL diagnostics or token acquisition/refresh implementation are included. Average power is not presented as an instantaneous measurement; observed storage deltas do not claim complete midnight-to-now coverage.

Validation

• Complete library suite: 62 passed on Python 3.14 and Python 3.10.
• Targeted mypy and Ruff checks/format passed.
• Tests use synthetic fixtures and intercepted HTTP transport.
• Upstream CI has not run yet.

Related work

• Home Assistant migration discussion: Solaredge Integration seems to use API V1 that will retire on 2026-11-01 home-assistant/core#185245
• Earlier related report: SolarEdge API V1 to V2 home-assistant/core#185302
• Existing Home Assistant V2 proposal: Migrate SolarEdge to the V2 API home-assistant/core#187939
• Official Basic API documentation: https://api-docs.solaredge.com/docs/basic-monitoring-api/ubalc8zzxypbr-solar-edge-basic-monitoring-api

This client is reusable independently of the integration approach chosen by Home Assistant. Versioning and release remain with the library maintainer's existing release process.

[Forcky]

I ran this against a real site (AU, SE5000H + SE-MTR-3Y import/export meter, no battery): I captured the live V2 responses and replayed them through a147eac.

• site, lifetime_energy, power and overview (day/month/year) are all OK. Lifetime matches V1 to the Wh, and the month and year totals match /energy?resolution=TOTAL.
• inventory() raises InvalidResponseError("Invalid inventory device").
  • /devices on this site returns two virtual meters (meterType SelfConsumption and Consumption, form: "virtual") with serialNumber: null. It also lists the physical meter twice under the same serial (meterType FeedIn and Purchased).
  • So it fails on the null serial, and would then fail the (type, serialNumber) duplicate check as well.
  • Keying meters on (serialNumber, meterType) and allowing a null serial for form == "virtual" would cover both.
• On 200 responses the only rate-limit headers are x-ratelimit-limit-minute: 10 and x-ratelimit-remaining-minute. I saw no -month headers, but I didn't hit a 429.
• At dawn, power() gets null samples before sunrise and then 0.0. It returns the latest 0.0, which is the intended behaviour.

I'm happy to help with the fix directly if that's useful. I can send a PR against your branch for the inventory handling, with a test built from this site's (anonymised) /devices response. Or I can just re-run against the real site after you push changes.

[stefanodecillis]

Sure!

[Forcky]

I opened the inventory fix against your branch: stefanodecillis#1. Meters are keyed by role, and a null serial is allowed only for virtual meters; everything else stays strict. The test results and the offline replay of the real site are in the description.

[codecov]

Codecov Report

❌ Patch coverage is 83.47458% with 39 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/aiosolaredge/monitoring.py	82.30%	12 Missing and 8 partials ⚠️
src/aiosolaredge/v2.py	84.55%	12 Missing and 7 partials ⚠️

📢 Thoughts on this report? Let us know!

[halibsbot]

@Forcky: I opened the inventory fix against your branch: stefanodecillis#1. Meters are keyed by ...

Fix merged as ce7be55. Inventory keys meters by role. Null serial only for virtual meters. Tests cover all reported shapes.

[halibsbot]

PR Review — Add a read-only SolarEdge Basic Monitoring V2 client

Solid, carefully bounded V2 client. One error-handling gap on Python 3.10 should be fixed before merge.

Strengths:

• Error redaction is strict: no bodies or tokens in errors, allow_redirects=False, and an allowlist of quota headers with length/size bounds.
• Site IDs are validated against path injection before the token is fetched.
• DST-correct site-zone calendar boundaries, with tests for both transitions and the fold hour.
• Numeric normalization rejects bool, inf, overflow and negative values, while null stays distinct from zero.
• Storage deltas detect counter resets, reject duplicate or out-of-window samples, and raise a typed StorageMembershipChanged instead of making up inventory.
• Forcky's live-site fix for virtual and multi-role meters is merged and covered by tests.

Needs attention:

• 🟡 except TimeoutError misses asyncio.TimeoutError on Python 3.10 (a supported runtime), so total-timeout failures escape SolarEdgeV2Error.
• 🟢 A non-JSON 200 response (ContentTypeError) is reported as 'Connection failed' instead of InvalidResponseError.
• 🟢 The token is fetched before the budget hook, which may wait long enough for it to expire.
• 🟢 A trailing null sample turns the whole day's battery delta into None, despite the 'first-to-last observed' wording.
• 🟢 The unit-factor table and conversion logic are duplicated between v2.py and monitoring.py.
• 🟢 The new clients are not exported from the package or documented (CONTRIBUTING asks for docs on significant features).

---

🟢 Suggestions

1. Non-JSON 200 body is reported as a connection failure
src/aiosolaredge/v2.py:163-167

response.json() raises aiohttp.ContentTypeError when a 200 response is not application/json, for example an HTML maintenance page from a proxy or CDN. ContentTypeError is a ClientResponseError subclass, so the ClientError branch catches it first. The result is SolarEdgeV2Error("Connection failed", http_status=200) instead of InvalidResponseError("Invalid JSON response").

Why it matters: callers that treat contract violations differently from transport failures (retry vs. surface or repair) will get the wrong class.

Fix: add except ContentTypeError before the ClientError clause and map it to InvalidResponseError. Alternatively, call response.json(content_type=None) so only real decode errors (ValueError) remain.

2. Access token is fetched before the budget hook may wait
src/aiosolaredge/v2.py:119-121

_get awaits self._access_token() and then awaits before_request(). The hook exists to reserve a shared request budget, so it may well sleep, e.g. waiting out a per-minute window. A token taken just before a long wait can expire before the request is sent, which produces a spurious AuthenticationError.

Fix: reserve the budget first, then get the token right before I/O. Swapping these two awaits costs nothing.

        token = await self._access_token()
        if self._before_request is not None:
            await self._before_request()

3. Trailing null sample drops the whole day's battery delta
src/aiosolaredge/monitoring.py:208-216

The PR describes this as a 'first-to-last observed' delta. The code, though, needs samples[0] and samples[-1] themselves to be non-null; it does not use the first and last non-null values in observed.

Why it matters: if the newest quarter-hour comes back null (power() shows the API does this for not-yet-reported intervals), the per-battery value becomes None, and so does the site aggregate. A whole day of valid observed data is discarded until the next sample arrives, so the sensor flickers to unknown.

The test case ([0, None], None) locks this in on purpose, so it may be intended. If so, say so in the docstring. If not, compute the delta from observed[0]/observed[-1] when len(observed) >= 2, and use the timestamp of the last non-null sample.

                    raise InvalidResponseError("Battery counter reset")
                value = None
                if (
                    len(samples) >= 2
                    and samples[0].value is not None
                    and samples[-1].value is not None
                ):
                    value = _number(samples[-1].value - samples[0].value)
                # No fake midnight value: preserve V1's observed first/last delta.

4. Unit-factor table and conversion duplicated across modules
src/aiosolaredge/v2.py:179-189

lifetime_energy builds its own {"WH": 1, "KWH": 1000, ...} map inline. monitoring.py:17 defines the same table as ENERGY_FACTORS, along with an energy() helper that does the same lookup, null handling and post-multiplication overflow check. Meanwhile, monitoring.py imports the private _number, _site_path and _timestamp from v2.

The layering is backwards: the shared normalization lives in the subclass module, while the base module copies it.

Fix: move ENERGY_FACTORS/energy() into v2.py, or into a small _normalize.py that both modules import. Then lifetime_energy becomes energy(data["energy"], data["unit"]) plus the null check, and a future unit (e.g. MWh casing) only has to be added in one place.

---

Checklist

• No hardcoded secrets
• Input validation at boundaries
• Errors are typed and redacted on all live paths — suggestion ＃1
• Correct handling of null/missing measurement data — suggestion ＃3
• No duplicated canonical helpers — suggestion ＃4
• Backward compatibility of existing V1 API

---

Silent Failure Analysis

🟡 **5. MEDIUM** — misclassified error / dead error branch
src/aiosolaredge/v2.py:163-166

Risk: pyproject.toml supports Python ^3.10. On 3.10, asyncio.TimeoutError is a different class from the builtin TimeoutError, so a ClientTimeout(total=30) expiry skips this handler and reaches callers as a raw exception that except SolarEdgeV2Error does not catch.

data = await response.json()
except (ClientError, TimeoutError):
    raise SolarEdgeV2Error("Connection failed", http_status=status) from None
except ValueError:
    raise InvalidResponseError("Invalid JSON response") from None

Fix: Catch asyncio.TimeoutError explicitly (it is the same class as TimeoutError on 3.11+) and add a timeout test.

🟡 **6. MEDIUM** — misclassified error
src/aiosolaredge/v2.py:162-166

Risk: If a 200 response has a non-JSON content type (for example an HTML maintenance or proxy page), aiohttp raises ContentTypeError, which is a ClientError. It gets reported as a generic transient "Connection failed" instead of InvalidResponseError, so a contract violation looks like a network hiccup and gets retried.

data = await response.json()
except (ClientError, TimeoutError):
    raise SolarEdgeV2Error("Connection failed", http_status=status) from None

Fix: Catch aiohttp.ContentTypeError before ClientError and map it to InvalidResponseError.

🟡 **7. MEDIUM** — silent null on missing data
src/aiosolaredge/monitoring.py:87-95

Risk: If a whole production or consumption section is missing (schema drift or an endpoint regression), every derived energy sensor becomes null without an error, and that looks the same as a legitimately null field.

part = data.get(section)
if part is None:
    value = None
elif not isinstance(part, dict):
    raise InvalidResponseError("Invalid overview section")

Fix: Raise InvalidResponseError when a required section key is missing, and keep None only for an explicit null section or field if the API documents that.

🟡 **8. MEDIUM** — asymmetric membership check / silent null
src/aiosolaredge/monitoring.py:174-178

Risk: An unknown battery in telemetry raises a typed error. An inventoried battery missing from the response (removed or replaced device) instead becomes a null reading, and the storage totals turn null with no signal that membership changed.

if seen.difference(serials):
    raise StorageMembershipChanged("Storage membership requires inventory")
...
for serial in serials:
    for key in ("charge", "discharge"):
        result.setdefault(f"{serial}_battery_{key}_energy", Reading(None, now))

Fix: Also raise StorageMembershipChanged when set(serials) - seen is non-empty. If missing batteries are meant to be tolerated, document that and test it.

---

Automated review by Kōan (Claude) HEAD=5091d68 1 min 31s

[halibsbot]

Tip

No blocking issues found — ready to merge.