Changing brightness of entity does not count as manual control during timeout

[adamkanovak]

Is your feature request related to a problem? Please describe. A clear and concise description of what the problem is. Ex. I'm always frustrated when [...] (3.2) An entity that is manually controlled within the time-out period should have its timer cancelled, and therefore stay on.

This point is not affected by changing brightness.

Use-case:

Light is turned on by ec. User changes the brightness during timeout. Ec turns the light off when the timeout expires.

Describe the solution you'd like A clear and concise description of what you want to happen.

Would be great if 3rd point would not occur and changing the brightness would count as manual control.

Describe alternatives you've considered A clear and concise description of any alternative solutions or features you've considered.

Additional context Add any other context or screenshots about the feature request here.

[danobot]

Hmm I guess I tested by changing a lights color. Any idea why brightness is not recognised as a state change?

[adamkanovak]

my guess is that ha entity state does not actually change with brightness, since it is always on. it is only an attribute of the entity.

[mikeage]

For what it's worth, I'd love to see this (brightness, color, temperature) as a customizable option (I can submit a PR if you think this is a reasonable idea) as I use https://github.com/claytonjn/hass-circadian_lighting and I wouldn't want those changes to be treated as a change that disables EC

[danobot]

What do you mean "as a customizable option"? If you have a PR to resolve this issue then you are more than welcome to contribute. I happily accept contributions.

[mikeage]

I mean that instead of always treating brightness and color changes as manual changes, it'd be able to ignore them.

I'll work on it today. Thanks.

[danobot]

This issue is reporting that (3.2) is not working, right? I think that needs to be resolved first. You can raise a second issue for an enhancement for what you suggest :) It should be something like a configurable list of enitty attributes that will be ignored on state updates. e.g.

  mtn_office:
    sensor: binary_sensor.office_motion
    trigger_on_activate: light.office_led
    delay: 120
    state_attributes_ignore:
        - brightness
        - color

Don't make it specific to brightness and color, it should be configurable for any type of entity :) Great idea actually

[mikeage]

Interestingly, I've been playing around a bit with the async updates, and it seems like entity_controller is getting all of the proper notifications on both brightness and color. @danobot , can you see if you can reproduce what @adamkanovak reported? [I think I also have the proper ifs ready for the state_attributes_ignore option, but I want to do proper testing when I'm physically present to confirm that everything is working before I open a PR]

[mikeage]

Sorry for the confusion; I was missing something. It seems like the blocked state is only entered via a sensor_on message, which can only come from sensor_state_change() which comes from when the sensor changes, not when the control changed. Is this the way it's supposed to be correct? If a light is on and an attribute is changed, we get to state_entity_state_change() which can only emit control and enable. Could that explain the defect here?

I'd first put some filtering in state_entity_state_change(), but as you mentioned, this improvement is dependent on fixing this bug. Is my analysis correct?

[sorry for the simple questions; I'm still trying to figure out exactly how this whole thing fits together, although the use of an FSM is a great choice]

[danobot]

@mikeage the FSM is complex and understanding it is difficult...even for me. This is why the need for automated unit tests becomes ever more apparent as raised in #101 . What I would do is implement what you suggest based on your understanding, let it run in your HASS instance for a few weeks and you will undoubtedly find any issues with it. (That is how I "sanity" test :)

[mikeage]

I agree about the tests, but the hardest part is understand what happens to make sure the tests are accurate :-)

I'd written an implementation, but it was not working as I expected, and I believe it was for the reason I mentioned. What do you think about a transition of sensor_on from state_entity_state_change? Part of the issue is also the fact that sensor_on seems to mean both sensor_turned_on and sensor_is_on_now; I think we only want to move to blocked if it's sensor_turned_on or sensor_is_on_and_a_relevant_attribute_changed (sorry for the awful names ;-) )

[danobot]

Hi, there is a single call to trigger the SENSOR_ON event. It is inside sensor_state_change.

state_entity_state_change is a callback to track state entities. (Are you using state entities? If not, dont worry about them).

Part of the issue is also the fact that sensor_on seems to mean both sensor_turned_on and sensor_is_on_now

You might be getting confused with is_sensor_on() which is a simple check to determine if the sensor is on.

Check the state diagram, EC will only react to SENSOR_ON in the Idle state. From there it may go to Active or Blocked state. I'm going to use formatting because there are a lot of method names and proper nouns here haha!

[mikeage]

state_entity_state_change tracks state entities, but if they aren't defined, it tracks the control entities (see https://github.com/danobot/entity-controller/blob/master/custom_components/entity_controller/__init__.py#L630 )

Changing brightness or color_temp of a light that's lit will NOT trigger sensor_state_change (of course), but it will trigger state_entity_state_change. For requirement 3.2, I think this is the only notification we'll ever get. How did you mean to implement 3.2?

[danobot]

Now I remember how 3.2 works! 3.2 should be implemented via the CONTROL trigger on this line.

The corresponding state transition is on this line of code here This applies only if the entity is in active_timer state. (as per 3.2)

   @callback
def state_entity_state_change(self, entity, old, new):
    """ State change callback for state entities """
    if self.is_active_timer():
        self.control()

This transition has a condition and EC will only change states if the state entities are off.

Again, refer to the diagram:

I think what we need is another transition to cater for the case where the state entities are ON!!

The most logical state to go would be blocked. How do we get out of that state?? I think when the state entity is switched off, EC will go back to idle. Makes sense?

[mikeage]

Understood. Ok, so that makes sense (I wasn't clear on what the control signal represented; now I get it). I need to spend a little more time understanding why this event would get sent if the entity is off, since entities that are off shouldn't have attributes in the first place, but I'm guessing it might be a race condition between the trigger of the event and the test of is_state_entities_off. Maybe.

I agree we need to go to blocked in that case. (Or, following my request, maybe ignore it if the only change is certain attributes)

As far as going back to idle from blocked if it's shut off... maybe. It makes sense, but think about the following. Imagine that the motion sensor turns on the device, and the user says "shh, people are sleeping", and shuts it off. Do we really want to go back to idle and have it trigger again in a few seconds? Manual turn_on means blocked until the device is (manually) shut off, but is a manual turn_off significant, too? There is some code there (with the enable signal), but I'm still working my way through what that means.

[danobot]

Feel free to leave comments as you go to help document. For now I will add this transition as it is required for 3.2. There will always be edge cases like that. In that case it can be solved using an override entity when people are sleeping. Something like that :)

An alternative would be a new state such as doomed where the entity controller is completely shut off. How do we get it back to idle though? The answer could be support for service calls such that HASS automations can trigger state transitions of the EC entity. (This has been on my backlog for a long time, but dont have any use for it yet, that's why I never bothreed to implement it.)

Your #106 can be easily integrated in the state_entity_state_change method.

[mikeage]

Nitpick: 106 wasn't mine :-)

I'll keep working on my request (I'm actually going to open a new issue for it now), and I'll wait for you to add the transition here. Thanks!

[danobot]

Resolved in v4.1.5

[danobot]

rolled back in v4.1.6

[jwelter1971]

Is your feature request related to a problem? Please describe. A clear and concise description of what the problem is. Ex. I'm always frustrated when [...] (3.2) An entity that is manually controlled within the time-out period should have its timer cancelled, and therefore stay on.

This point is not affected by changing brightness.

Use-case:

Light is turned on by ec. User changes the brightness during timeout. Ec turns the light off when the timeout expires.

Describe the solution you'd like A clear and concise description of what you want to happen.

Would be great if 3rd point would not occur and changing the brightness would count as manual control.

Describe alternatives you've considered A clear and concise description of any alternative solutions or features you've considered.

Additional context Add any other context or screenshots about the feature request here.

I'm not sure this is the best behavior. I have a dimming scheme that runs to lower light levels of all lights that are on in the evening. These dimming events should not impact EC operation in any way as I want lights switched on./off based on motion.

Ideally HA would allow us to understand if the lights were turned on/dimmed manually or under programatic control as that would be ideal.

[mikeage]

@jwelter1971 , unfortunately, there's no easy way to identify the source of a change. HA doesn't provide any ID for what triggered a change.

My motivation behind issue #109 and the fix in PR #112 was to allow one to switch between "brightness is treated as manual intervention" and "brightness is ignored", but I agree that "manual brightness is significant, automatic brightness is not" would certainly be the best option! I just don't know how it can be done.

You might find the thread at https://github.com/claytonjn/hass-circadian_lighting/issues/51 interesting; it's a sort of running log of my attempts to get an automatic brightness (and color temp) component to recognize changes I did as significant, without getting confused by the big changes the component itself makes if you turn off a bulb at noon and on at midnight. One other challenge is that entities that are off do not have attributes, so you can't do something like "always update the brightness in the background, but just leave it off".

Good luck; I'd love to hear if you have any ideas for either that thread or this one (though for this one, it should probably move to issue #109, as this is really a bug report

[jwelter1971]

@jwelter1971 , unfortunately, there's no easy way to identify the source of a change. HA doesn't provide any ID for what triggered a change.

My motivation behind issue #109 and the fix in PR #112 was to allow one to switch between "brightness is treated as manual intervention" and "brightness is ignored", but I agree that "manual brightness is significant, automatic brightness is not" would certainly be the best option! I just don't know how it can be done.

You might find the thread at claytonjn/hass-circadian_lighting#51 interesting; it's a sort of running log of my attempts to get an automatic brightness (and color temp) component to recognize changes I did as significant, without getting confused by the big changes the component itself makes if you turn off a bulb at noon and on at midnight. One other challenge is that entities that are off do not have attributes, so you can't do something like "always update the brightness in the background, but just leave it off".

Good luck; I'd love to hear if you have any ideas for either that thread or this one (though for this one, it should probably move to issue #109, as this is really a bug report

Supposedly HA has been extended to have a "reference" in the trigger data but it's not populated yet. The idea was this would contain the automation that triggered the change, or hopefully some way to indicate if it was a manual trigger. That way we should be able to get what we need to really deal with these corner cases that right now are compromised from handling properly.

For now I guess we just need to have this as an option, so we can pick the best compromise for our setups.

[danobot]

What is the outcome of this discussion? Is it possible to close this issue or not?

[danobot]

Closing this issue.

[broyuken]

Is 3.2 still broken? I am just trying to implement it myself now, and if I manually change the brightness either via HA or via the wall it does not put it into a blocked state. The timer still shuts off after no motion.