[SmartDevice] - Enhancement - Extends the API

[kdschlosser]

as requested by @rytilahti this is a separate PR for just the API changes to the SmartDevice class

additional information can be found in pr #58

[rytilahti]

Looks good, thanks! Could you add test cases for those functionalities that are easily testable, so to those which just act on the device (getters & setters). I hope someone can then test it also out not against fake data but on a real device.

[kdschlosser]

I am working on the having it tested in a real environment. and I am still reading through the code for the tests to make sure i add them properly.

[rytilahti]

@kdschlosser any news on tests? :)

[kdschlosser]

I have been working on it. There are a lot of changes to the testing system. I have gotten the data for all of the different devices. I separated the testing into the 3 device types so you have the constants for sysinfo and the schema in those files. as well as any device type specific testing functions. there was an error in the original testing system where there were things being tested for that a specific device does not support. as an example the LB100 does not support time. I am still working on it it's simply a whole lot of information to have to go through and to make sure the proper things are being tested for each device. It's going to take me a while longer to finish it up and bug test it.

and as noted in #66 the HS105 does not support locations. things like this have to be accounted for,

I have another things that has forced me to stop working on this for a bit. I have to move our website and forum to a new host. and we decided to go with a VPS so there is some work involved there as well. so the brakes got put on with working on this for a bit until i can get that all sorted out and finished up.

[rytilahti]

Thanks for the update @kdschlosser, sounds good. I was just wondering whether we should wait for that part to finish before making a new release (e.g. to fix the issue in #66) and to add support for bulbs for the cli tool. As these changes will break the API I was thinking that it makes no sense to make a release before we get your work included too.

Anyway, if there's anything we can help you with please let us know! Splitting up tests to be against specific classes sounds good though.

[kdschlosser]

well I do have a question. You may be able to test this crazy theory that I have. I have been staring at this code and I noticed a pattern to some things.. and I am wondering if this pattern is global.

as an example

self._query_helper("system", "set_relay_state", {"state": 1})

would this work?

self._query_helper("system", "get_relay_state", {})

the reason i ask this is because of things like the location is get_location and set_location and I am almost wondering this this holds true for any and all API functions. because if it does hold true. it would be extremely easy to create a dynamic class at that point. simply grab the sysinfo and have it dynamically add functions as properties for the ones that are there. and all the ones that are not supported by that device would then be added producing a Not Implemented exception. You would have to go through all of the sysinfo items and try sending the set_ and get_ for each one to see.

[rytilahti]

It's really hard to say, I think I tried some command using a pattern at some point but it didn't work out. I can't really recall whether it'd have been caused by some other part but I think it makes more sense to figure out the API itself and hardcode it, there are not that many different items, right?

Another way would indeed be testing whether they do really work in that manner but I'm unsure if its worth the effort if there are really exceptions for that format.

[kdschlosser]

one of the reasons as to why I personally like the dynamic building of the class is it will make the code footprint a whole lot smaller. and the maintenance specifically having to add new devices will almost turn to a 0 it will be able to populate any device they come along with if it uses the same API. I am still a newbie to the whole TP-Link Smart Devices and I do not know how often they create devices. But the code could become quite congested if you have to have a class for each device. which is pretty much what is needed at this point because otherwise there is going to be a whole lot of having to check the sysinfo to see if the attribute is there and if it is then do a return and if not then to raise an error. That is going to be a huge amount of repeat code. or like the mic_mac or the mic_type there is going to have to be checking for that as well. and if they continue to change the attribute names I can see the code getting really ugly pretty quickly.

and to code something up that would simply grab the sysinfo take each of the attributes and simply add a get_ to the front of it and send it back out to see if it responds and print the information that is returned I do not think would be all that difficult. something that could be coded up in maybe 15 minutes.

I am trying to rub my crystal ball and see the future. and what i am seeing is more of the different devices support different things like location and time. and more of changing the attribute names. I personally hate it when companies do this kind of thing I don't see a need for them to change the names of the attributes. but who knows what their reasoning is.

[rytilahti]

I have modified the test system as well as sysinfo handling a bit, maybe this makes writing tests easier? For example non-existing sysinfo keys will return None, which allows avoiding exception handling and should simplify test cases.

[rytilahti]

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!