Skip to content
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

detect_non_ha_changes fixes/additions - tests pending #450

Conversation

th3w1zard1
Copy link
Collaborator

@th3w1zard1 th3w1zard1 commented Mar 17, 2023

A couple of suggested fixes for #447 , tested and working on my setup.
EDIT: Do not use, a recent merge with upstream/master broke this PR. Please see #486 for the updated PR.

3 new config options:

  1. strict_adapting: false will check if the last_adapted value matches the current state after an update_entity() call. If they don't match within the already preconfigured error range, this will set manual_control: true for that switch.
  2. alt_detect_method: true will check for any significant changes in the opposite direction of where adaptive-lighting tried to adapt last. Ex: during a transition from 255 to 3 the brightness may change like 255->237->210->178->159->184->255, if this option is set, it'll detect 159->184->255 as a manual change.
  3. autoreset_control_time: <seconds> was a feature I added solely because I already added 90% of this feature from code from the other config options. If a light is set as manually controlled, after this number of seconds adaptive-lighting will reset manual_control for that specific light.

Important things to note:
> I've no idea if this will break working setups using the current detect_non_ha_changes: true as that feature doesn't work on my setup, hence why I wrote this PR detect_non_ha_changes will now do exactly what is already does in the main build, all config options are separate
> strict_adapting: false utilizes homeassistant.update_entity().
>autoreset_control_time applies to all lights on the switch equally. Couldn't be bothered to code it per light. this now operates on each light individually! Max value is 2147483647 or 0x7FFFFFFF, which is ~24855.13 days
> autoreset_control_time will possibly not reset an ongoing timer if a new manual event happens. If you notice this happening, please post debug logs.
>When strict_adapting: false is set,_async_update_at_interval (the basic interval function) will always wait for any ongoing transitions from prior adapt calls before attempting another adapt.

Again please see #447 for more info, specifically #447 (comment)

Current TODO list:

  • Update README.md
  • Resolve all remaining reviews.

Any user wishing to test this may download the pr here: detect_non_ha_changes_during_transition
Press the Download ZIP button, extract, and upload the custom_components folder to your HASS /config directory
image
please don't post issues with this PR in the main repo! Post all issues on this specific thread

If you'd like to test all of my changes in one build, checkout the dev-branch

@th3w1zard1
Copy link
Collaborator Author

th3w1zard1 commented Mar 18, 2023

Tested the config flow today and for some reason these new config options are not showing their descriptions correctly in the integration setup page.
image
I've no idea why this is happening, all of the new config options are correctly defined in strings.json and const.py

@th3w1zard1 th3w1zard1 changed the title Detect non ha changes fixes/additions detect_non_ha_changes fixes/additions Mar 18, 2023
@th3w1zard1
Copy link
Collaborator Author

@basnijholt Why are you only reviewing formatting concerns in my PRs? I don't mind fixing those for you! But I'd honestly love to hear your opinion and get feedback on the overall implementation.

Copy link

@nathang21 nathang21 left a comment

Choose a reason for hiding this comment

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

Overall makes sense to me, but I don't have much context on this repo or HA best practices.

custom_components/adaptive_lighting/strings.json Outdated Show resolved Hide resolved
Max int value is 2147483647 or 0x7FFFFFFF
@basnijholt
Copy link
Owner

@th3w1zard1, that’s a fair question 😄

The honest answer is that I am super busy with moving and work. So I haven’t properly read everything, but when quickly glancing over the code I just notice these small things and leave some replies. Hope to get some time soon to properly review these 😊

@th3w1zard1
Copy link
Collaborator Author

@basnijholt No worries! Thank you so much for blessing us with your project! I'm grateful for any time you have regardless.

@th3w1zard1 th3w1zard1 force-pushed the detect_non_ha_changes-during-transition branch from c2345a3 to 24f0464 Compare March 19, 2023 06:11
…y()` unnecessarily.

`take_over_control` is required for the following:
`detect_non_ha_changes: true`
`strict_adapting: false`
`alt_detect_method: true`
if any of the above config options are set, and `take_over_control: false`, a logger message will warn the user that take_over_control will be automatically enabled.

All of the above config options no longer rely on each other, and can be used separately or in harmony without combinational issues arising.

`alt_detect_method` no longer will call `update_entity()` unnecessarily
removed `_last_adapted_state` from switch class as `last_service_data` already contains that data.

Class TurnOnOffListener now knows how to get an AdaptiveSwitch object from a light's entity_id.

`detect_non_ha_changes` option now functions exactly the same as the main build
@th3w1zard1
Copy link
Collaborator Author

th3w1zard1 commented Mar 27, 2023

@nathang21 Perfect! I believe you have the build I'm looking for. Could you post the switch.py on https://gists.github.com or somewhere? If not no worries.

@th3w1zard1 th3w1zard1 changed the title detect_non_ha_changes fixes/additions detect_non_ha_changes fixes/additions - tests pending before merging with master Mar 27, 2023
@th3w1zard1 th3w1zard1 changed the title detect_non_ha_changes fixes/additions - tests pending before merging with master detect_non_ha_changes fixes/additions - tests pending Mar 27, 2023
@th3w1zard1 th3w1zard1 self-assigned this Mar 27, 2023
@basnijholt basnijholt force-pushed the master branch 4 times, most recently from 6974e9c to c05e0cc Compare March 27, 2023 16:05
@nathang21
Copy link

@nathang21 Perfect! I believe you have the build I'm looking for. Could you post the switch.py on gists.github.com or somewhere? If not no worries.

Sorry for the delay, here is the latest one I had.
adaptive-lighting-detect_non_ha_changes-during-transition (5).zip

@nathang21
Copy link

Looks like both my kitchen lightstrips, and my office desk_lightstrip are still getting flagged with manual control despite not changing anything.

I installed the latest build from this PR, restarted HA, and noticed manual control was set on all 3. I toggled AL on/off to clear it, and within a few minutes it was re-applied to all 3.

Config: (no changes)
image
image

Logs:
al2.log

Let me know if I should use #449 or file another issue.

@th3w1zard1
Copy link
Collaborator Author

@nathang21 Thanks for the update. Come back in an hour then check #486 I should be done by then.

@th3w1zard1 th3w1zard1 closed this Mar 29, 2023
@th3w1zard1 th3w1zard1 deleted the detect_non_ha_changes-during-transition branch March 30, 2023 01:41
th3w1zard1 added a commit that referenced this pull request Apr 3, 2023
* 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]>
basnijholt added a commit that referenced this pull request Apr 3, 2023
* 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]>
basnijholt added a commit that referenced this pull request Apr 3, 2023
* 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>
basnijholt added a commit that referenced this pull request Apr 4, 2023
* Add auto_reset_manual_control with async timer

* cherry-pick wait for transition stuff

* Update switch.py

* not renamed in this branch yet.

* Update switch.py

* update tests

* Update switch.py

* merge related fix

* cleanup

* Revert "cleanup"

This reverts commit 3aa2f32.

* Update switch.py

* Update switch.py

* Update switch.py

* Small refactor

* Move test to old position for better diffs

* Revert "Small refactor"

This reverts commit b986b3f.

* Update README.md

* fix the test

last_state_change isn't updated quick enough.

* #510 changes (#516)

* 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>

* No need to wrap the reset

* Remove unused attrs

* Shorter log message

* revert unrelated tests change

* remove unused function

* Use patch

* Bump to 1.10.0

---------

Co-authored-by: Bas Nijholt <[email protected]>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Bas Nijholt <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants