Add Dimmer Configuration Support #1484
Conversation
ryenitcher
commented
Jan 25, 2025
- Add: Support for configuring minimum dimming level.
- Add: Support for configuring dimmer ramp rate.
- Add: Support for configuring dimmer gentle fade time.
- Add: Support for configuring dimmper fade time.
|
Hopefully breaking out the dimmer configuration into a feature from the module is acceptable. I've tested this against an KS220, and it appears to work well, but I can look into adding proper tests for this if the change seems reasonable to you. |
|
This should cover #1464 |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1484 +/- ##
==========================================
+ Coverage 92.64% 92.71% +0.06%
==========================================
Files 149 150 +1
Lines 9428 9524 +96
Branches 957 968 +11
==========================================
+ Hits 8735 8830 +95
+ Misses 497 496 -1
- Partials 196 198 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Looks really great and it's nice to see also older devices getting some love, thanks @ryenitcher! 💯
I added some comments (which likely apply for all these settings) inline, let me know if you think those changes make sense or not. On tests, it would be great to have some, either test_motion.py or test_ambientlight.py could be a good starting point.
kasa/iot/modules/dimmer.py
Outdated
|
|
||
| :param min: The minimum dimming level, in the range 0-51. | ||
| """ | ||
| if (min < 0) or (min > 51): |
There was a problem hiding this comment.
| if (min < 0) or (min > 51): | |
| if min < 0 or min > 51: |
Is it really up to 51 and not 50? In any case, no need for parentheses here.
There was a problem hiding this comment.
Yeah if you use the app to set the dimmer trim to max, it sets the value to 51, which surprised me as well. The API rejects 52 as a value though, so it has to end on 51. Perhaps there is a special bypass mode on the triac or something for no dimming, and that's a special value used to denote that?
There was a problem hiding this comment.
Could be something special indeed, but we can keep it as it is and adjust when someone chimes in with an issue report :)
There was a problem hiding this comment.
Could be worth adding a small comment here about the tested app behaviour.
- Add: Support for configuring minimum dimming level. - Add: Support for configuring dimmer ramp rate. - Add: Support for configuring dimmer gentle fade time. - Add: Support for configuring dimmper fade time.
- Fix: Return time values as `timedelta` instead of `int`. - Fix: Move constants to top of file. - Fix: Correct feature types. - Fix: Expose range of bounded features.
77299dd to
cdbac94
Compare
- Add: Tests for IOT Dimmer feature.
|
I'm not sure if it's worth writing a test for this line? I doubt it will ever be triggered, and even then all it does its immediately return? |
There was a problem hiding this comment.
Just some minor style-related nitpicks but this looks otherwise good to go, thanks @ryenitcher!
It looks like the KS240 might be the only “new” dimmer so far (from searching the fixture files), but it looks like the config layout for the IOT version is roughly the same. I can copy over support to IOT in a follow up PR if that works. The new matter dimmer (KS225) might also be an IOT dimmer, but I don’t currently have one to test against. |
- Fix: Correct inline comment formatting. - Fix: Remove un-needed check for `get_dimmer_parameters` on dimmers. - Fix: Remove extra newline from docstring summaries.
|
I think this PR should now be ready to go. I've opened #1495 in draft status for calibration support on the SMART devices. |
## [0.10.1](https://github.com/python-kasa/python-kasa/tree/0.10.1) (2025-02-02) [Full Changelog](0.10.0...0.10.1) **Release summary:** Small patch release for bugfixes **Implemented enhancements:** - dustbin\_mode: add 'off' mode for cleaner downstream impl [\#1488](#1488) (@rytilahti) - Add Dimmer Configuration Support [\#1484](#1484) (@ryenitcher) **Fixed bugs:** - Do not return empty string for custom light effect name [\#1491](#1491) (@sdb9696) - Add FeatureAttributes to smartcam Alarm [\#1489](#1489) (@sdb9696) **Project maintenance:** - Add module.device to the public api [\#1478](#1478) (@sdb9696)