-
Notifications
You must be signed in to change notification settings - Fork 15
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
Fix consumption update for live sensor of second zaehlpunkt #233
Conversation
d61df9b
to
fe27e83
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #233 +/- ##
==========================================
+ Coverage 81.25% 85.06% +3.81%
==========================================
Files 5 5
Lines 288 288
==========================================
+ Hits 234 245 +11
+ Misses 54 43 -11 ☔ View full report in Codecov by Sentry. |
Both live sensors receive the meter readings the same way now by calling the historical data API with the value type METER_READ which returns the meter reading for each day. The latest value is then used to update the live sensor. This can be done with one or more zaehlpunkte and all return the same value type now, which is the total usage/feeding since implementation of the zaehlpunkt. This fixes the problems with the live sensor on a second zaehlpunkt as well as makes the output uniform. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you rebase onto master?
This project follows the pattern of a linear history. 🙃
) | ||
return | ||
self._available = True | ||
self._updatets = datetime.now().strftime("%d.%m.%Y %H:%M:%S") | ||
except TimeoutError as e: | ||
self._available = False | ||
_LOGGER.warning("Error retrieving data from smart meter api - Timeout: %s" % e) | ||
_LOGGER.warning( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are these line breaks coming from your setup, or is this a too strict reformatting config?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which line breaks do you mean? I tried to kept the formatting mostly as it was before, let me know if I missed something. So in this case the multiline warning was kept in the same style as before.
) | ||
if yesterdays_sum > 0: | ||
self._state = yesterdays_sum | ||
else: | ||
_LOGGER.error("Unable to load consumption") | ||
_LOGGER.error( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are these line breaks coming from your setup, or is this a too strict reformatting config?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above
tests/it/test_api.py
Outdated
@@ -301,7 +343,7 @@ def test_verbrauch_raw(requests_mock: Mocker): | |||
customer_id = "123456789" | |||
valid_verbrauch_raw_response = verbrauch_raw_response() | |||
expect_login(requests_mock) | |||
expect_history(requests_mock, enabled(zaehlpunkt())['zaehlpunktnummer']) | |||
expect_history(requests_mock, customer_id, enabled(zaehlpunkt())['zaehlpunktnummer']) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are these line breaks coming from your setup, or is this a too strict reformatting config?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which line breaks do you mean here? I have some auto formatting in my setup, but it's fairly limited.
@tschoerk I fear you've created another merge commit instead of rebasing the branch onto master. 😬 |
Sorry not too experienced with git. What to do, to fix this? |
!Be careful! Do point 6 after you've successfully confirmed, that this branch is working as you'd expect it, otherwise throw away the branch, check it out anew and rebase again. This should resolve all conflicts and remove all merge commits that you've introduced |
Just tested this PR and it seems to work! I was bothered by this issue forever now... 😅 |
5f6e847
to
abb153e
Compare
Sorry, I probably did something wrong here. I rebased following your instructions and now all my commits in my branch are gone and it's just like main, because I am stupid and did not check properly. |
@tschoerk no worries. when I did my first rebase I also messed it up. :) So that's part of the journey Maybe something that helps Just file a new PR with the changes applied. :) |
The Rebase tools in VSCode are usually very good :-) (by now it will automatically allow you to continue and understand you are in a rebase)
I usually tell all devs on my team to use autorebase+autostash on pull, once you get comfortable with it it is a great workflow ;-)
…On 7. Oct 2024 at 11:06 +0200, Christoph Spörk ***@***.***>, wrote:
@tschoerk no worries. when I did my first rebase I also messed it up. :) So that's part of the journey
Maybe something that helps
https://saraford.net/2017/04/21/how-to-visualize-a-rebase-in-the-git-visualization-tool-111/
Just file a new PR with the changes applied. :)
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you commented.Message ID: ***@***.***>
|
When updating the consumption data for the second zaehlerpunkt, the base meter readings don't have the information for this zaehlerpunkt, since it's not the "default" zaehlerpunkt. So the live sensor tries to get the information by checking the consumption data for yesterday to update the sensor with this value. This second method has several bugs though, which this pull request aims to fix.
This should resolve #227