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

[WIP] Importing hourly consumption #35

Merged
merged 44 commits into from
Apr 23, 2023
Merged

[WIP] Importing hourly consumption #35

merged 44 commits into from
Apr 23, 2023

Conversation

DarwinsBuddy
Copy link
Owner

@reox I moved your branch and created a new PR, so we can follow up in the base repo. You should be able to commit.

Sebastian Bachmann and others added 30 commits December 31, 2022 14:29
This imports the hourly consumption into the statistics entity.
If the integration is set-up for the first time, the current meter
reading is fetched and used as a baseline.
@reox
Copy link
Collaborator

reox commented Mar 2, 2023

Thanks!
So indeed, you get a float now... oO

edit: I get the same float error now after restarting HA. And the first time you get this other error, which seems to be cause by the database upgrade.

@reox
Copy link
Collaborator

reox commented Mar 2, 2023

I just pushed a workaround and also asked in the HA community: https://community.home-assistant.io/t/get-last-statistics-returned-str-then-datetime-now-float/542311

@codecov
Copy link

codecov bot commented Mar 2, 2023

Codecov Report

Merging #35 (05365db) into main (f2a46a5) will decrease coverage by 2.21%.
The diff coverage is 8.73%.

@@            Coverage Diff             @@
##             main      #35      +/-   ##
==========================================
- Coverage   31.77%   29.56%   -2.21%     
==========================================
  Files           9        9              
  Lines         384      433      +49     
  Branches       48       55       +7     
==========================================
+ Hits          122      128       +6     
- Misses        258      301      +43     
  Partials        4        4              
Impacted Files Coverage Δ
custom_components/wnsm/const.py 0.00% <0.00%> (ø)
custom_components/wnsm/sensor.py 0.00% <0.00%> (ø)
custom_components/wnsm/utils.py 0.00% <0.00%> (ø)
custom_components/wnsm/api/client.py 68.21% <71.42%> (+3.06%) ⬆️
custom_components/wnsm/api/constants.py 86.36% <100.00%> (+3.03%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@jozefKruszynski
Copy link

I just pushed a workaround and also asked in the HA community: https://community.home-assistant.io/t/get-last-statistics-returned-str-then-datetime-now-float/542311

Seems to fix the logs at least.

@jozefKruszynski
Copy link

I just pushed a workaround and also asked in the HA community: https://community.home-assistant.io/t/get-last-statistics-returned-str-then-datetime-now-float/542311

Looks like there was indeed in the changeling a breaking change listed to statistics that relates to using timestamp columns

https://www.home-assistant.io/changelogs/core-2023.3

home-assistant/core#87321

@reox
Copy link
Collaborator

reox commented Mar 2, 2023

Looks like there was indeed in the changeling a breaking change listed to statistics that relates to using timestamp columns

Thanks for digging this up. It's funny that the "only" breaking change is the unavailability during migration...

@jozefKruszynski
Copy link

I have been testing this since early January, and it's working beautifully for me too, with one exception: while my energy dashboard shows my energy consumption correctly, my energy cost is always 0.

Yes - this is a major drawback of this branch. The issue is that I simply import the historical statistical data only, while the original implementation updates the energy counter (however with the wrong time of consumption). Due to lack of functions in HA, nothing else seems to be an option. At least no one in the HA discourse had another idea :/

One idea was to have two energy meters, one that has the correct statistics and one that has the correct meter reading. But I have no clue (yet) how that could be implemented.

There should be an associated statistics_meta entry for the cost sensor that gets created along with the consumption sensor right?

Would it be possible to write the statistics entries for the cost sensor at the same time as writing the statistics for the consumption sensor?

I noticed that with this branch, the cost sensor does not get created in the statistics_meta table, however, there is a sensor that is created with the same name as the consumption sensor suffixed with "_cost"

Would it then be feasible to take an input_number sensor for example and use this as the basis for the statistics entries for the cost sensor?

I think that in order for this to work, you would also first need to create the meta entry, but perhaps it might do the trick...

@reox
Copy link
Collaborator

reox commented Mar 2, 2023

I do not know the internals of HA that much, but I would guesstimate that writing statistics does not help, as statistics are not states.
One idea would be to have two sensors, one that has the hourly consumption stats and one that has the meter reading + costs.
Unfortunately, both cannot be mixed.
I don't even have any costs entity, but maybe that is because I haven't configured any costs?

@jozefKruszynski
Copy link

jozefKruszynski commented Mar 2, 2023

I do not know the internals of HA that much, but I would guesstimate that writing statistics does not help, as statistics are not states.
One idea would be to have two sensors, one that has the hourly consumption stats and one that has the meter reading + costs.
Unfortunately, both cannot be mixed.
I don't even have any costs entity, but maybe that is because I haven't configured any costs?

The costs entity is only visible in the devtools -> statistics tab, and for me it exists as soon as the sensor from the integration is created.
However, I see your point, and I guess that the cost sensor in this case is irrelevant. :(

@reox
Copy link
Collaborator

reox commented Mar 2, 2023

ah, you have to configure costs for the sensor first. I had checked "do not track costs".

@DarwinsBuddy
Copy link
Owner Author

@reox I've to admit that, I haven't installed the PR on my system. afaik it replaces the sensor by its historical counterpart.
Are there any drawbacks to it? Afaik that we cannot trigger any automations from it, right?
If that's the case I'd create a second sensor working as used to be. So the integration would create two sensors per zählpunkt. One statistics and one "live" one (which is updated presumably ever day).

Is this correct and also a suitable solution iyo?

@reox
Copy link
Collaborator

reox commented Mar 21, 2023

Is this correct and also a suitable solution iyo?

yes! I started to implement this, but got lost in setting up the devcontainer.. apparently, the one that is configured right now is deprecated and shall not be used.

So, my plan was to add another parameter to __init__ of the Sensor to switch between a statistics sensor and a counting sensor.
But I'm not sure if this is the right way to go...

@reox
Copy link
Collaborator

reox commented Mar 26, 2023

Now I feel stupid.. On closer thought on the summer/wintertime issue: it is no issue at all. UTC is monotonic and does not jump around. Thus, I'm certain that the things get imported correctly and just the UI has to display them properly.

@reox
Copy link
Collaborator

reox commented Mar 26, 2023

okay, so the only thing that might be neat is #83 and i created a new ticket for that.

We could now start to merge this into a dual-sensor setup, where we have both the consumption and the meter reading!

@DarwinsBuddy
Copy link
Owner Author

DarwinsBuddy commented Apr 1, 2023

Sorry for the late reply.
Unfortunately I was involved in an accident and am slowly recovering.

I'll try to split this up into two separate sensors then.

@DarwinsBuddy
Copy link
Owner Author

Is this correct and also a suitable solution iyo?

yes! I started to implement this, but got lost in setting up the devcontainer.. apparently, the one that is configured right now is deprecated and shall not be used.

So, my plan was to add another parameter to __init__ of the Sensor to switch between a statistics sensor and a counting sensor. But I'm not sure if this is the right way to go...

should we remove it again @TheRealVira ?

@DarwinsBuddy DarwinsBuddy deleted the f/statistics branch April 1, 2023 20:10
@DarwinsBuddy DarwinsBuddy restored the f/statistics branch April 1, 2023 20:10
@DarwinsBuddy DarwinsBuddy reopened this Apr 1, 2023
@salzrat
Copy link

salzrat commented Apr 7, 2023

I would really love the hourly consumption - does it make sense to try the statistics branch yet, or should I wait a bit more?

@reox
Copy link
Collaborator

reox commented Apr 7, 2023

I would really love the hourly consumption - does it make sense to try the statistics branch yet, or should I wait a bit more?

sure! If you are starting from scratch anyways, maybe test #89 directly :)

@salzrat
Copy link

salzrat commented Apr 7, 2023

No, I'm not starting from scratch, I have the main branch running for several months already. Above are some mentions of things that still need to be done. Will they impact the experience if I try it now or should I be good to go?

@DarwinsBuddy DarwinsBuddy merged commit 05365db into main Apr 23, 2023
@DarwinsBuddy DarwinsBuddy deleted the f/statistics branch April 30, 2023 08:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants