Implement floodlight control for Tapo C720 Floodlight Camera #1629
Conversation
|
@sameer As someone else who has been submitting requests, one thing to do for your environment, run: Then for commits and testing, make sure to run: And: These will check all of the test coverages and commit testing to make sure that everything works out. Another thing I would do is pull in a fixture file for the camera with the floodlight pieces on it so that all the tests work there too, run: And then: Those commands will help make sure that things you're running, adding, and changing, don't break things for existing code as well. It will also help make sure that all of the checks pass for the PR and that code looks good and follows existing guidelines. |
|
Will do, thanks!
|
rytilahti
left a comment
rytilahti
left a comment
There was a problem hiding this comment.
Thanks for the PR and improving the support! Would you mind adding some tests similarly to what's done on other modules?
It would also be great if this new module would implement the Light interface as that will allow providing a common interface no matter what type of light device is being controlled.
| from ..smartcammodule import SmartCamModule | ||
|
|
||
|
|
||
| class Floodlight(SmartCamModule): |
There was a problem hiding this comment.
If you would implement the Light interface (see how smart module does it), it would make interfacing with lights consistent across the different platforms.
| if ( | ||
| config is not None | ||
| and status is not None | ||
| and capability is not None | ||
| and "intensity_level" in config | ||
| and "intensity_level_max" in capability | ||
| and "min_intensity" in capability | ||
| ): |
There was a problem hiding this comment.
Instead of having multiple conditions and parsing data out from dicts, please consider creating a container class and using mashumaro's functionaity for parsing.
3 participants
Hello 👋
I'm adding floodlight control for the Tapo C720 and am looking for some feedback on my approach. Since it's a smart camera, I added floodlight control as a module -- does this make sense?
I tested and can confirm that the floodlight is controllable with the dev tool (
feature floodlight_state True). Will add tests for this PR if the way it's written looks good.