Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a phase iteration bug in the Shelly device integration. Some Shelly devices (e.g., Shelly EM) may return fewer than 3 phases, while others can return extra entries in their meters/emeters JSON arrays beyond the expected 3 phases (e.g., Shelly 3EM-63 Gen3). The old code iterated over all entries unconditionally, which caused either index-out-of-range errors or incorrect data assignment. The fix caps iteration at 3 and also refactors local ['a', 'b', 'c'] lists into a shared constants.py.
Changes:
- Introduced
constants.pywith a sharedALPHABETICAL_INDEXconstant to replace repeated local['a', 'b', 'c']lists inbat.py,counter.py, andinverter.py. - Changed
for i in range(len(meters))tofor i in range(0, min(3, len(meters)))in all three component files for Gen 1 meter types, preventing out-of-bounds list access when more than 3 meters are present. - Replaced guarded conditional expressions (
x if key else 0) with.get(key, 0)for cleaner and safer dict access across all three component files.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
constants.py |
New file introducing shared ALPHABETICAL_INDEX constant |
bat.py |
Applies iteration cap, uses .get() for safe field access, and moves power = 0 inside the try block |
counter.py |
Applies iteration cap, uses .get() for safe field access, and uses .get() for total_act_power |
inverter.py |
Applies iteration cap, uses .get() for safe field access, but still uses direct key access for total_act_power on line 69 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for i in range(0, min(3, len(meters))): | ||
| powers[(i+self.phase-1) % 3] = float(meters[i].get('power', 0)) * self.factor |
There was a problem hiding this comment.
The PR description and referenced issue state that the bug occurs with devices that have more than 3 entries in their meters or emeters array (e.g., Shelly 3EM-63 Gen3 which may include extra non-phase entries). The core fix — capping iteration at 3 with min(3, len(meters)) — is not covered by any new test case. Given that the test file (shelly_test.py) has comprehensive parametrized test cases for all existing device types and scenarios, a test case using a meters/emeters list with more than 3 items would be needed to verify the fix and prevent regressions.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Es gibt normal dreiphasige Shellys (Shelly EM) die dann weniger Phasen haben, aber auch Shellys bei denen noch andere Werte im meters-JSON sind.
basiert auf #3132