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 timezone setter #55

Closed
wants to merge 1 commit into from
Closed

Conversation

jeffsf
Copy link

@jeffsf jeffsf commented Apr 25, 2017

While an HS110 may ignore the time in the setter,
it will allow the timezone to be changed.

#53

While an HS110 may ignore the time in the setter,
it will allow the timezone to be changed.

GadgetReactor#53
Copy link
Collaborator

@kirichkov kirichkov left a comment

Choose a reason for hiding this comment

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

Can you please add a file TIMEZONES / TIMEZONES.md that lists all the time zones with their respective index, so that it's easier for newcomers to figure it out instead of digging through the github issues to find the index for their time zone.

A reference from README.md will also be good I think.

@jeffsf
Copy link
Author

jeffsf commented Apr 25, 2017

Sounds good -- I'll get the raw output into something a little easier to read.

I had, at first, thought that it could easily be generated with a simple for loop, but the HS105 I literally just opened up isn't as descriptive

tp105.timezone
{'index': 6}

@rytilahti
Copy link
Collaborator

I'd much prefer to have this as a part of setter for time, where it imo API-wise belongs.

@jeffsf
Copy link
Author

jeffsf commented Apr 25, 2017

@rytilahti -- Since it doesn't seem to set the time, but just the time zone, at least with an HS110, I chose to have it as @timezone.setter rather than @time.setter

I'm wrestling with an HS105 and will take a look to see if it sets the time, hopefully this evening (Pacific Time). If so, then it makes sense to me to have both a function to set time, as well as one that only sets the time zone.

@rytilahti
Copy link
Collaborator

@jeffsf yeah, I made that NotImplemented there exactly as it wasn't updating the time. I didn't however test if it'd allow updates assuming the NTP servers are not accessible (maybe there's a logic when it allows those changes).

Anyway, I see your point and I find it reasonable, maybe the timezone setter & getter should accept and return a (pytz?) timezone object?

@jeffsf
Copy link
Author

jeffsf commented Apr 26, 2017

I've pulled the access to my DNS-spoofed NTP server from my HS110 (It's on a non-routed VLAN), power cycled it, and will let it thrash through it's list of servers for a while and then re-check to see if the tie can be sent if it can't reach an NTP server.

I'll also look at my HS105, assuming that I can get it to take WiFi credentials through either pyHS100 or tplink-smartplug. While I can get the relay state, it wasn't responding to the "netif" commands for "get_scaninfo" or "set_stainfo"

I'll take a look at pytz and see if it makes sense somehow. From what I can tell, there are "only" 109 zones in the TP-Link table, but

>>> from pytz import common_timezones
>>> len(common_timezones)
439

I've got some roughed out code that uses the captured results of probing a device to be able to convert the index to a fuller description, as well as a human-readable string, as well as dumping the entire list into markdown. It's a lot more than a half-dozen lines of code, so I'll have to figure out your testing framework before I consider a pull request on that code, itself.

It does let me provide a TIMEZONES.md that I'll add to this once I better understand how the HS110 and HS105 react.

@jeffsf
Copy link
Author

jeffsf commented Apr 26, 2017

Very interesting, it might be that you can only set the time once per boot.

>>> tp110._query_helper('time', 'set_timezone', {"year":2016, "month":1, "mday":1, "hour":10, "min":10, "sec":10, "index":5})
{}
>>> print(tp110.timezone)
{'index': 5, 'zone_str': '(UTC-08:00) Pacific Standard Time (US & Canada)', 'tz_str': 'PST8', 'dst_offset': 0}
>>> print(tp110.time)
2016-01-01 10:10:28
tp110._query_helper('time', 'set_timezone', {"year":2000, "month":1, "mday":2, "hour":3, "min":4, "sec":5, "index":6})
{}
>>> print(tp110.time)
2016-01-01 10:11:19
tp110._query_helper('time', 'set_timezone', {"year":2017, "month":1, "mday":2, "hour":3, "min":4, "sec":5, "index":6})
{}
>>> print(tp110.time)
2016-01-01 10:12:12

Confirmed that, at least on the HS110, you can set the time once on boot, and not again, at not at least within a few minutes. Also that NTP will override manually set time and that manually setting the time does not disable NTP.

@kirichkov
Copy link
Collaborator

They might be using the time in some manner when communicating with the "cloud". They definitely need that to ensure certificate validity. That can explain why they are limiting the time setting.

@jeffsf
Copy link
Author

jeffsf commented Apr 26, 2017

Please put this on hold until I can determine how to make this work for the HS105 as well.

>>> tp105._query_helper('time', 'set_timezone', {"year":2001, "month":1, "mday":2, "hour":3, "min":4, "sec":5, "index":6})
{'err_msg': 'invalid argument'}
>>> tp110._query_helper('time', 'set_timezone', {"year":2001, "month":1, "mday":2, "hour":3, "min":4, "sec":5, "index":6})
{}

@rytilahti
Copy link
Collaborator

Interesting findings, thanks for keeping us up to date. Waiting for a more proper solution sounds fair enough.

@kirichkov
Copy link
Collaborator

@jeffsf do you have any news?

@rytilahti
Copy link
Collaborator

Adding a dictionary containing the values from #53 (comment) (keyed with tz_str?) and making it use that information for the API call would be an acceptable solution, if someone wants to pursue this.

@rytilahti
Copy link
Collaborator

We are closing this repository for new changes in favor of a new, asyncio-enabled continuation project: https://github.com/python-kasa/python-kasa

If you are still interested in contributing, please feel free to create a new PR to the new repository, thanks!

@rytilahti rytilahti closed this Mar 16, 2020
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.

None yet

3 participants