frontend icon indicating copy to clipboard operation
frontend copied to clipboard

extend optimistic state timeout to 10 seconds

Open marcelveldt opened this issue 2 years ago • 21 comments

Proposed change

Discussed many times already in forums, discord etc. Some integrations and/or devices are just a bit slow to update, for example some Z-Wave devices.

You send a call service to turn on a light but the new state only arrives seconds later. This causes a weird "flip flop" in the UI. You turn on the toggle, 2 seconds later it turns off again and a few seconds later it will turn on again. For most people this will mean they will probably go wild toggling the entity as they do not understand why it toggled back in the UI.

Some attempts have been made to try to fix this in the integrations but that just feels hacky. Not to say that this approach is better but it is way more simpler and works for all integrations at once.

This extends the optimistic timeout from the current 2 seconds to 10 seconds. In other words: This will give the integration up to 10 seconds to report back the actual/updated state.

Type of change

  • [ ] Dependency upgrade
  • [X] Bugfix (non-breaking change which fixes an issue)
  • [ ] New feature (thank you!)
  • [ ] Breaking change (fix/feature causing existing functionality to break)
  • [ ] Code quality improvements to existing code or addition of tests

Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue or discussion: https://github.com/home-assistant/core/pull/59385
  • Link to documentation pull request:

Checklist

  • [ ] The code change is tested and works locally.
  • [ ] There is no commented out code in this PR.
  • [ ] Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

marcelveldt avatar Nov 30 '21 15:11 marcelveldt

10 seconds is too long to "lie". We should explore some indicator that it's pending.

balloob avatar Nov 30 '21 17:11 balloob

10 seconds is too long to "lie". We should explore some indicator that it's pending.

The pending indicator is even better approach.

  1. Turn on light
  2. Set "pending" variable
  3. Have some kind of visual feedback about this pending state (e.g. disable the control)
  4. Reset pending state after the timeout OR when state update arrives for this entity

Easiest approach for first version would be to tie disabled state of the control to the pending state.

AND/OR have an entity attribute with the update delay, defaulting to 2 seconds and can be overridden at entity-level.

marcelveldt avatar Nov 30 '21 18:11 marcelveldt

The downside with disabling is that is what we do when the entity is unavailable.

balloob avatar Nov 30 '21 18:11 balloob

Hmmm, yeah you are right. So some kind of "loading indicator" would be better perhaps. I'll try if I can create something like that. Is there already some kind of in-place loading indicator used in the frontend to re-use for this ?

marcelveldt avatar Nov 30 '21 18:11 marcelveldt

We don't have something like this for toggles. For buttons we have a "spinner on button" although I am not sure if we still use it?

We should let @matthiasdebaat take a look too.

balloob avatar Nov 30 '21 18:11 balloob

The UX pattern of a Switch state that it immediately activate or deactivate something. 10 seconds is in that case way to long. The virtual switch is like a real world one. If you tap it and it doesn’t show immediate result you get confused. And maybe tap it again. So technically the switch should only toggle when there is really something happening. That is in our situation not nice, so I think the current two seconds is a good solution.

This is my suggestion: When the users click the switch, it toggles like we do now. When the integration doesn’t report back after 2 seconds, we replace the switch with a spinner. I want to keep the UI as clean as possible for the 80% most common situation.

matthiasdebaat avatar Dec 01 '21 18:12 matthiasdebaat

That sounds like a very good solution @matthiasdebaat. I'm not sure about numbers but I think we're talking about a small amount of integrations/devices that do not report back within 2 seconds. In fact I once had a few of those devices and when both I and my wife got frustrated I ditched them to the trash.

Anyways this issue gets reported many times on the forums and issue trackers so would be nice to fix. I'll have a look at the spinner approach. Shouldn't be hard code-wise but maybe you can point me at an already existing spinner to (re)use for this purpose ?

Schermafbeelding 2021-12-02 om 16 03 32

marcelveldt avatar Dec 02 '21 14:12 marcelveldt

That video is an example from the docs about what not to do 😝

balloob avatar Dec 02 '21 16:12 balloob

Yeah, I was just referencing that small spinner icon/animation :-)

marcelveldt avatar Dec 02 '21 16:12 marcelveldt

src/components/ha-circular-progress.ts

bramkragten avatar Dec 02 '21 18:12 bramkragten

This approach works for people who are watching the UI. It doesn't solve the problem for those using voice commands (e.g. Alexa) though.

kpine avatar Dec 03 '21 18:12 kpine

This approach works for people who are watching the UI. It doesn't solve the problem for those using voice commands (e.g. Alexa) though.

What do you mean? Does the voice assistant give feedback that setting the new value failed ?

EDIT: Confirmed by reading the forum post. The voice assistant responds that the command failed while the light actually turned on/off. So the voice assistant (or any other listener for the command) should also retrieve a temporary optimistic state.

marcelveldt avatar Dec 03 '21 19:12 marcelveldt

Converted to draft as this needs more discussion

marcelveldt avatar Dec 03 '21 19:12 marcelveldt

Given the response of @kpine that this also has impact to the voice assistants this needs more thought and can't be handled in the UI entirely.

Idea:

  1. Entity attribute with an optimistic_state timeout (or a bool and have a static timeout).
  2. After a command has been executed set some flag that the state is optimistic
  3. Set a timeout to cancel the optimistic state if a new state did not come in within the timespan
  4. UI can act on the optimistic flag by showing the spinner

Or something like that.

marcelveldt avatar Dec 03 '21 19:12 marcelveldt

BTW: Do we have any idea which integrations actually have this issue ? I'm aware of Z-wave but are there any others that are slow with their state reporting ? I quickly browsed some integrations this week and I spotted some that actually report the set state without actually relying on the actual state from the device.

marcelveldt avatar Dec 03 '21 19:12 marcelveldt

Hello, I started an issue related to this PR a few months ago https://github.com/home-assistant/core/issues/59379 and wondering if the change in 2022.2 https://github.com/home-assistant/core/pull/64621 is related to or resolves this? Thanks!

mwolter805 avatar Feb 05 '22 14:02 mwolter805

It would be nice to have this fixed. What needs to be worked out RE the voice assistants?

blhoward2 avatar Feb 06 '22 04:02 blhoward2

It would be nice to have this fixed. What needs to be worked out RE the voice assistants?

Imo the only way to fix this is at core level (for all integrations):

  1. Send a command to a device/light
  2. Update the state and mark it as optimistic
  3. Expect a state update to happen from the device within X seconds
  4. If a state is being marked as optimistic, show this in some visual way to the user and prevent a new command until the confirmation is there.
  5. State update received from device --> remove optimistic mark and update state
  6. Timeout expired (no state received from device) --> remove optimistic mark and restore state

This is an architecture decision. Speaking of making things easier and reorganizing stuff in HA I think this fits there exactly. This is a good example where the user experience differs completely from the technical one. A user doesn't care at all if a state update was retrieved for a device/light. All they see is "why the heck did my command not work ?"

Created an issue for this in the architecture repo: https://github.com/home-assistant/architecture/discussions/740

marcelveldt avatar Feb 24 '22 09:02 marcelveldt

10 seconds is too long to "lie". We should explore some indicator that it's pending.

The pending indicator is even better approach.

The Simple Thermostat card shows this in a nice way: when changing the setpoint the color of the setpoint numbers turn red. Once the same value comes back from the climate entity then the color of the numbers goes back to normal.

bouwew avatar Mar 09 '22 08:03 bouwew

BTW: Do we have any idea which integrations actually have this issue ? I'm aware of Z-wave but are there any others that are slow with their state reporting ? I quickly browsed some integrations this week and I spotted some that actually report the set state without actually relying on the actual state from the device.

The Plugwise integration has this issue after fully implementing use of the DUC, see https://github.com/home-assistant/core/issues/67687

bouwew avatar Mar 09 '22 08:03 bouwew

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. Thank you for your contributions.

github-actions[bot] avatar Jun 07 '22 09:06 github-actions[bot]

Gonna close this, there is currently a architecture discussion going to fix this on a larger scale.

bramkragten avatar Jun 21 '23 09:06 bramkragten