Skip to content
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

Extract mystrom switch energy attributes into sensors #53319

Closed
balloob opened this issue Jul 22, 2021 · 22 comments · Fixed by #97024
Closed

Extract mystrom switch energy attributes into sensors #53319

balloob opened this issue Jul 22, 2021 · 22 comments · Fixed by #97024
Labels
energy Related to energy management integration: mystrom to do

Comments

@balloob
Copy link
Member

balloob commented Jul 22, 2021

This issue is for the switch platform of the mystrom integration.

The switch integration has embedded sensors for energy usage. This is no longer the preferred approach. Integrations using this approach should turn this data into standalone sensors.

This is important because Home Assistant is going to introduce home energy management (HEM) in 2021.8. HEM will be able to consume the data if it is exposed as standalone sensors.

To make sure that the sensors that represent total kWh is usable for HEM, set state_class to STATE_CLASS_TOTAL_INCREASING. See sensor documentation for more information.

Link to the architecture discussion.

@probot-home-assistant
Copy link

Hey there @fabaff, mind taking a look at this issue as its been labeled with an integration (mystrom) you are listed as a codeowner for? Thanks!
(message by CodeOwnersMention)

@balloob balloob added the to do label Jul 22, 2021
@agners
Copy link
Member

agners commented Aug 6, 2021

Besides not using a standalone sensor, the current code only extracts power (W) from what I can tell.

According to the myStrom API documentation the report endpoint returns Ws:

The average of energy consumed per second from last call this request.

So it seems averaged, which is definitely not ideal. But I guess it could be used to calculate energy used as part of the library/integration.

I don't have one of these but was thinking to get one. However, is energy extraction turns out to be not possible/inaccurate, I'll probably won't go for it.

@balloob
Copy link
Member Author

balloob commented Aug 6, 2021

Ws can be converted to Wh

@agners
Copy link
Member

agners commented Aug 6, 2021

It is actually weird that they say "average of energy consumed per second". Energy already has a unit with a time component (like watts per second), if they write energy per second it sound like time unit squared 😅

I am probably overthinking it. Most likely they just return energy in watt seconds and we can summarize it.

Would statistics etc. work if the integration would set last_reset to when the last request has been made, even if that is just say 30s ago?

@balloob
Copy link
Member Author

balloob commented Aug 8, 2021

No. The state that changes reset is set as new "base line", no growth is calculated.

@frenck frenck added the energy Related to energy management label Aug 9, 2021
@github-actions
Copy link

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates.
Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment 👍
This issue has now been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale label Nov 18, 2021
@balloob balloob removed the stale label Nov 18, 2021
@github-actions
Copy link

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates.
Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment 👍
This issue has now been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.

@Schnabulation
Copy link

Now with the update to 2022.4 the attribute 'current_power_w' is being removed from the switch core entity platform. Can this issue be fixed in a timely manner? I really need that attribute to notify when my coffee machine has switched off...

@frenck
Copy link
Member

frenck commented Apr 7, 2022

Can this issue be fixed in a timely manner?

It's not an issue, it's a deprecation of an old feature, that is no longer available.
Other options to implement this are available, unfortunately, no one has contributed it until now.

Feel free to jump in to make that happen 👍

../Frenck

@Schnabulation
Copy link

Unfortunately my codings skills are really basic, otherwise I think I would.

But as a workaround: because the current drawn power of the switch can be read via REST ([IP of switch]/report) I think I'm just going to configure my sensor to pull the data from there.

@juame
Copy link

juame commented Apr 7, 2022

@Schnabulation same issue here... I use a RESTful Sensor as workaround (not a solution), example:

sensor:
  - platform: rest
    name: mystrom_wifi_switch_1_current_power
    state_class: measurement
    device_class: power
    unit_of_measurement: "W"
    resource: "http://192.168.1.61/report"
    value_template: "{{ value_json.power }}"

@OneCyrus
Copy link

OneCyrus commented Apr 7, 2022

Can this issue be fixed in a timely manner?

It's not an issue, it's a deprecation of an old feature, that is no longer available.

Other options to implement this are available, unfortunately, no one has contributed it until now.

Feel free to jump in to make that happen 👍

../Frenck

will a PR for just the additional sensors be accepted or is a change to user flow required?

@OneCyrus
Copy link

OneCyrus commented Apr 9, 2022

just opened a PR to add the power consumption as a sensor. not sure if this is the right approach as I'm new to HA internals and python. Feedback would be great. I will try to add energy sensors if this is the correct way.

#69745

@github-actions
Copy link

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates.
Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment 👍
This issue has now been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added stale and removed stale labels Jul 15, 2022
@issue-triage-workflows
Copy link

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates.
Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment 👍
This issue has now been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.

@dotdoom
Copy link

dotdoom commented Jan 15, 2023

Still need it, I think the PR is currently blocked by the review of #74719. Please, kind robot, do not close this just yet!

@issue-triage-workflows
Copy link

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates.
Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment 👍
This issue has now been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.

@Dodoooh
Copy link

Dodoooh commented Apr 22, 2023

it's still a issue, PR: #74719

@issue-triage-workflows
Copy link

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates.
Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment 👍
This issue has now been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.

@chrisburr
Copy link

This is waiting on #69745

@joostlek
Copy link
Member

Oh, looks like we have 2 PRs for the same thing I guess? #69745 and #97024. I think I like the #97024 approach better as it correctly splits up the sensors into its own file. Let's discuss here how we're going to pick this up, otherwise it would be double work. cc @OneCyrus @MadMonkey87

@OneCyrus
Copy link

@joostlek thanks for pointing this out. let's move on with #97024. I wasn't sure how to combine the multiple entities.

@github-actions github-actions bot locked and limited conversation to collaborators Nov 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
energy Related to energy management integration: mystrom to do
Projects
None yet