Skip to content

Fix changing brightness when effect is active #1019

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

Merged
merged 14 commits into from
Jul 1, 2024
Merged

Conversation

rytilahti
Copy link
Member

@rytilahti rytilahti commented Jun 27, 2024

This PR changes the behavior of brightness module if an effect is active.
Currently, changing the brightness disables the effect when the brightness is changed, this fixes that.
This will also improve the set_effect interface to use the current brightness when an effect is activated.

  • light_strip_effect: passing bAdjusted with the changed properties changes the brightness.
  • light_effect: the brightness is stored only in the rule, so we modify it when adjusting the brightness. This is also done during the initial effect activation.

@rytilahti rytilahti added the enhancement New feature or request label Jun 27, 2024
@rytilahti rytilahti changed the title Implement effect brightness Fix changing brightness when effect is active Jun 27, 2024
@rytilahti rytilahti added this to the 0.7.0.2 milestone Jun 27, 2024
Copy link

codecov bot commented Jun 27, 2024

Codecov Report

Attention: Patch coverage is 95.89041% with 3 lines in your changes missing coverage. Please review.

Project coverage is 91.42%. Comparing base (2a62849) to head (88e3310).
Report is 89 commits behind head on master.

Files with missing lines Patch % Lines
kasa/smart/modules/lightstripeffect.py 88.00% 1 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1019      +/-   ##
==========================================
+ Coverage   91.33%   91.42%   +0.08%     
==========================================
  Files          84       84              
  Lines        5657     5724      +67     
  Branches     1371     1392      +21     
==========================================
+ Hits         5167     5233      +66     
  Misses        382      382              
- Partials      108      109       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rytilahti rytilahti force-pushed the fix/effect_brightness branch from 801744a to 9526ca2 Compare June 28, 2024 13:33
Copy link
Collaborator

@sdb9696 sdb9696 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks really good. Happy for you to merge if you want to defer the comment about the custom effect test to later.

Will update the branch with master.

docs/tutorial.py Outdated
Device ID: 0000000000000000000000000000000000000000\nState: True\nSignal Level: 2\nRSSI: -52\nSSID: #MASKED_SSID#\nOverheated: False\nBrightness: 50\nCloud connection: True\nHSV: HSV(hue=0, saturation=100, value=50)\nColor temperature: 2700\nAuto update enabled: True\nUpdate available: False\nCurrent firmware version: 1.1.6 Build 240130 Rel.173828\nAvailable firmware version: 1.1.6 Build 240130 Rel.173828\nLight effect: Party\nLight preset: Light preset 1\nSmooth transition on: 2\nSmooth transition off: 2\nDevice time: 2024-02-23 02:40:15+01:00
Device ID: 0000000000000000000000000000000000000000\nState: True\nSignal Level: 2\nRSSI: -52\nSSID: #MASKED_SSID#\nOverheated: False\nBrightness: 100\nCloud connection: True\nHSV: HSV(hue=0, saturation=100, value=50)\nColor temperature: 2700\nAuto update enabled: True\nUpdate available: False\nCurrent firmware version: 1.1.6 Build 240130 Rel.173828\nAvailable firmware version: 1.1.6 Build 240130 Rel.173828\nLight effect: Party\nLight preset: Not set\nSmooth transition on: 2\nSmooth transition off: 2\nDevice time: 2024-02-23 02:40:15+01:00
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessing the test_readme_examples.py picked this up. Great to see it working 😄

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it was picked up by tests indeed 🎉 Bad news is, that this is actually wrong output as setting the effect is supposed to keep the brightness in the previously set 50...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, the issue here was that the code was not changing the brightness when activating the effect. This should be now fixed, but the test bulb still thinks that it has an active preset.

@sdb9696 sdb9696 closed this Jun 28, 2024
@sdb9696 sdb9696 reopened this Jun 28, 2024
@rytilahti rytilahti requested a review from sdb9696 June 30, 2024 15:31
Copy link
Collaborator

@sdb9696 sdb9696 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, let's merge!

@rytilahti rytilahti merged commit b31a2ed into master Jul 1, 2024
27 checks passed
@rytilahti rytilahti deleted the fix/effect_brightness branch July 1, 2024 11:59
@sdb9696 sdb9696 added bug Something isn't working and removed enhancement New feature or request labels Jul 1, 2024
@sdb9696 sdb9696 mentioned this pull request Jul 1, 2024
sdb9696 added a commit that referenced this pull request Jul 1, 2024
## [0.7.0.2](https://github.com/python-kasa/python-kasa/tree/0.7.0.2) (2024-07-01)

[Full Changelog](0.7.0.1...0.7.0.2)

This patch release fixes some minor issues found out during testing against all new homeassistant platforms.

**Fixed bugs:**

- Disable multi-request on unknown errors [\#1027](#1027) (@sdb9696)
- Disable multi requests on json decode error during multi-request [\#1025](#1025) (@sdb9696)
- Fix changing brightness when effect is active [\#1019](#1019) (@rytilahti)
- Update light transition module to work with child devices [\#1017](#1017) (@sdb9696)
- Handle unknown error codes gracefully [\#1016](#1016) (@rytilahti)

**Project maintenance:**

- Make parent attribute on device consistent across iot and smart [\#1023](#1023) (@sdb9696)
- Cache SmartErrorCode creation [\#1022](#1022) (@bdraco)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants