New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add phase entities to Enphase Envoy #108725
Conversation
Hey there @bdraco, @cgarwood, @dgomes, @joostlek, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
@@ -104,6 +105,7 @@ class EnvoyProductionSensorEntityDescription( | |||
suggested_unit_of_measurement=UnitOfPower.KILO_WATT, | |||
suggested_display_precision=3, | |||
value_fn=lambda production: production.watts_now, | |||
translation_placeholders={"phase_name": ""}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we unset these?
translation_placeholders={"phase_name": ""}, |
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
Move device name logic to separate function. Refactor native value for phases Use dataclasses.replace for phase entities, add on-phase to base class as well, no need for phase entity descriptions anymore
Move model determination to library. Revert states test for future split to sensor test.
I think its good to go from my perspective. I'll test it shortly |
Works fine on my systems but they are all 2 phase |
I approved it but didn't merge to give joostlek a chance top followup on his comment #108725 (comment) in case any more changes are needed |
fwiw I ran it with my envoy simulator using the 3 phase fixture files from pyenphase. |
entities.extend( | ||
EnvoyConsumptionPhaseEntity(coordinator, description) | ||
for use_phase, phase in envoy_data.system_consumption_phases.items() | ||
for description in CONSUMPTION_PHASE_SENSORS[PhaseNames(use_phase)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we could simplify this further and make use_phase
the key to it becomes CONSUMPTION_PHASE_SENSORS[use_phase]
and the dict construction above is adjusted accordingly
Sorry for missing this one the first go.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The linter didn't like the
for description in CONSUMPTION_PHASE_SENSORS[use_phase]
when the walrus was added and the PhaseNames
had to be added. To optimize it we need to change the definition in the EnvoyConsumptionRequiredKeysMixin to str
on_phase: str | None
and make sure the key is str
CONSUMPTION_PHASE_SENSORS = {
(on_phase := str(PhaseNames(PHASENAMES[phase]))): [
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No worries. Let's leave it as is
* add phase entities to Enphase Envoy * Implement review feedback for translation strings * Enphase Envoy multiphase review changes Move device name logic to separate function. Refactor native value for phases Use dataclasses.replace for phase entities, add on-phase to base class as well, no need for phase entity descriptions anymore * Enphase Envoy reviewe feedback Move model determination to library. Revert states test for future split to sensor test. * Enphase_Envoy use model description from pyenphase library * Enphase_Envoy refactor Phase Sensors * Enphase_Envoy use walrus in phase sensor --------- Co-authored-by: J. Nick Koston <nick@koston.org>
* add phase entities to Enphase Envoy * Implement review feedback for translation strings * Enphase Envoy multiphase review changes Move device name logic to separate function. Refactor native value for phases Use dataclasses.replace for phase entities, add on-phase to base class as well, no need for phase entity descriptions anymore * Enphase Envoy reviewe feedback Move model determination to library. Revert states test for future split to sensor test. * Enphase_Envoy use model description from pyenphase library * Enphase_Envoy refactor Phase Sensors * Enphase_Envoy use walrus in phase sensor --------- Co-authored-by: J. Nick Koston <nick@koston.org>
Proposed change
Enphase Envoy metered with current transformer (CT) clamps installed on multiple phases provides data at the phase level in addition to the overall summary. This PR add entities for phase production and consumption data. Entity names are the same as the overall summary entities with phase names l1, l2 or l3 post-fixed.
Type of change
Additional information
Checklist
ruff format homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
..coveragerc
.To help with the load of incoming pull requests: