-
-
Notifications
You must be signed in to change notification settings - Fork 30.9k
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
Change friendly names of all photovoltaic entities to "PV …" #131179
Conversation
There are two groups of "solar" production entities, one for photovotaic and one for thermal solar production in the vicare integration. Currently they both have identical friendly names labeling them as "Solar …". This leads to confusion and make it impossible to keep them apart. This PR fixes this by all photovoltaic entities friendly names that begin with "PV …". In addition the 'circulation_pump' entity is renamed as "Heating circulation pump" to clearly differentiate it from the "DHW circulation pump". Otherwise many will misunderstand these, in German for example a "Zirkulationspumpe" is always for DHW, while the "Heating circulation pump" is called "Heizkreisumwälzpumpe" or just "Heizkreispumpe".
Hey there @CFenner, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
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.
Hi there @NoRi2909 👋
CI is failing. Also, considering the branch name of your PR, is seems like this was done in the browser, if that is the case, this wouldn't be acceptable.
This changes logic in this integration, for which one locally needs to ensure the changes are correct and tested in a local development environment. The unit tests should pass before you'd open a PR.
Please pull this locally, test your changes and adapt the unit tests to make the CI pass.
../Frenck
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
@frenck Currently I can only do my PRs in the browser, so I'm afraid I cannot heat that problem in my own. Didn't know that changing the frienly names interferes with the logic. |
The test snapshots for the circulation pump are named by the entity names, as this changed, the snapshots are no longer found during testing. Please also adjust the names in the snapshots. |
Snapshots should never be manually touched. |
In that case, we have to close this PR; as we cannot accept PRs in these state. They should be tested and passing CI. Nevertheless, thanks for being willing to contribute 👍 ../Frenck |
Sorry for my mistakes here, still learning all the details of github etc. @CFenner perhaps you can do a working PR with the suggested changes. |
There are two groups of "solar" production entities, one for photovotaic and one for thermal solar production, in the vicare integration.
Currently they both have completely identical friendly names labeling them both as "Solar …".
This leads to confusion and makes it impossible to keep them apart.
This PR fixes this by giving all photovoltaic entities friendly names that begin with "PV …".
In addition the 'circulation_pump' entity is renamed as "Heating circulation pump" to clearly differentiate it from the "DHW circulation pump".
Otherwise many will misunderstand these, in German for example a "Zirkulationspumpe" is always for DHW, while the "Heating circulation pump" is called "Heizkreisumwälzpumpe" or just "Heizkreispumpe".
Proposed change
Change "Solar …" to "PV …" on all 'photovoltaic' keys.
Add "Heating …" to the 'circulation_pump' entity.
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
.To help with the load of incoming pull requests: