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

contributing KasaCam support #530

Open
bstrdsmkr opened this issue Oct 24, 2023 · 4 comments · May be fixed by #537
Open

contributing KasaCam support #530

bstrdsmkr opened this issue Oct 24, 2023 · 4 comments · May be fixed by #537

Comments

@bstrdsmkr
Copy link

Hey folks,

I've been playing with my EC70 and want to integrate it with HomeAssistant but don't have time to maintain my own library so I'd like to contribute support for it (and other KasaCam devices) to yours. The trouble is I don't think it fits the same paradigm as currently supported devices.

For example, there's no central call (besides system.get_info) to get the device state so there's a choice between sending 10+ different commands to gather state or breaking the convention of caching to the .data attribute. Worse, it seems this device doesn't accept multiple commands in a single call so those would be discrete calls.

Next, they don't support discovery so the Smart device child class will always need to be created manually (not a deal breaker, but still "different").

Finally, all the available API calls (I've found 30+ looking through the android app) are spread across multiple namespaces making it a hard fit for the Modules concept (either 1 massive module or a bunch of modules with only one or two functions).

I've written a working Transport for these devices and stubbed out all the function calls, but I'm running low on time to spend on the project.

I guess my questions are:

  1. How would you prefer to tackle the challenges above?
  2. How far does an implementation need to be before you'd accept it as "a good start to iterate on later?" I hope that I'd have more time to iterate on it in the future and contribute more, but I can't promise that yet
@rytilahti
Copy link
Member

Hi @bstrdsmkr,

would you mind explaining how much similarities these devices have in common to the already existing devices? You talk about the transport protocol, is it the same as what is currently implemented (or maybe the same as described in #509)? If not, how many lines of code are we talking about here?

Reading between the lines, my understanding is that these devices are just using a different command set (i.e., modules and methods) that do not match with the existing SmartDevice API? If so, that's not an issue and we can adapt the library as needed. I think it would be really helpful if you would create a draft PR or at least show some more details, just to get a feeling of what would integrating these devices involve in terms of changes to this library.

I am now assuming that the command set is still delivered using the same (or similar?) protocol, so I think the big question is how is the video stream accessed considering that is probably the main feature someone using the library would be looking for? Or would implementing just the control protocol be the goal, leaving the streams out of scope?

Now, to your questions, it is hard to answer those questions off-hand, but I'll try:

  1. I would personally like to see how much overlap there exists between the already existing devices (i.e., how much code would be needed to support those cams, and
  2. The tests (preferably using real device payloads) for the new functionality would be nice to have to help avoid breaking anything, when and if refactorings happen, but if the communication with these devices is not that much different we can easily start with some basic controls and go forward from there.

@bstrdsmkr
Copy link
Author

Thanks @rytilahti, I'll try to get a WIP up soon. The good news is I was wrong about one thing -- they do support multiple commands per request, so that's much better.

They're similar to the Tapo protocol but the method of encryption is different, thus another Protocol implementation.

Id like to focus on control of the PTZ and other features as the streaming is pretty standard so that's at least covered by other libraries.

The main question at the moment is around the tests. All the responses come back from these devices nested one level deeper than the standard devices so the basic SmartDevice tests fail when checking the time zone for example. That means either lifting out the time, relay, etc checks into yet another super class or a bunch of if-else statements in the "standard" modules.

@rytilahti
Copy link
Member

Hi again @bstrdsmkr, if you were testing against the 0.5.3, it the issues with multiple commands may have been related to something that was fixed in the just released 0.5.4 (#502).

About the protocol, is it the klap protocol I mentioned? In any case, it would be helpful for reference if you could try kasa discover --show-unsupported on the new release and post the content of the discovery response.

If the commands are alike and it is just about nesting in repsonses, I think we can figure out a solution for that. For the moment, I would ignore the tests until we get a better picture on what changes and in which parts of the class hierarchy would be required to support these devices :-)

@bstrdsmkr
Copy link
Author

Just a note for anyone that comes across this, #537 is the WIP PR I'm working through

@rytilahti rytilahti linked a pull request Nov 18, 2023 that will close this issue
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 a pull request may close this issue.

2 participants