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

adds timezone processing support to API. #23

Closed
wants to merge 2 commits into from

Conversation

xannor
Copy link
Contributor

@xannor xannor commented Apr 19, 2023

This adds parsing of the GetTime results.

get_timezone() provides a timezone from the currently loaded GetTime data, or None if missing.

get_time() provides an approximation of the current device time by using the local system time plus the host offset and then forcing it to be the device timezone

async_get_time() forces a GetTime status call first to ensure time settings are loaded and supported, then returns the results as the device time.

@xannor xannor marked this pull request as ready for review April 19, 2023 19:13
Copy link
Owner

@starkillerOG starkillerOG left a comment

Choose a reason for hiding this comment

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

Thank you for this PR, it looks good, just some small comments.
However I do think it is a lot of code in the typings.py file, I wonder if there would not be a simpler/much more compact approach. But I guess this is fine.

reolink_aio/api.py Show resolved Hide resolved
reolink_aio/typings.py Show resolved Hide resolved
reolink_aio/typings.py Outdated Show resolved Hide resolved
reolink_aio/typings.py Outdated Show resolved Hide resolved
@starkillerOG
Copy link
Owner

@xannor I will be on vaccation for a week starting tomorrow.
So unfortunately I will not be able to respond in the comming week.
Hopefully we can merge this today, otherwise it will have to wait 1 week, sorry.

@xannor
Copy link
Contributor Author

xannor commented Apr 26, 2023

I am working on your suggestions, and I had a question. What are your thoughts on slots, I noticed you do not use them, and I have refrained from it to try to mach your current code style. However, for things like this, where the class could be hit many times, and should never have subclasses, I feel they would be warranted. What are your thoughts?

Copy link
Contributor Author

@xannor xannor left a comment

Choose a reason for hiding this comment

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

new commit should include changes request.

@xannor
Copy link
Contributor Author

xannor commented May 3, 2023

I am withdrawing this as I want to cleanup/redo this branch with some changed I need in my other branch that deals with the VOD_file and search parts. I should have made the smaller changes first, but hindsight. I will submit a new pull once I have it ready, I will submit the tz pull first then the vod pull when it is ready (it will somewhat depend on the tz changes (but only minor-ly)

@xannor xannor closed this May 3, 2023
@xannor xannor deleted the dev_tz branch May 3, 2023 14:35
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