Allow multiple configs for srp energy #96573

briglx · 2023-07-15T00:24:26Z

Breaking change

Proposed change

Allow multiple configurations for different locations. This includes adding a separate device for each location. The sensor name uses the device name to distinguish between the two energy consumption sensors.

Type of change

Dependency upgrade
Bugfix (non-breaking change which fixes an issue)
New integration (thank you!)
New feature (which adds functionality to an existing integration)
Deprecation (breaking change to happen in the future)
Breaking change (fix/feature causing existing functionality to break)
Code quality improvements to existing code or addition of tests

Additional information

This PR fixes or closes issue: fixes srp_engry doesn't create any devices or entities #95578
This PR is related to issue:
Link to documentation pull request:

Checklist

The code change is tested and works locally.
Local tests pass. Your PR cannot be merged unless tests pass
There is no commented out code in this PR.
I have followed the development checklist
I have followed the perfect PR recommendations
The code has been formatted using Black (black --fast homeassistant tests)
Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

Documentation added/updated for www.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

The manifest file has all fields filled out correctly.
Updated and included derived files by running: python3 -m script.hassfest.
New or updated dependencies have been added to requirements_all.txt.
Updated by running python3 -m script.gen_requirements_all.
For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
Untested files have been added to .coveragerc.

To help with the load of incoming pull requests:

I have reviewed two other open pull requests in this repository.

joostlek

The tests are still failing, can you take a look?

home-assistant · 2023-07-15T00:55:05Z

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

gjohansson-ST · 2023-08-19T14:52:17Z

How is this PR solving the issue by adding a location name?
Also why would there be a good thing to add multiple entries just with different location names?

briglx · 2023-08-22T02:34:14Z

@gjohansson-ST This is part of an overall update to the integration that is split out between several PRs. This is an energy integration where a user could have several different homes/locations to monitor their energy. Part of the reason why there is an issue is because of the way the upgrade to the integration is split out between multiple PRs

briglx · 2023-08-22T02:35:02Z

@joostlek it's been a while since you asked for code changes. I've made the changes. Can you please review and approve?

homeassistant/components/srp_energy/config_flow.py

homeassistant/components/srp_energy/sensor.py

frenck · 2023-10-08T15:08:10Z

A small and friendly notification.

Are you planning on moving forward on this PR?
I didn't see any activity recently, while there are still things to do. I hope you consider picking it up soon! 👍

../Frenck

briglx · 2023-10-16T18:05:57Z

@frenck yes, I'll take a look this week. Thanks

briglx · 2023-10-17T23:46:05Z

@gjohansson-ST and @joostlek can you please check the review?

briglx · 2023-11-22T15:32:02Z

@frenck can you help move this forward. All the changes have been made. Just need a final review

homeassistant/components/srp_energy/config_flow.py

homeassistant/components/srp_energy/const.py

homeassistant/components/srp_energy/sensor.py

briglx · 2023-12-13T22:04:10Z

@gjohansson-ST let me know if you have any more concerns or feedback.

emontnemery

LGTM, thanks @briglx 👍

stale

MartinHjelmare

Please address the comments in a new PR. Thanks!

MartinHjelmare · 2023-12-14T10:21:29Z

homeassistant/components/srp_energy/strings.json

@@ -17,7 +17,7 @@
      "unknown": "[%key:common::config_flow::error::unknown%]"
    },
    "abort": {
-      "single_instance_allowed": "[%key:common::config_flow::abort::single_instance_allowed%]"
+      "already_configured": "[%key:common::config_flow::abort::already_configured_device%]"


Change the reference to already_configured_service.

MartinHjelmare · 2023-12-14T10:22:23Z

homeassistant/components/srp_energy/config_flow.py

        return self.async_show_form(
            step_id="user",
            data_schema=vol.Schema(
                {
+                    vol.Required(
+                        CONF_NAME, default=self.hass.config.location_name


Don't ask for the name here. Just default the title to the location name. The user can change the config entry title afterwards if they want.

home-assistant bot added bugfix cla-signed code-quality has-tests integration: srp_energy by-code-owner Quality Scale: No score labels Jul 15, 2023

joostlek changed the title ~~Allow multiple configs.~~ Allow multiple configs for srp energy Jul 15, 2023

joostlek previously requested changes Jul 15, 2023

View reviewed changes

home-assistant bot marked this pull request as draft July 15, 2023 00:55

briglx marked this pull request as ready for review July 17, 2023 23:32

home-assistant bot requested a review from joostlek July 17, 2023 23:32

gjohansson-ST requested changes Aug 22, 2023

View reviewed changes

home-assistant bot marked this pull request as draft August 22, 2023 06:47

briglx force-pushed the srp_energy_202307.coordinator branch from e75f2be to 69ebef4 Compare August 22, 2023 15:54

briglx force-pushed the srp_energy_202307.coordinator branch from bbd724f to 152979c Compare October 17, 2023 23:22

briglx marked this pull request as ready for review October 17, 2023 23:38

home-assistant bot requested a review from gjohansson-ST October 17, 2023 23:38

gjohansson-ST previously requested changes Nov 24, 2023

View reviewed changes

home-assistant bot marked this pull request as draft November 24, 2023 18:25

briglx added 2 commits December 13, 2023 21:37

Allow multiple configs.

dcab32e

Rename test configs.

4b1b955

briglx added 3 commits December 13, 2023 21:37

Remove unused property

448aa9a

Merge branch 'dev' into srp_energy_202307.coordinator

38de1db

Use title in device name.

a00664c

briglx force-pushed the srp_energy_202307.coordinator branch from 2940af4 to a00664c Compare December 13, 2023 21:55

briglx marked this pull request as ready for review December 13, 2023 22:03

home-assistant bot requested a review from gjohansson-ST December 13, 2023 22:03

emontnemery approved these changes Dec 14, 2023

View reviewed changes

emontnemery merged commit 7721840 into home-assistant:dev Dec 14, 2023
23 checks passed

MartinHjelmare reviewed Dec 14, 2023

View reviewed changes

MartinHjelmare mentioned this pull request Dec 14, 2023

Address srp_energy late review comment #105756

Merged

20 tasks

github-actions bot locked and limited conversation to collaborators Dec 15, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow multiple configs for srp energy #96573

Allow multiple configs for srp energy #96573

briglx commented Jul 15, 2023

joostlek left a comment

home-assistant bot commented Jul 15, 2023

gjohansson-ST commented Aug 19, 2023

briglx commented Aug 22, 2023

briglx commented Aug 22, 2023

frenck commented Oct 8, 2023

briglx commented Oct 16, 2023

briglx commented Oct 17, 2023 •

edited

briglx commented Nov 22, 2023

briglx commented Dec 13, 2023

emontnemery left a comment

MartinHjelmare left a comment

MartinHjelmare Dec 14, 2023

MartinHjelmare Dec 14, 2023 •

edited

Allow multiple configs for srp energy #96573

Allow multiple configs for srp energy #96573

Conversation

briglx commented Jul 15, 2023

Breaking change

Proposed change

Type of change

Additional information

Checklist

joostlek left a comment

Choose a reason for hiding this comment

home-assistant bot commented Jul 15, 2023

gjohansson-ST commented Aug 19, 2023

briglx commented Aug 22, 2023

briglx commented Aug 22, 2023

frenck commented Oct 8, 2023

briglx commented Oct 16, 2023

briglx commented Oct 17, 2023 • edited

briglx commented Nov 22, 2023

briglx commented Dec 13, 2023

emontnemery left a comment

Choose a reason for hiding this comment

MartinHjelmare left a comment

Choose a reason for hiding this comment

MartinHjelmare Dec 14, 2023

Choose a reason for hiding this comment

MartinHjelmare Dec 14, 2023 • edited

Choose a reason for hiding this comment

briglx commented Oct 17, 2023 •

edited

MartinHjelmare Dec 14, 2023 •

edited