feat: add site-data API endpoints (PR 1) by it-rec · Pull Request #126 · Solarlibs/aiosolaredge

it-rec · 2026-05-20T07:24:52Z

Adds the SolarEdge Monitoring API "Site Data" endpoints that were not yet wrapped by the client:

get_sites -> /sites/list
get_data_period -> /site/{id}/dataPeriod
get_data_period_bulk -> /sites/{ids}/dataPeriod
get_energy -> /site/{id}/energy
get_energy_bulk -> /sites/{ids}/energy
get_time_frame_energy[_bulk] -> /site|sites/.../timeFrameEnergy
get_power -> /site/{id}/power
get_power_bulk -> /sites/{ids}/power
get_overview_bulk -> /sites/{ids}/overview
get_power_details -> /site/{id}/powerDetails
get_environmental_benefits -> /site/{id}/envBenefits

Introduces shared infrastructure reused by later endpoint PRs: the Meter/TimeUnit/SystemUnits/SortOrder Literal aliases, the _format_date/_format_datetime/_join_ids helpers (so methods accept date, datetime or pre-formatted str), and the _get_sites_url builder for bulk calls.

One PR in a series splitting the full endpoint coverage work into reviewable pieces.

Adds the SolarEdge Monitoring API "Site Data" endpoints that were not yet wrapped by the client: - get_sites -> /sites/list - get_data_period -> /site/{id}/dataPeriod - get_data_period_bulk -> /sites/{ids}/dataPeriod - get_energy -> /site/{id}/energy - get_energy_bulk -> /sites/{ids}/energy - get_time_frame_energy[_bulk] -> /site|sites/.../timeFrameEnergy - get_power -> /site/{id}/power - get_power_bulk -> /sites/{ids}/power - get_overview_bulk -> /sites/{ids}/overview - get_power_details -> /site/{id}/powerDetails - get_environmental_benefits -> /site/{id}/envBenefits Introduces shared infrastructure reused by later endpoint PRs: the Meter/TimeUnit/SystemUnits/SortOrder Literal aliases, the _format_date/_format_datetime/_join_ids helpers (so methods accept date, datetime or pre-formatted str), and the _get_sites_url builder for bulk calls. One PR in a series splitting the full endpoint coverage work into reviewable pieces.

codecov · 2026-05-20T07:26:27Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

it-rec · 2026-05-20T21:35:31Z

@bluetoothbot review

bluetoothbot · 2026-05-20T21:41:27Z

❌ Permission denied. Only users with write access can trigger bot commands.

it-rec · 2026-05-21T07:04:25Z

@claude

it-rec · 2026-06-06T13:12:03Z

@bdraco I also have this open since a while now.

As you said there is much on your plate. No worries.

My intend was to just draw your attention to this that it doesn't fall through the cracks either.

bluetoothbot · 2026-06-22T16:07:59Z

@it-rec: @bdraco I also have this open since a while now.

Reviewed. Source merges clean. One blocker: new tests use aioresponses. Main dropped it in #136. Now uses aiointercept. Rebase, then port tests to async with aiointercept(...). Recheck the query-param regexes after porting.

bluetoothbot · 2026-06-22T16:08:01Z

PR Review — feat: add site-data API endpoints (PR 1)

Solid, well-structured endpoint additions blocked by a test-framework mismatch with current main.

The source code is clean and merges cleanly: the shared _format_date/_format_datetime/_join_ids helpers and _get_sites_url builder are well-factored, the new code correctly uses an immutable () default for meters (avoiding the mutable-default trap the older get_energy_details still has), docstrings are thorough, and the date/datetime/str overloads are handled correctly (datetime is a subclass of date).

Blocking: the eight new tests use aioresponses, which main removed in test: switch from aioresponses to aiointercept #136 (migrated to aiointercept) — verified absent from pyproject.toml and poetry.lock. Tests won't collect after rebase.
The multi-param URL regexes rely on aioresponses' alphabetical param sorting; must be re-verified under aiointercept.
Minor: confirm SystemUnits literal ("Metrics" vs "Metric") against the SolarEdge spec; add a test for the metric case.

🔴 Blocking

1. New tests use aioresponses, which main has removed
tests/test_init.py:113-313

All eight added tests are written against aioresponses (with aioresponses() as mocked:), but main switched the test stack to aiointercept in commit #136 (9f0abd5). I verified aioresponses is gone from both pyproject.toml and poetry.lock, and tests/test_init.py on main now imports from aiointercept import aiointercept and uses async with aiointercept(mock_external_urls=True) as mocked:.

Why it matters: after rebasing onto current main, these tests fail at collection — aioresponses is neither imported nor installed (NameError / missing dependency). CI will not run them, so the new endpoints ship effectively untested.

How to fix:

Rebase the branch on current main.
Migrate each new test to the existing pattern: async with aiointercept(mock_external_urls=True) as mocked: (note: async context manager, not sync with).
Re-verify the regex URL matchers against aiointercept's query-string semantics (see separate note on param ordering).

with aioresponses() as mocked:  # main now uses: async with aiointercept(mock_external_urls=True) as mocked:

🟡 Important

1. Query-param regexes assume alphabetical param ordering
tests/test_init.py:121-126

The get_sites matcher expects params in the order searchText, size, sortOrder, sortProperty, startIndex, status — alphabetical, not the insertion order the client builds (size, startIndex, searchText, sortProperty, sortOrder, status). This only matches because aioresponses sorts query params before regex matching.

Why it matters: when these tests are ported to aiointercept (required — see the critical finding), if aiointercept matches against the raw, insertion-ordered query string instead of a sorted one, this regex (and any other multi-param matcher here) silently fails to match and the test breaks or matches the wrong mock.

How to fix: after migrating, confirm aiointercept's matching order and either keep alphabetical or rewrite the patterns to insertion order. Prefer asserting on the parsed query params rather than ordering-sensitive substring regexes.

r"searchText=Lyon.*size=5.*sortOrder=DESC.*sortProperty=Name.*startIndex=10.*status=Active"

Checklist

No hardcoded secrets
Input validation at boundaries
No mutable default arguments
New code branches tested — critical ＃1
Tests run on current main — critical ＃1
Test matching robust to framework — warning ＃1
Public API values verified — suggestion ＃1

To rebase specific severity levels, mention me: @bluetoothbot rebase critical (fixes 🔴 only), @bluetoothbot rebase important (fixes 🔴 + 🟡), or just @bluetoothbot rebase for all.

Automated review by Kōan (Claude) HEAD=8e29fe3 2 min 46s

bluetoothbot

Blocking issues found.

New tests use aioresponses, which main has removed
Query-param regexes assume alphabetical param ordering

Copilot

Pull request overview

Adds SolarEdge Monitoring API “Site Data” endpoints to the async client, plus shared helpers/types to support date/datetime coercion and bulk-site URL construction.

Changes:

Implemented new Site Data API methods (single-site and bulk variants) on SolarEdge.
Added shared helpers (_format_date, _format_datetime, _join_ids, _get_sites_url) and Literal aliases for common parameter domains.
Extended test coverage with new async tests for the added endpoints.

Reviewed changes

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

File	Description
`src/aiosolaredge/solaredge.py`	Adds site-data endpoint wrappers plus shared helper/type infrastructure for query formatting and bulk URL building.
`tests/test_init.py`	Adds tests covering the new endpoints and their URL/query construction.

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

+        pattern = re.compile(
+            r"^https://monitoringapi\.solaredge\.com/sites/list\?.*"
+            r"searchText=Lyon.*size=5.*sortOrder=DESC.*sortProperty=Name.*"
+            r"startIndex=10.*status=Active"
+        )


+        pattern = re.compile(
+            r"^https://monitoringapi\.solaredge\.com/site/123/energy\?.*"
+            r"endDate=2013-05-30.*startDate=2013-05-01.*timeUnit=DAY"
+        )


+        pattern = re.compile(
+            r"^https://monitoringapi\.solaredge\.com/site/123/energy\?.*"
+            r"endDate=2013-05-30.*startDate=2013-05-01.*timeUnit=HOUR"
+        )


+        pattern = re.compile(
+            r"^https://monitoringapi\.solaredge\.com/sites/1,4/energy\?.*"
+            r"endDate=2013-05-30.*startDate=2013-05-01"
+        )


+        pattern = re.compile(
+            r"^https://monitoringapi\.solaredge\.com/site/123/timeFrameEnergy\?.*"
+            r"endDate=2013-05-06.*startDate=2013-05-01"
+        )


+        pattern = re.compile(
+            r"^https://monitoringapi\.solaredge\.com/sites/1,4/timeFrameEnergy\?.*"
+            r"endDate=2013-05-06.*startDate=2013-05-01"
+        )


+        pattern = re.compile(
+            r"^https://monitoringapi\.solaredge\.com/site/123/power\?.*"
+            r"endTime=2013-06-04.*startTime=2013-06-04"
+        )


+Meter = Literal["PRODUCTION", "CONSUMPTION", "SELFCONSUMPTION", "FEEDIN", "PURCHASED"]
+TimeUnit = Literal["QUARTER_OF_AN_HOUR", "HOUR", "DAY", "WEEK", "MONTH", "YEAR"]
+SystemUnits = Literal["Metrics", "Imperial"]
+SortOrder = Literal["ASC", "DESC"]


+def _join_ids(site_ids: Iterable[int | str]) -> str:
+    """Join site IDs with a comma for bulk API calls."""
+    return ",".join(str(site_id) for site_id in site_ids)


it-rec changed the title ~~feat: add site-data API endpoints~~ feat: add site-data API endpoints (PR 1) May 20, 2026

Merge branch 'main' into claude/se-api-site-data

8e29fe3

bluetoothbot suggested changes Jun 22, 2026

View reviewed changes

bdraco requested a review from Copilot June 22, 2026 16:11

Copilot started reviewing on behalf of bdraco June 22, 2026 16:11 View session

Copilot AI reviewed Jun 22, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

feat: add site-data API endpoints (PR 1)#126

feat: add site-data API endpoints (PR 1)#126
it-rec wants to merge 2 commits into
Solarlibs:mainfrom
it-rec:claude/se-api-site-data

it-rec commented May 20, 2026

Uh oh!

codecov Bot commented May 20, 2026

Uh oh!

it-rec commented May 20, 2026

Uh oh!

bluetoothbot commented May 20, 2026

Uh oh!

it-rec commented May 21, 2026

Uh oh!

it-rec commented Jun 6, 2026

Uh oh!

bluetoothbot commented Jun 22, 2026

Uh oh!

bluetoothbot commented Jun 22, 2026

Uh oh!

bluetoothbot left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Uh oh!

Conversation

it-rec commented May 20, 2026

Uh oh!

codecov Bot commented May 20, 2026

Codecov Report

Uh oh!

it-rec commented May 20, 2026

Uh oh!

bluetoothbot commented May 20, 2026

Uh oh!

it-rec commented May 21, 2026

Uh oh!

it-rec commented Jun 6, 2026

Uh oh!

bluetoothbot commented Jun 22, 2026

Uh oh!

bluetoothbot commented Jun 22, 2026

PR Review — feat: add site-data API endpoints (PR 1)

🔴 Blocking

🟡 Important

Checklist

Uh oh!

bluetoothbot left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants