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

New method fetch_metric_data() -> MetricData #61

Merged
merged 6 commits into from
Feb 11, 2024
Merged

New method fetch_metric_data() -> MetricData #61

merged 6 commits into from
Feb 11, 2024

Conversation

yozik04
Copy link
Owner

@yozik04 yozik04 commented Feb 10, 2024

The plan is to replace all fetch_metric calls with fetch_metric_data that will return new MetricData class instance.
MetricData class works the same way as dictionary returned by fetch_metric, but it has useful additional properties and functions. See MetricData class source code for details.

@yozik04 yozik04 changed the title MetricData New method fetch_metric_data() -> MetricData Feb 11, 2024
@yozik04
Copy link
Owner Author

yozik04 commented Feb 11, 2024

@slovdahl, take a look. I think, you want to use this new method. That will reduce amount of data reads required to populate entities with values.

metric_data.profile
metric_data.next_filter_change_date
metric_data.get_temperature_setting(metric_data.profile)
...

@yozik04 yozik04 merged commit ecab24f into master Feb 11, 2024
7 checks passed
@slovdahl
Copy link
Contributor

Cool!

I never took a good look at how the library works under the hood 😀 Does calling the old client.fetch_metrics() method fetch all the data that the Vallox unit provides? And by using the new API the library will only fetch the data that is actually requested?

@yozik04
Copy link
Owner Author

yozik04 commented Feb 13, 2024

Yes fetch_metrics() always reads all data. I remember I wanted to implement a separate read for a subset of metrics, but it was never needed. So today after it reads all I just filter out what was requested. fetch_metric() -> currently calls fetch_metrics() so the same read all is executed.
Same for any other method like get_profile, get_info, get_temperature, get_fan_speed, get_next_filter_change_date. I would like to deprecate these in future and use only MetricData class for getting values.

@slovdahl
Copy link
Contributor

I think I got it to work in HA. But I have so little experience with Python's unit testing and mocking that I cannot figure out how to get the tests to work 🙈 Pushed the current state here: https://github.com/slovdahl/core/commits/bump-vallox-websocket-api-to-4.2.0/

@yozik04
Copy link
Owner Author

yozik04 commented Feb 14, 2024

Will take a look tomorrow.

@yozik04
Copy link
Owner Author

yozik04 commented Feb 16, 2024

@slovdahl I took a look. It was tough. Here is the result: home-assistant/core#110752
Please review.

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