| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## master #1713 +/- ##
==========================================
+ Coverage 93.42% 93.47% +0.04%
==========================================
Files 160 161 +1
Lines 10091 10168 +77
Branches 1041 1050 +9
==========================================
+ Hits 9428 9505 +77
Misses 471 471
Partials 192 192 ☔ View full report in Codecov by Harness.
|
Sorry, something went wrong.
There was a problem hiding this comment.
Thanks for the PR @ijardillier, I have done a quick review on it. The module looks very straightforward, so this can be merged rather quickly after the issues mentioned in the comments are fixed.
Sorry, something went wrong.
| @@ -0,0 +1,96 @@ | |||
| """Module for white lamp (floodlight) controls on Tapo outdoor cameras.""" | |||
There was a problem hiding this comment.
If it is a floodlight, feel free to name the class also like that, no need to keep the internal namings :-) The name of the class and the file could be simply FloodLight and floodlight.py respectively. If you want to keep the internal naming available in docstrings, you can put that in parentheses.
Sorry, something went wrong.
| """Implementation of white lamp controls for outdoor cameras (e.g. C325WB). | ||
|
|
||
| The white lamp is the built-in floodlight that can be triggered manually | ||
| or by motion events. It is distinct from the LED status indicator. |
There was a problem hiding this comment.
| """Implementation of white lamp controls for outdoor cameras (e.g. C325WB). | |
| The white lamp is the built-in floodlight that can be triggered manually | |
| or by motion events. It is distinct from the LED status indicator. | |
| """Implementation of floodlight controls for outdoor cameras. |
Sorry, something went wrong.
| @@ -0,0 +1,96 @@ | |||
| """Module for white lamp (floodlight) controls on Tapo outdoor cameras.""" | |||
There was a problem hiding this comment.
| """Module for white lamp (floodlight) controls on Tapo outdoor cameras.""" | |
| """Module for floodlight controls ("whitelamp" module) on Tapo outdoor cameras.""" |
If it is a floodlight, feel free to name the class also like that, no need to keep the internal namings :-) The name of the class and the file could be simply FloodLight and floodlight.py respectively. If you want to keep the internal naming available in docstrings, you can put that in parentheses.
Sorry, something went wrong.
| Feature( | ||
| self._device, | ||
| id="white_lamp_state", | ||
| name="White lamp state", |
There was a problem hiding this comment.
See my comment above, these are user-interfacing names.
Sorry, something went wrong.
There was a problem hiding this comment.
I am not sure what is going on here, newline separator changes?
Sorry, something went wrong.
| _LOGGER = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| class WhiteLamp(SmartCamModule): |
There was a problem hiding this comment.
| class WhiteLamp(SmartCamModule): | |
| class WhiteLamp(SmartCamModule, LightInterface): |
It would be nice to inherit from the lightinterface like other light modules do. This will make it also easier to integrate elsewhere.
Sorry, something went wrong.
- New WhiteLamp module under kasa/smartcam/modules/ - Exposes white_lamp_state (on/off toggle via reverseWhitelampStatus) and white_lamp_brightness (1-100, only if device supports it) - Adds Module.SmartCamWhiteLamp and Module.WhiteLamp module names - Adds fixture for C325WB(EU) 1.0 / 1.4.4 - Adds unit tests in tests/smartcam/modules/test_whitelamp.py - Fixes fake protocol to handle reverseWhitelampStatus toggle command
to match the class name used as key in dev.modules - Add SmartCamModule.SmartCamWhiteLamp assertion in test_whitelamp_is_on - Add test_whitelamp_no_brightness to cover the case where brightness is not supported by the device"
|
Hi @rytilahti Thanks for the review ans sorry for the delay! |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
New WhiteLamp module under kasa/smartcam/modules/
Pre-commit OK
Tests : OK
Also tested with python-kasa CLI and my own Home Assistant instance with 2 C325WB cameras.
Hope all is compliant with your guidelines and nothing missing.