Add sensors for myStrom plugs #97024

MadMonkey87 · 2023-07-21T18:15:54Z

Proposed change

The myStrom power plugs do measure power consumption and temperature too, but the current HA is only representing them as a switch. This PR provides two more sensor entities to bring consumption & temperature into HA too.

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 Extract mystrom switch energy attributes into sensors #53319 fixes New myStrom integration lacking entities for plugs #95979
This PR is related to issue:
Link to documentation pull request: Adding documentation for new sensors on myStrom switches home-assistant.io#29144

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.

home-assistant · 2023-07-21T18:15:57Z

Hi @MadMonkey87

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

home-assistant · 2023-07-21T18:16:00Z

Hey there @fabaff, mind taking a look at this pull request as it has been labeled with an integration (mystrom) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of mystrom can trigger bot actions by commenting:

@home-assistant close Closes the pull request.
@home-assistant rename Awesome new title Renames the pull request.
@home-assistant reopen Reopen the pull request.
@home-assistant unassign mystrom Removes the current integration label and assignees on the pull request, add the integration domain after the command.

home-assistant · 2023-07-21T18:16:38Z

Hi @MadMonkey87

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

joostlek

Some initial feedback

homeassistant/components/mystrom/sensor.py

homeassistant/components/mystrom/switch.py

home-assistant · 2023-07-21T18:35:30Z

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.

joostlek · 2023-07-21T19:00:53Z

Oh and fixes #53319 fixes #95979

Edit: Oh apparently "fixes" doesnt work in comments.

joostlek · 2023-07-21T19:05:55Z

I worked on some bugs in this release and I noticed that not every device is consistent with the rest, which ones do you have?

It might be a good idea to add tests so we don't break this in a regression again, but I can get that it's a big step for a first PR. If you want I can help you with that!

MadMonkey87 · 2023-07-21T20:44:24Z

I worked on some bugs in this release and I noticed that not every device is consistent with the rest, which ones do you have?

It might be a good idea to add tests so we don't break this in a regression again, but I can get that it's a big step for a first PR. If you want I can help you with that!

That would be very helpful, thx! Jep this is the very first time I am working on HA ;P

PS: I have some legacy plugs (the ones with power measurement, but no temperature sensor and limited reporting) as well as a newer plug and some of the buttons

joostlek · 2023-07-21T23:04:35Z

So if you don't have a temperature sensor, we should look if it works, and if it doesn't, how we work with it :). But that's something for tomorrow

MadMonkey87 · 2023-07-25T15:01:35Z

have some legacy plugs (the ones with power measurement, but no temperature sensor and limited reporting) as well as a newer plug and s

I have just one of the newer ones with temperature sensor, so it works;P

joostlek · 2023-08-31T16:31:46Z

Looks like I need to provide a documentation PR? Is this really needed for this changes?

It would be nice to have some documentation for people to read about this nifty feature you added ;).

Doesn't have to be huge, it can be just a sentence like, yea and the plugs have two sensors for temperature and humidity.

Updated documentation for home-assistant/core#97024

edenhaus

Please fix the CI and make sure it is passing.
Please also implement the requested changes of @gjohansson-ST

edenhaus · 2023-09-01T07:49:14Z

homeassistant/components/mystrom/__init__.py

@@ -16,7 +16,7 @@
 from .const import DOMAIN
 from .models import MyStromData

-PLATFORMS_SWITCH = [Platform.SWITCH]
+PLATFORMS_SWITCH = [Platform.SWITCH, Platform.SENSOR]


Please do the renaming in this PR, as it will improve the readability of the code. This constant is only used in two places in this file.

Siggy101 · 2023-09-11T07:35:06Z

Happy Monday everyone.
Is there any chance that this new feature will go live in the near future? I am not familiar with the process, so please forgive my ignorance, but it appears that the hard work has been done and there are some documentation and validation remaining.

edenhaus · 2023-09-11T07:43:10Z

Is there any chance that this new feature will go live in the near future?

This depends on @MadMonkey87, which is required to implement the requested changes. For this reason, the PR is in draft. We can merge the PR only if all is done, including creating a PR for the documentation.

Siggy101 · 2023-09-11T08:31:00Z

Is there any chance that this new feature will go live in the near future?

This depends on @MadMonkey87, which is required to implement the requested changes. For this reason, the PR is in draft. We can merge the PR only if all is done, including creating a PR for the documentation.

Thanks for the clarification.

@MadMonkey87 Do you have a timeline to do these last bits so we can all benefit from your hard work? I would really love to be able to update my HA install without killing all of my automations. Thank you!

Siggy101 · 2023-09-19T07:55:47Z

Is there any chance that this new feature will go live in the near future?

This depends on @MadMonkey87, which is required to implement the requested changes. For this reason, the PR is in draft. We can merge the PR only if all is done, including creating a PR for the documentation.

Thanks for the clarification.

@MadMonkey87 Do you have a timeline to do these last bits so we can all benefit from your hard work? I would really love to be able to update my HA install without killing all of my automations. Thank you!

@MadMonkey87 Ciao! Is there any way you can find the time to complete the docs etc to get these really valuable sensors pushed to be merged?
I can imagine that you are very busy but some of us will be extremely grateful to be able to use your hard work.
Thanks!

jdebetaz · 2023-10-04T08:23:39Z

Hello,
As it takes time, I made the suggested changes in the code. The only question is whether I should open a new PR on his repo or on this one?

PR documentation will follow in the next few days.

Kind regards 😄

gjohansson-ST · 2023-10-04T08:26:51Z

Hello, As it takes time, I made the suggested changes in the code. The only question is whether I should open a new PR on his repo or on this one?

We don't want a new PR on this as we already have it here. You can always open a PR on @MadMonkey87 's branch to contribute from the back and see if he's still active.

jdebetaz · 2023-10-04T09:42:58Z

A PR has been made to update the documentation. You'll find it here

frenck · 2023-10-08T17:16:14Z

There is still a mypy error that needs to be addressed. Please don't forget to mark this PR ready for review, once you are done 👍

../Frenck

jdebetaz · 2023-10-08T17:34:51Z

@frenck As explained above, I opened the PR on @MadMonkey87's repo so as not to have duplicate PRs

homeassistant/components/mystrom/sensor.py

Changes addressed

MartinHjelmare

Please address the comments in a new PR. Thanks!

MartinHjelmare · 2023-10-18T20:09:22Z

homeassistant/components/mystrom/sensor.py

+    """Representation of the consumption or temperature of a myStrom switch/plug."""
+
+    entity_description: MyStromSwitchSensorEntityDescription
+    device: MyStromSwitch


We don't need a type annotation and class attribute for device here as there's already a type annotation of the entity device parameter.

MartinHjelmare · 2023-10-18T20:10:28Z

homeassistant/components/mystrom/__init__.py

@@ -16,7 +16,7 @@
 from .const import DOMAIN
 from .models import MyStromData

-PLATFORMS_SWITCH = [Platform.SWITCH]
+PLATFORMS_PLUGS = [Platform.SWITCH, Platform.SENSOR]


Please sort 🔡.

MadMonkey87 added 2 commits July 21, 2023 17:50

support sensors for myStrom plugs

92c1724

added myStrom sensor to coveragerc

5ce5e6a

MadMonkey87 requested a review from fabaff as a code owner July 21, 2023 18:15

home-assistant bot added cla-needed integration: mystrom new-feature new-platform labels Jul 21, 2023

home-assistant bot assigned fabaff Jul 21, 2023

home-assistant bot added the Quality Scale: No score label Jul 21, 2023

home-assistant bot added cla-recheck cla-signed and removed cla-recheck cla-needed labels Jul 21, 2023

joostlek requested changes Jul 21, 2023

View reviewed changes

home-assistant bot marked this pull request as draft July 21, 2023 18:35

MartinHjelmare changed the title ~~support sensors for myStrom plugs~~ Add sensors for myStrom plugs Jul 21, 2023

some improvements from pr reviews

16c039f

Merge branch 'dev' into myStrom-improvements

8150f58

joostlek mentioned this pull request Jul 23, 2023

Extract mystrom switch energy attributes into sensors #53319

Closed

OneCyrus mentioned this pull request Jul 23, 2023

Add power sensor for MyStrom #69745

Closed

22 tasks

adapt to the SensorEntityDescription pattern

0147a5a

MadMonkey87 added a commit to MadMonkey87/home-assistant.io that referenced this pull request Aug 31, 2023

Update mystrom.markdown

b50848f

Updated documentation for home-assistant/core#97024

edenhaus previously requested changes Sep 1, 2023

View reviewed changes

home-assistant bot marked this pull request as draft September 1, 2023 07:56

jdebetaz mentioned this pull request Oct 4, 2023

Adding documentation for new sensors on myStrom switches home-assistant/home-assistant.io#29144

Merged

8 tasks

joostlek added 3 commits October 18, 2023 15:49

Update __init__.py

32c10b9

Update const.py

c289e4a

Update sensor.py

d62b8fa

joostlek reviewed Oct 18, 2023

View reviewed changes

homeassistant/components/mystrom/sensor.py Outdated Show resolved Hide resolved

joostlek reviewed Oct 18, 2023

View reviewed changes

homeassistant/components/mystrom/sensor.py Outdated Show resolved Hide resolved

joostlek added 2 commits October 18, 2023 15:51

Apply suggestions from code review

69f618c

Merge branch 'dev' into myStrom-improvements

aaf9741

joostlek marked this pull request as ready for review October 18, 2023 13:53

home-assistant bot requested review from edenhaus and joostlek October 18, 2023 13:53

joostlek approved these changes Oct 18, 2023

View reviewed changes

joostlek merged commit 5f35eec into home-assistant:dev Oct 18, 2023
23 checks passed

MartinHjelmare reviewed Oct 18, 2023

View reviewed changes

github-actions bot locked and limited conversation to collaborators Oct 19, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add sensors for myStrom plugs #97024

Add sensors for myStrom plugs #97024

MadMonkey87 commented Jul 21, 2023 •

edited by edenhaus

home-assistant bot commented Jul 21, 2023

home-assistant bot commented Jul 21, 2023

home-assistant bot commented Jul 21, 2023

joostlek left a comment

home-assistant bot commented Jul 21, 2023

joostlek commented Jul 21, 2023 •

edited

joostlek commented Jul 21, 2023

MadMonkey87 commented Jul 21, 2023 •

edited

joostlek commented Jul 21, 2023

MadMonkey87 commented Jul 25, 2023

joostlek commented Aug 31, 2023 •

edited

edenhaus left a comment

edenhaus Sep 1, 2023

Siggy101 commented Sep 11, 2023

edenhaus commented Sep 11, 2023

Siggy101 commented Sep 11, 2023

Siggy101 commented Sep 19, 2023

jdebetaz commented Oct 4, 2023

gjohansson-ST commented Oct 4, 2023

jdebetaz commented Oct 4, 2023

frenck commented Oct 8, 2023

jdebetaz commented Oct 8, 2023

MartinHjelmare left a comment

MartinHjelmare Oct 18, 2023

MartinHjelmare Oct 18, 2023

Add sensors for myStrom plugs #97024

Add sensors for myStrom plugs #97024

Conversation

MadMonkey87 commented Jul 21, 2023 • edited by edenhaus

Proposed change

Type of change

Additional information

Checklist

home-assistant bot commented Jul 21, 2023

home-assistant bot commented Jul 21, 2023

home-assistant bot commented Jul 21, 2023

joostlek left a comment

Choose a reason for hiding this comment

home-assistant bot commented Jul 21, 2023

joostlek commented Jul 21, 2023 • edited

joostlek commented Jul 21, 2023

MadMonkey87 commented Jul 21, 2023 • edited

joostlek commented Jul 21, 2023

MadMonkey87 commented Jul 25, 2023

joostlek commented Aug 31, 2023 • edited

edenhaus left a comment

Choose a reason for hiding this comment

edenhaus Sep 1, 2023

Choose a reason for hiding this comment

Siggy101 commented Sep 11, 2023

edenhaus commented Sep 11, 2023

Siggy101 commented Sep 11, 2023

Siggy101 commented Sep 19, 2023

jdebetaz commented Oct 4, 2023

gjohansson-ST commented Oct 4, 2023

jdebetaz commented Oct 4, 2023

frenck commented Oct 8, 2023

jdebetaz commented Oct 8, 2023

MartinHjelmare left a comment

Choose a reason for hiding this comment

MartinHjelmare Oct 18, 2023

Choose a reason for hiding this comment

MartinHjelmare Oct 18, 2023

Choose a reason for hiding this comment

MadMonkey87 commented Jul 21, 2023 •

edited by edenhaus

joostlek commented Jul 21, 2023 •

edited

MadMonkey87 commented Jul 21, 2023 •

edited

joostlek commented Aug 31, 2023 •

edited