feat: add Enphase Envoy powermeter backend#245
feat: add Enphase Envoy powermeter backend#245CoRfr wants to merge 1 commit intotomquist:developfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds Enphase Envoy support: new ENVOY config example and GENERAL:DEVICE_IDS option, config loader wiring to instantiate Envoy powermeters, a new powermeter.envoy implementation with Enlighten token handling and multi‑phase parsing, tests, and masking of sensitive values in ha_addon/run.sh. Changes
Sequence Diagram(s)sequenceDiagram
participant Envoy as Envoy Class
participant Enlighten as Enlighten Cloud
participant EnvoyDevice as Envoy Device (local)
participant Session as requests.Session
rect rgba(100, 150, 200, 0.5)
note over Envoy,Enlighten: Token obtain / refresh
Envoy->>Enlighten: POST /login (username, password)
Enlighten-->>Envoy: session_id
Envoy->>Enlighten: POST /token (session_id, serial)
Enlighten-->>Envoy: JWT token
Envoy->>Envoy: Store token
end
rect rgba(150, 150, 100, 0.5)
note over Envoy,EnvoyDevice: Production data fetch (with retry on 401)
Envoy->>Session: GET https://host/production.json?details=1 (Authorization: Bearer token)
alt 401 Response
Session-->>Envoy: 401 Unauthorized
Envoy->>Enlighten: obtain_token(...)
Enlighten-->>Envoy: new JWT token
Envoy->>Session: GET https://host/production.json?details=1 (Authorization: Bearer new_token)
Session->>EnvoyDevice: GET request
EnvoyDevice-->>Session: 200 + production.json
Session-->>Envoy: 200 + JSON
else 200 Response
Session->>EnvoyDevice: GET request
EnvoyDevice-->>Session: 200 + production.json
Session-->>Envoy: 200 + JSON
end
Envoy->>Envoy: Parse consumption (net or total fallback), build phase watts
Envoy-->>Caller: Return phase watts array
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
CoRfr
changed the title
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@powermeter/envoy.py`:
- Around line 121-129: The current logic in powermeter/envoy.py silently falls
back from "net-consumption" to "total-consumption" via the
_find_measurement(consumption_list, "total-consumption") call; remove that
unconditional fallback so that if _find_measurement(consumption_list,
"net-consumption") returns None the code logs and raises immediately (preserve
the existing logger.error and raise ValueError) OR implement an explicit
configuration flag (e.g., allow_total_fallback or USE_TOTAL_CONSUMPTION) that
must be true to perform the fallback; update the code paths around net_meter and
the uses of net_meter to rely only on the configured behavior and ensure the
error path remains when fallback is not enabled.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 67ff73f8-36af-462f-a166-f8d6ed7c8c71
📒 Files selected for processing (5)
config.ini.exampleconfig/config_loader.pypowermeter/__init__.pypowermeter/envoy.pypowermeter/envoy_test.py
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@main.py`:
- Line 207: When parsing cfg_device_ids into device_ids, filter out empty tokens
after splitting and stripping to avoid blank device IDs from trailing or
consecutive commas; change the comprehension that builds device_ids (currently
using cfg_device_ids.split(",") and strip) to include a truthiness check so only
non-empty strings are kept (mirror the filtering used in
config/config_loader.py) so variable device_ids contains only valid IDs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9f2c31e5-f085-489e-91e7-6b875a140cdc
📒 Files selected for processing (2)
config.ini.examplemain.py
🚧 Files skipped from review as they are similar to previous changes (1)
- config.ini.example
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
powermeter/envoy.py (1)
86-87: Global warning suppression affects entire process.
urllib3.disable_warnings()is a process-wide operation that will suppressInsecureRequestWarningfor all HTTP clients, not just this Envoy instance. If other parts of the application rely on these warnings for debugging, they'll be silently suppressed.Consider using a context manager or suppressing warnings at the call site instead:
♻️ Suggested approach using warnings filter
- if not verify_ssl: - urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning) + self._suppress_ssl_warnings = not verify_sslThen in
_fetch, wrap the request:import warnings def _fetch(self): # ... with warnings.catch_warnings(): if self._suppress_ssl_warnings: warnings.filterwarnings("ignore", category=urllib3.exceptions.InsecureRequestWarning) response = self.session.get(url, headers=headers, verify=self.verify_ssl, timeout=10) # ...🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@powermeter/envoy.py` around lines 86 - 87, The current use of urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning) in the module-wide scope suppresses warnings process-wide; instead, remove that global call and limit suppression to the request site: update the Envoy class to track the per-instance flag (e.g., verify_ssl or self._suppress_ssl_warnings) and inside the _fetch method wrap the session.get call with warnings.catch_warnings() and warnings.filterwarnings("ignore", category=urllib3.exceptions.InsecureRequestWarning) only when the instance-level flag is set, while passing verify=self.verify_ssl to session.get so only this instance’s requests have suppressed warnings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@powermeter/envoy.py`:
- Around line 134-138: The current branch for self.phases > 1 builds watts as
[int(line.get("wNow", 0)) for line in lines[:3]] which can produce fewer than
self.phases elements if net_meter.get("lines") is short; update the method that
reads net_meter (use the existing self.phases, net_meter, and lines symbols) to
ensure the returned list has exactly self.phases entries by taking up to
self.phases values from lines, converting each to int with a default of 0, and
then padding with zeros (or truncating) as necessary so callers always receive a
list of length self.phases.
---
Nitpick comments:
In `@powermeter/envoy.py`:
- Around line 86-87: The current use of
urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning) in the
module-wide scope suppresses warnings process-wide; instead, remove that global
call and limit suppression to the request site: update the Envoy class to track
the per-instance flag (e.g., verify_ssl or self._suppress_ssl_warnings) and
inside the _fetch method wrap the session.get call with
warnings.catch_warnings() and warnings.filterwarnings("ignore",
category=urllib3.exceptions.InsecureRequestWarning) only when the instance-level
flag is set, while passing verify=self.verify_ssl to session.get so only this
instance’s requests have suppressed warnings.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 62792794-e9bd-4b55-bdf7-cf0756abda67
📒 Files selected for processing (1)
powermeter/envoy.py
|
Addressed all CodeRabbit review feedback in b84cab4:
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
powermeter/envoy_test.py (1)
81-96: Add a regression test for shortlinesarrays with multi-phase config.The new padding behavior in
Envoy.get_powermeter_watts()should be explicitly asserted (e.g.,phases=3with only 1 line returns[x, 0, 0]).Suggested test addition
class TestEnvoyThreePhase(unittest.TestCase): @@ def test_get_powermeter_watts_three_phase(self, mock_session_class): @@ self.assertEqual(result, [-100, -80, -120]) + + `@patch`("powermeter.envoy.requests.Session") + def test_three_phase_pads_missing_lines(self, mock_session_class): + data = { + "production": [], + "consumption": [ + { + "measurementType": "net-consumption", + "lines": [{"wNow": -150.0}], + } + ], + } + mock_response = MagicMock() + mock_response.json.return_value = data + mock_response.status_code = 200 + mock_response.raise_for_status = MagicMock() + mock_session = MagicMock() + mock_session.get.return_value = mock_response + mock_session_class.return_value = mock_session + + envoy = Envoy(host="192.168.1.200", token="test-token", phases=3) + self.assertEqual(envoy.get_powermeter_watts(), [-150, 0, 0])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@powermeter/envoy_test.py` around lines 81 - 96, Add a regression test in envoy_test.py that exercises Envoy.get_powermeter_watts() with phases=3 and a mocked API response whose "lines" array contains only a single entry; assert the method returns a 3-element list where the first element is the measured value and the remaining two elements are zeros (e.g., [x, 0, 0]). Use the existing patch of requests.Session and MagicMock response setup like in TestEnvoyThreePhase to return SAMPLE_RESPONSE modified to have a single-line payload, and name the test to indicate it verifies padding behavior for Envoy.get_powermeter_watts().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@powermeter/envoy.py`:
- Around line 74-90: Validate the phases parameter in the Envoy class __init__:
ensure phases is an integer greater than 0 and raise a ValueError with a clear
message if not (e.g., "phases must be a positive integer"); update the __init__
(constructor) to perform this check before assigning self.phases so invalid
configs fail fast and cannot produce malformed outputs.
- Around line 128-132: The code assumes data.get("consumption") is a list before
calling _find_measurement, which will raise if it's None or another type; update
the parsing after data = self._fetch() to validate and coerce the consumption
payload into a safe list (e.g., assign consumption_list =
data.get("consumption") and if it's not a list replace with []), then call
_find_measurement(consumption_list, "net-consumption") so net_meter handling is
robust against null or unexpected shapes.
---
Nitpick comments:
In `@powermeter/envoy_test.py`:
- Around line 81-96: Add a regression test in envoy_test.py that exercises
Envoy.get_powermeter_watts() with phases=3 and a mocked API response whose
"lines" array contains only a single entry; assert the method returns a
3-element list where the first element is the measured value and the remaining
two elements are zeros (e.g., [x, 0, 0]). Use the existing patch of
requests.Session and MagicMock response setup like in TestEnvoyThreePhase to
return SAMPLE_RESPONSE modified to have a single-line payload, and name the test
to indicate it verifies padding behavior for Envoy.get_powermeter_watts().
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: eea9ef88-9283-4534-bfac-56df79cd7d92
📒 Files selected for processing (3)
main.pypowermeter/envoy.pypowermeter/envoy_test.py
🚧 Files skipped from review as they are similar to previous changes (1)
- main.py
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ha_addon/run.sh`:
- Line 65: Update the sed masking command in run.sh that operates on "$CONFIG"
to accept optional whitespace around the equals sign so keys like "TOKEN =
value" are masked; adjust the regex used in the sed invocation that currently
targets (PASSWORD|TOKEN|ACCESSTOKEN|SECRET) to allow spaces around "=" and also
consider adding USERNAME to the matched keys for masking PII. Ensure the
modified sed still preserves case-insensitive matching and replaces the
right-hand side with a fixed mask while keeping the key name.
877a162 to
c727605
Compare
|
Thanks for the PR! This looks like a solid addition. One thing — it looks like this is hitting the Enphase cloud API for meter reads. Have you considered using the documented local API instead? The Envoy exposes the same data directly on the LAN, which would avoid the cloud dependency and latency for every read and would continue to work if the internet connection cuts out. According to the "Accessing IQ Gateway local APIs or UI using token" technical brief, there are several local endpoints that would work:
The cloud login would still be needed to obtain/renew the JWT token, but the actual meter reads could all happen locally. What do you think? |
Thanks for your suggestion, but, actually this is using the local API already for meter reading. |
Reads grid power from the Envoy's local API (/production.json?details=1) using net-consumption data. Supports single and three-phase reporting. JWT token is obtained from the Enphase Enlighten cloud API and refreshed automatically on 401. Actual meter reads are always local. - New powermeter: src/b2500_meter/powermeter/envoy.py - Config loader: ENVOY section with lazy import to avoid circular dependency - config.ini.example: document ENVOY section parameters - Unit tests for token flow, single/three-phase parsing, error handling Co-Authored-By: Claude Opus 4.6 <[email protected]>
CoRfr
force-pushed
the
feat-enphase-envoy
branch
from
f932036 to
9ccc3ef
Compare
| @@ -0,0 +1,152 @@ | |||
| from .base import Powermeter | |||
| import requests | |||
There was a problem hiding this comment.
The codebase migrated to asyncio after this PR was opened. All powermeter I/O now uses aiohttp.ClientSession. synchronous requests will block the event loop. This affects both the local Envoy calls and the Enlighten token flow. See shelly.py for the pattern.
| self.username = username | ||
| self.password = password | ||
| self.serial = serial | ||
| self.session = requests.Session() |
There was a problem hiding this comment.
HTTP-based powermeters need async def start() / async def stop() lifecycle methods now. The session should be created in start() and closed in stop(). The main loop calls these. Creating it in __init__ skips that contract.
| response.raise_for_status() | ||
| return response.json() | ||
|
|
||
| def get_powermeter_watts(self): |
There was a problem hiding this comment.
This needs to be async def to match the base class signature (async def get_powermeter_watts(self) -> list[float]).
| self.serial = serial | ||
| self.session = requests.Session() | ||
| if not verify_ssl: | ||
| urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning) |
There was a problem hiding this comment.
This suppresses SSL warnings globally for the entire process, affecting all other HTTP clients. Needs to be scoped to this powermeter only.
| ) | ||
|
|
||
| if self.phases == 1: | ||
| return [int(net_meter.get("wNow", 0))] |
There was a problem hiding this comment.
Nit: base class return type is list[float]. Other powermeters return floats. Consider using float() for consistency.
|
|
||
|
|
||
| class TestEnvoySinglePhase(unittest.TestCase): | ||
| @patch("powermeter.envoy.requests.Session") |
There was a problem hiding this comment.
Import path is wrong, should be b2500_meter.powermeter.envoy.requests.Session. Same issue in all test patches throughout this file.
| SHELLY_SECTION = "SHELLY" | ||
| TASMOTA_SECTION = "TASMOTA" | ||
| SHRDZM_SECTION = "SHRDZM" | ||
| ENVOY_SECTION = "ENVOY" |
There was a problem hiding this comment.
The __init__.py export is missing from this PR. Needs from .envoy import Envoy and an "Envoy" entry in __all__. Also missing a README configuration subsection.
| "grid CTs are required" | ||
| ) | ||
|
|
||
| if self.phases == 1: |
There was a problem hiding this comment.
Can we infer this from the response? For example, check if three lines are available; if not, fallback to single phase. I'm not sure if lines can be unavailable, but I see no reason to use the net power if individual phases are accessible.
2 participants
Summary
Envoypowermeter backend that reads net grid power from the Enphase Envoy local API (/production.json?details=1)net-consumptiontototal-consumptionfor Envoys without CTs on the gridFiles changed
powermeter/envoy.py—Envoyclass +obtain_token()helperpowermeter/envoy_test.py— 11 unit testspowermeter/__init__.py— exportEnvoyconfig/config_loader.py—ENVOYconfig section supportconfig.ini.example— documented configuration optionsConfiguration example
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Configuration
Enhancement
Security
Tests