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

Initial Import of Historical Data #101

Merged
merged 60 commits into from Apr 23, 2023
Merged

Initial Import of Historical Data #101

merged 60 commits into from Apr 23, 2023

Conversation

reox
Copy link
Collaborator

@reox reox commented Apr 8, 2023

yet another thing to test ;)

Implemented the fetching of the historical data and import it on initial sensor setup.

This resolves #26 - at least partially, because it grabs the data directly instead of asking

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.
@DarwinsBuddy
Copy link
Owner

@DarwinsBuddy I do not understand why the tests are failing here. The response should be mocked including the expire times, and thus also on calling login() also the expire variables should be set - no?

My bad. I made a mistake setting up that test case

@codecov
Copy link

codecov bot commented Apr 8, 2023

Codecov Report

Merging #101 (3f2db14) into main (edc9a67) will decrease coverage by 3.72%.
The diff coverage is 15.67%.

@@            Coverage Diff             @@
##             main     #101      +/-   ##
==========================================
- Coverage   33.16%   29.44%   -3.72%     
==========================================
  Files           9       12       +3     
  Lines         392      618     +226     
  Branches       48       84      +36     
==========================================
+ Hits          130      182      +52     
- Misses        258      428     +170     
- Partials        4        8       +4     
Impacted Files Coverage Δ
custom_components/wnsm/base_sensor.py 0.00% <0.00%> (ø)
custom_components/wnsm/const.py 0.00% <0.00%> (ø)
custom_components/wnsm/live_sensor.py 0.00% <0.00%> (ø)
custom_components/wnsm/sensor.py 0.00% <0.00%> (ø)
custom_components/wnsm/statistics_sensor.py 0.00% <0.00%> (ø)
custom_components/wnsm/utils.py 0.00% <0.00%> (ø)
custom_components/wnsm/api/client.py 75.00% <82.45%> (+7.85%) ⬆️
custom_components/wnsm/api/constants.py 89.28% <100.00%> (+5.95%) ⬆️
custom_components/wnsm/api/errors.py 92.30% <100.00%> (+0.64%) ⬆️

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

reox and others added 2 commits April 8, 2023 22:15
… out

Replace f-formatting by lazy formatting in various debug msgs
@DarwinsBuddy
Copy link
Owner

DarwinsBuddy commented Apr 8, 2023

I launched it, got a warning stating that the updating of the sensor took > 10 seconds. Maybe 3 years is a bit far in the past for one shot.

Afterwards I was not able to see any historical data in the sensor. Where should I be able to see the pulled data? Never mind in the energy dashboard it now pops up.

That is amazing! How did you find out this is a feature of the b2b api?

Copy link
Owner

@DarwinsBuddy DarwinsBuddy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please run

flake8 custom_components/wnsm --count --select=E9,F63,F7,F82 --show-source --statistics --config tests/setup.cfg
and
flake8 custom_components/wnsm --count --exit-zero --max-complexity=10 --statistics --config tests/setup.cfg

and resolve the issues. (sometimes it pays of to disable them inline #noqa: XXX

custom_components/wnsm/api/client.py Outdated Show resolved Hide resolved
custom_components/wnsm/api/client.py Outdated Show resolved Hide resolved
custom_components/wnsm/api/client.py Outdated Show resolved Hide resolved
custom_components/wnsm/api/client.py Show resolved Hide resolved
custom_components/wnsm/api/client.py Show resolved Hide resolved
custom_components/wnsm/api/client.py Show resolved Hide resolved
# Keep that in mind if for someone this fails.
logger.debug(f"Returned data: {data}")
raise SmartmeterQueryError("Returned data does not match given zaehlpunkt!")
obis_code = data[0]['zaehlwerke'][0]['obisCode']
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a test for this

custom_components/wnsm/api/client.py Show resolved Hide resolved
custom_components/wnsm/api/client.py Outdated Show resolved Hide resolved
custom_components/wnsm/api/client.py Outdated Show resolved Hide resolved
@reox
Copy link
Collaborator Author

reox commented Apr 9, 2023

That is amazing! How did you find out this is a feature of the b2b api?

just watched the webpage with the developer tools 😀

Maybe 3 years is a bit far in the past for one shot.

When you visit the dataexport in the smartmeter-web, 3 years is the standard setting. Thus I thought it might be OK to do the same. Does it take very long for you? I only have smartmeter entries since November 2023, and thus with any date range it is actually quite fast.

I just thought about an issue with this approach: If for some reason it fails to gather the data, the init procedure will be called again and again. So we need some way of determining that the init was called once and do not start it again.

reox and others added 4 commits April 9, 2023 10:21
@DarwinsBuddy
Copy link
Owner

That is amazing! How did you find out this is a feature of the b2b api?

just watched the webpage with the developer tools grinning

So same technique as I did it. :) but I've not discovered this part yet. Good catch! ❤️

Maybe 3 years is a bit far in the past for one shot.

When you visit the dataexport in the smartmeter-web, 3 years is the standard setting. Thus I thought it might be OK to do the same. Does it take very long for you? I only have smartmeter entries since November 2023, and thus with any date range it is actually quite fast.

I just thought about an issue with this approach: If for some reason it fails to gather the data, the init procedure will be called again and again. So we need some way of determining that the init was called once and do not start it again.

hmm... I guess then 3years should be fine. But you have a point there. What we could do though is only try it once at set-up. If it fails we shouldn't trigger it anew. We can implement an option in the config_flow to trigger it manually. I have to look into the details though. so better add an at most once logic, create another issue and let's solve this after this monster of a branch is in main 😃

@DarwinsBuddy
Copy link
Owner

@reox unless you have any concerns. I'd release this feature. It's downwards compatible and runs smoothly already.

Regarding

When you visit the dataexport in the smartmeter-web, 3 years is the standard setting. Thus I thought it might be OK to do the same. Does it take very long for you? I only have smartmeter entries since November 2023, and thus with any date range it is actually quite fast.
I just thought about an issue with this approach: If for some reason it fails to gather the data, the init procedure will be called again and again. So we need some way of determining that the init was called once and do not start it again.

hmm... I guess then 3years should be fine. But you have a point there. What we could do though is only try it once at set-up. If it fails we shouldn't trigger it anew. We can implement an option in the config_flow to trigger it manually. I have to look into the details though. so better add an at most once logic, create another issue and let's solve this after this monster of a branch is in main smiley

I'd create another issue, when I've got your Okay. :)

@reox
Copy link
Collaborator Author

reox commented Apr 16, 2023

@DarwinsBuddy it runs also smoothly on my side! Except some missing test cases, I think it is good to go :)

Copy link
Owner

@DarwinsBuddy DarwinsBuddy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@DarwinsBuddy DarwinsBuddy merged commit 71cdf72 into main Apr 23, 2023
7 of 10 checks passed
@DarwinsBuddy DarwinsBuddy deleted the f/initial_import branch April 23, 2023 10:24
@DarwinsBuddy DarwinsBuddy added the enhancement New feature or request label Apr 23, 2023
This was referenced Apr 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add csv import capability and hook it up to init sensor (or config flow)
2 participants