MindrustUK / Heatmiser-for-home-assistant

Heatmiser Neo-Hub / Neostat support for home-assistant.io
87 stars 33 forks source link

Improved user experience of Away mode #207

Closed ocrease closed 1 week ago

ocrease commented 3 weeks ago

This PR addresses #206 I haven't made changes for the plugs because I'm not sure how exactly they are supposed to behave. For timers, its like this:

Plugs don't have standby, instead they have a manual mode where they can be set to on or off. @MindrustUK would you be able to check on the heatmiser app what you are allowed to do on a plug when the hub is away/holiday? Are you still able to switch it on manually? Then I can complete the changes to include the plug as well.

There is a small bug if you try to select Override On/Override Off on a timer if the state is away/holiday. It is not meant to do anything and it doesn't, but the dropdown selector stays in the newly selected state. The state hasn't actually changed and refreshing the screen sets it to the expected value (eg away), but even an update from hub isn't enough to change back on its own. Ideally it would switch back on its own immediately

MindrustUK commented 2 weeks ago

@ocrease, as requested: If the Hub is on 'Away/Holiday' The Neoplug seems to just behave as 'normal'. No changes I can see.

MindrustUK commented 2 weeks ago

Hi @ocrease Looks like this PR picked up a conflict, did you also need to do anything further with regards to the neoplugs?

ocrease commented 2 weeks ago

@MindrustUK I'll resolve the conflict, but it's not ready for merge yet. I do have more questions about the NeoPlugs, but I need time to formulate them. Leave it with me for now.

ocrease commented 2 weeks ago

@MindrustUK as far as I can tell, plugs can be in 5 but possibly 6 states:

Now, when it comes to setting the Hub away/holiday, I would expect that if the plug is in Auto, it won't turn on if the profile says to turn on at a particular time.

Can you do the following tests from the heatmiser app:

If you can add some screenshots of the app and some diagnostics output, that would be great too!

I have a bit of a dilemma as to whether to include Away for timers and plugs (I am leaning towards yes). If they are affected by the away status of the hub (timers definitely are), then it makes sense to include Away in the mode selection dropdown. However, we need to be aware of the following:

I think we can handle this with a note in the readme and it matches the behaviour of the heatmiser app reasonably closely. If users don't like the away mode behaviour, they can always use the standby/off mode on individual devices instead.

The alternative is to not include Away for timers/plugs. But then you can be in a situation where the mode selector says Auto (for example), but the device is actually away so it won't come on when you would expect. This feels worse than the option above.

Do you have any other thoughts?

ocrease commented 2 weeks ago

@MindrustUK I found a few bugs so I have pushed an update

ocrease commented 1 week ago

@MindrustUK I'll fix the merge conflicts in the next few days.

MindrustUK commented 1 week ago

@ocrease all good, in your own time etc. Apologies it's taken me a bit longer to get to catching up.

ocrease commented 1 week ago

@MindrustUK I fixed the merge conflicts

MindrustUK commented 1 week ago

@ocrease soon as I get a chance to test, I'll feedback on this one, all looks good. As requested I'll test and get you screenshots etc.

MindrustUK commented 1 week ago

@ocrease as requested:

I think Away / Holiday needs to be a hub level function and it's sufficient to include it at a readme level, reflecting that the behavior matches the Heatmsier app.

A major stand out to me is making the away duration configurable, maybe a case for another service?

I note that Away is very much global and even the official Heatmiser app now includes options to standby specific timers.

Maybe having a synthetic condition in the Home Assistant drop down boxes is the way to go? IE "System in Away mode" and locking the selections or disabling the drop downs in general if away is active? This should go some way to alerting any unsuspecting users that their conditions will not be respected if in away / holiday mode?

ocrease commented 1 week ago

@MindrustUK I'll add a numbered list so we know what we are referring to in each other's reply.

  1. I'm little confused by your test results. You mention a NeoStat V1 timer, but I would like to see the results for a NeoPlug. You also say "Unaffected / No state change." but also say the "output turned off".
  2. Regarding the away duration, you are referring to the holiday mode rather than the away mode (which is basically permanently away or a holiday without an end date). I agree that setting a holiday period will need to be implemented as a service, but that will be a later PR.
  3. You say "the official Heatmiser app now includes options to standby specific timers." The way I understand it, Away/Holiday mode is intended to affect all devices, but standby affects individual devices. The heatmiser app does indeed let you set multiple devices to away in one go. I believe setting individual devices to away is possible, but its not recommended in the heatmiser developer docs, so I don't think its something we should support. Users can easily use standby instead.
  4. I tried populating the options on the select dropdown dynamically, but the experience wasn't great. I think instead we should display the full set of options (which can be different by device type, eg plugs vs timers) and allow users to change to any option. The behaviour would need to be documented in the readme (or maybe its time to introduce docs in github pages?)
    • If you are on standby and you switch to another option, you disable standby
    • If you are away and switch to another option, you disable away/holiday mode which applies to all devices (depending on your findings from point 1 regarding NeoPlugs, maybe a transition from Away to Manual ON would be allowed without taking all devices out of Away)
    • If you switch to away, all devices go to away
    • The purpose of Away should be to turn off all devices because you are actually away from home and you want everything off. If a user wants more fine grained control, they should use standby instead. Note that the dev branch already behaves this way for all climate/thermostat entities. This PR changes it to also report the correct state for Timers/Plugs
MindrustUK commented 1 week ago

@ocrease

  1. Sorry, I'm creating confusion; I had a NeoTimer attached to make sure the system as a whole was behaving as expected when going "On Holiday" or "Permanently Away". The notes recorded with "Unaffected / No state change." are specifically against the Heatmiser Neo Plug device when in "Holiday" or "Permanently Away" mode. If the Heatmiser NeoPlug has been manually toggled it will stay in that state. All other devices seem to shift into "Standby".

  2. As you've described.

  3. Yes as you've described. I was merely pointing out that the Heatmiser App now reflects this behavior (The app is still a nightmare, at least on android).

  4. Docs in Github Pages, this is a new one on me? I'm assuming: https://docs.github.com/en/pages ? If so I'll do some reading.

While I'm in agreement that having a consistent approach is good. It doesn't feel like Away belongs as a drop down options. I see the reasons for your argument but it feels hacky, as I don't have a better solution for now I'll go with it.

Away really should be a hub level / system wide switch rather than having all devices reflect that state. Although contradicting myself somewhat, I see when Away is set in the Heatmiser app all devices shift to "Standby" so maybe in point of fact it's actually "better than factory" to borrow a turn of phrase.

MindrustUK commented 1 week ago

Additional, Neoplugs in any state, even auto, do not seem to be affected by the away mode.

ocrease commented 1 week ago

@MindrustUK I have removed away mode from the NeoPlugs. It seems odd that setting away mode on the app doesn't affect plugs that are following a profile, but I guess we can deal with it if someone raises an issue in the future.

Regarding away mode in the drop down, here is a gif of it in action

AwayMode

I don't think I have anything else to add here, so if you are happy with it you can go ahead and merge

ocrease commented 1 week ago

Regarding github pages, I started playing with Just the Docs. You can take a look at what I've done in the Docs branch of my fork, and the generated output here

The content is just copied from the readme, but you get an idea of what can be done. I won't have time to look at that further for the next couple of weeks.

MindrustUK commented 1 week ago

Looks good, Merging. Let's pick up docs work under as separate enhancement ticket but from your demo the output looks most impressive.