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?