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

Add second sensor for historical data #58

Merged
merged 27 commits into from
Sep 6, 2023
Merged

Conversation

kdeyev
Copy link
Owner

@kdeyev kdeyev commented Aug 31, 2023

This PR addresses #30 and #43
The integration will add 2 different water meters:

  • regular with attr_state_class = SensorStateClass.TOTAL_INCREASING
  • "statistic" attr_state_class = None

The "statistic" one should be used in the Energy Dashboard as a workaround for the negative water usage values issue discussed in #30.

@kdeyev kdeyev requested a review from disforw August 31, 2023 15:22
@kdeyev kdeyev marked this pull request as ready for review August 31, 2023 22:02
@disforw
Copy link
Collaborator

disforw commented Sep 1, 2023

This looks great, do you think I should reconstruct it a little differently and separate out the sensors in to different classes since this is only “temporary”? It would be much easier later on to remove all of the secondary sensor stuff that way. Otherwise, this will work the way it is.

@kdeyev
Copy link
Owner Author

kdeyev commented Sep 1, 2023

Yes, thank you!
Do you want me to merge it first?

@kdeyev
Copy link
Owner Author

kdeyev commented Sep 1, 2023

BTW, I never had those crazy problems with statistics, so the regular sensor works for me great

@disforw
Copy link
Collaborator

disforw commented Sep 1, 2023

No, i’ll work inside of your branch. I have not seen any abnormal values either! I’ll work on this on Sunday, tonight is a drinking evening 😁.

@disforw disforw marked this pull request as draft September 2, 2023 22:38
@disforw
Copy link
Collaborator

disforw commented Sep 5, 2023

Theres an import_historical_data in both the super(coordinater) and the sensor? Are they the same thing?

@disforw
Copy link
Collaborator

disforw commented Sep 6, 2023

Extra sensor created 👍🏼

@kdeyev
Copy link
Owner Author

kdeyev commented Sep 6, 2023

@disforw Sorry, I missed your message.
Yes, I made them almost identical and the only difference is the attr_state_class.
I found that the statistic with attr_state_class = None has some limitations, for example, it does not calculate the consumed water cost.
So I decided to create 2 almost identical sensors, one for the happy path and another as a fallback.

@kdeyev
Copy link
Owner Author

kdeyev commented Sep 6, 2023

I see pre-commit checks are failing. let me fix it

@kdeyev
Copy link
Owner Author

kdeyev commented Sep 6, 2023

Should we merge it?

@kdeyev kdeyev closed this Sep 6, 2023
@kdeyev kdeyev reopened this Sep 6, 2023
@disforw disforw marked this pull request as ready for review September 6, 2023 04:33
@disforw
Copy link
Collaborator

disforw commented Sep 6, 2023

I cleaned us up a little bit, looks good to me. Wanna just check it with a beta first?

@kdeyev kdeyev enabled auto-merge (squash) September 6, 2023 14:00
@kdeyev kdeyev merged commit 725030f into master Sep 6, 2023
4 checks passed
@kdeyev kdeyev deleted the feature/add-historical-sensor branch September 6, 2023 14:01
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.

2 participants