Correctly wait for transitions #510

th3w1zard1 · 2023-04-03T03:56:40Z

This PR changes two things:

Wait for transitions before adapting a light
Remove all of the unnecessary code that assumes a transition could still be happening (e.g. cnt_significant_changes)

I've named all the PRs with number headings representing the ascending order they should merge in.

Hope this simplifies the changes a bit as I totally agree that having everything in one PR was a bit too much upkeep.

This reverts commit 3aa2f32.

This reverts commit b986b3f.

basnijholt · 2023-04-03T07:42:07Z

I seemed to have broken something accidentally so I reverted my change. I will continue in #516.

last_state_change isn't updated quick enough.

@basnijholt

* Change (WIP) * Update test_switch.py * Refactor * Revert "Revert "Small refactor"" This reverts commit 3731c99. * Update README.md * Fix the test * Bump to 1.9.0 (#518) * Basic community fixes PR (#460) * Fixes #423 #423 * Do not adapt lights turned on with custom payloads. * Update switch.py * Issue fixes #423, #378, #403, #449 * quickly test #274 * Revert feature requests, this branch only has fixes. Reverted FR 274 * pre-commit fix * Create automerge.yaml * test * Delete automerge.yaml My bad. * Fix #460 and #408 * see @basnijholt 's comment in #450. * @basnijholt requested changes. --------- Co-authored-by: Bas Nijholt <[email protected]> * Undo accidental changes introduced in #509, but adds the changes from #460 (#521) * Release 1.9.1 (#522) * Bump to 1.9.1 * Add CODEOWNERS --------- Co-authored-by: Benjamin Auquite <[email protected]> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

th3w1zard1 · 2023-04-03T21:03:30Z

Once this is implemented, I highly recommend changing the default config values.

e.g. interval: 5 transition: 60 as updating attributes isn't nearly as cpu-intensive as calling light.turn_on. A reminder that we're also checking if the service data differs from the last adapt in 1.9.1

th3w1zard1 · 2023-04-03T21:04:40Z

Awesome!

I believe I didn't explain the purpose of cnt_significant_changes properly. The idea is that sometimes the light changes not because of a light.turn_on event, but because the light has been changed e.g., on a dimmer in the wall, or via Google Home.

Therefore we are seeing whether a light has changed.

I think your fix is orthogonal to the cnt_significant_changes code being broken.

We need your fix regardless, but I think we shouldn't remove the cnt_significant_changes functionality (given we still want that fixed).

I don't quite see the purpose of this? Once a transition finishes, we're updating the light's state via update_entity to see if it correctly matches the last service data. What more needs to be done? If a light is updated outside of home assistant, update_entity should still refresh its state in HASS.

Also the current implementation in this PR passes the test_significant_change test where we test non ha changes.

custom_components/adaptive_lighting/switch.py

th3w1zard1 · 2023-04-03T21:59:10Z

@basnijholt I moved this here for accessibility:

The ONLY possible cases where detect_non_ha_changes can fail in this pr (ordered most to least problematic):

Light never got the light.turn_on command we sent with the last service data somehow. Haven't seen this happen in the two weeks I've been using the new significant_change code. Perhaps this could be tested in our tests?
Light went offline during the transition of the last adapt, code will currently fire the manual control event for any bulbs that go offline. This was an already outstanding issue in the old code. (see Initial Adjustment Delay after light comes online (from unavailable) #307)
Light is improperly configured by the user and doesn't report any state changes to hass even after an update_entity() call (e.g. zigbee bulb where reporting is configured to off)
(fixed in Fix RGB Color Temp Swaps #514) Light's entity state was correctly adapted to last_service_data but the light converted the color data (e.g. color_temp_kelvin used in our service data but was converted by the light to rgb_color)

Please add to this list if you think of any outstanding issues!

th3w1zard1 · 2023-04-03T22:28:57Z

@basnijholt What is the is_async argument used for in _fire_manual_control_event? I notice it's only False for significant_change

tests/test_switch.py

basnijholt · 2023-04-04T00:02:16Z

This is really great!

Glad you got to the bottom of this 3-year-old bug 😄

I changed the manifest.json version to 1.10.0, such that I can tag a new version right away.

th3w1zard1 · 2023-04-04T00:03:42Z

This is really great!

Glad you got to the bottom of this 3-year-old bug 😄

I changed the manifest.json version to 1.10.0, such that I can tag a new version right away.

Can we include #513 and #512 in the release? Having alt_detect_method may be nice for users still experiencing problems with detect_non_ha_changes

basnijholt · 2023-04-04T00:05:39Z

I prefer to take it one at the time because those PRs are another 500+ lines of code changes.

I also don't have time today to review those anymore :-(

th3w1zard1 · 2023-04-04T00:06:44Z

I prefer to take it one at the time because those PRs are another 500+ lines of code changes.

I also don't have time today to review those anymore :-(

Good idea! Thank you SO much!!

basnijholt · 2023-04-04T01:55:45Z

@all-contributors please add @th3w1zard1 for bugs

allcontributors · 2023-04-04T01:55:54Z

@basnijholt

I've put up a pull request to add @th3w1zard1! 🎉

basnijholt · 2023-04-04T07:57:34Z

@all-contributors please add @th3w1zard1 for maintenance

allcontributors · 2023-04-04T07:57:43Z

@basnijholt

I've put up a pull request to add @th3w1zard1! 🎉

Add auto_reset_manual_control with async timer

52275bf

th3w1zard1 force-pushed the wait_for_transitions branch from 59f358b to 52275bf Compare April 3, 2023 03:57

th3w1zard1 added 7 commits April 2, 2023 23:01

Merge branch 'main' into wait_for_transitions

d7aecc9

cherry-pick wait for transition stuff

8b2fcf9

Update switch.py

22bae9e

not renamed in this branch yet.

8e847e7

Update switch.py

30c62bf

update tests

120efff

Update switch.py

1571dfa

th3w1zard1 changed the title ~~Wait for transitions~~ (Main) Wait for transitions Apr 3, 2023

th3w1zard1 requested a review from basnijholt April 3, 2023 04:54

merge related fix

732fc17

th3w1zard1 changed the title ~~(Main) Wait for transitions~~ (1.) (Main) Wait for transitions Apr 3, 2023

th3w1zard1 changed the title ~~(1.) (Main) Wait for transitions~~ < 1 > (Main) Wait for transitions Apr 3, 2023

th3w1zard1 added 5 commits April 3, 2023 00:55

cleanup

3aa2f32

Revert "cleanup"

8365950

This reverts commit 3aa2f32.

Update switch.py

cebf31a

Update switch.py

2f0bad8

Update switch.py

4479761

th3w1zard1 added the status/done+waiting-for-review label Apr 3, 2023

basnijholt and others added 5 commits April 3, 2023 00:20

Small refactor

b986b3f

Move test to old position for better diffs

b4c6bf8

Revert "Small refactor"

3731c99

This reverts commit b986b3f.

Merge 3731c99 into 9caf304

26c32ac

Update README.md

a649198

th3w1zard1 and others added 4 commits April 3, 2023 03:29

fix the test

96f59f0

last_state_change isn't updated quick enough.

Merge branch 'master' into wait_for_transitions

9cc3954

Merge remote-tracking branch 'origin/main' into wait_for_transitions

445b5da

basnijholt reviewed Apr 3, 2023

View reviewed changes

custom_components/adaptive_lighting/switch.py Show resolved Hide resolved

basnijholt added 2 commits April 3, 2023 15:21

Remove unused attrs

197d2ab

Shorter log message

b720a4c

th3w1zard1 changed the title ~~< 1 > (Main) Wait for transitions~~ [ 1 ] (Main) Wait for transitions Apr 3, 2023

basnijholt added 2 commits April 3, 2023 16:25

revert unrelated tests change

24a784b

remove unused function

23790fb

basnijholt reviewed Apr 3, 2023

View reviewed changes

tests/test_switch.py Outdated Show resolved Hide resolved

basnijholt added 2 commits April 3, 2023 16:56

Use patch

08daa19

Merge remote-tracking branch 'origin/main' into wait_for_transitions

fd4e47a

basnijholt changed the title ~~[ 1 ] (Main) Wait for transitions~~ Correctly wait for transitions Apr 3, 2023

basnijholt approved these changes Apr 3, 2023

View reviewed changes

Bump to 1.10.0

5c9547c

basnijholt merged commit c7f44e4 into main Apr 4, 2023

basnijholt deleted the wait_for_transitions branch April 4, 2023 00:04

allcontributors bot mentioned this pull request Apr 4, 2023

docs: add th3w1zard1 as a contributor for bug #534

Merged

allcontributors bot mentioned this pull request Apr 4, 2023

docs: add th3w1zard1 as a contributor for maintenance #538

Merged

th3w1zard1 restored the wait_for_transitions branch April 6, 2023 03:27

th3w1zard1 mentioned this pull request Apr 6, 2023

Revert broken code introduced in 1.10.0 #547

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Correctly wait for transitions #510

Correctly wait for transitions #510

th3w1zard1 commented Apr 3, 2023 •

edited

Loading

basnijholt commented Apr 3, 2023

th3w1zard1 commented Apr 3, 2023

th3w1zard1 commented Apr 3, 2023 •

edited

Loading

th3w1zard1 commented Apr 3, 2023 •

edited

Loading

th3w1zard1 commented Apr 3, 2023

basnijholt commented Apr 4, 2023

th3w1zard1 commented Apr 4, 2023 •

edited

Loading

basnijholt commented Apr 4, 2023

th3w1zard1 commented Apr 4, 2023

basnijholt commented Apr 4, 2023

allcontributors bot commented Apr 4, 2023

basnijholt commented Apr 4, 2023

allcontributors bot commented Apr 4, 2023

Correctly wait for transitions #510

Correctly wait for transitions #510

Conversation

th3w1zard1 commented Apr 3, 2023 • edited Loading

basnijholt commented Apr 3, 2023

th3w1zard1 commented Apr 3, 2023

th3w1zard1 commented Apr 3, 2023 • edited Loading

th3w1zard1 commented Apr 3, 2023 • edited Loading

th3w1zard1 commented Apr 3, 2023

basnijholt commented Apr 4, 2023

th3w1zard1 commented Apr 4, 2023 • edited Loading

basnijholt commented Apr 4, 2023

th3w1zard1 commented Apr 4, 2023

basnijholt commented Apr 4, 2023

allcontributors bot commented Apr 4, 2023

basnijholt commented Apr 4, 2023

allcontributors bot commented Apr 4, 2023

th3w1zard1 commented Apr 3, 2023 •

edited

Loading

th3w1zard1 commented Apr 3, 2023 •

edited

Loading

th3w1zard1 commented Apr 3, 2023 •

edited

Loading

th3w1zard1 commented Apr 4, 2023 •

edited

Loading