nebulous / infinitude

Open control of Carrier/Bryant thermostats
MIT License
224 stars 50 forks source link

Hold API confusing and unreliable. #180

Closed electricessence closed 6 months ago

electricessence commented 9 months ago

Of course let me start by saying THANK YOU for what you've done here. It truly has made a huge impact in my quality of life.

I just have one nuanced issue that I'm struggling with. When setting the hold mode, especially when providing a "until time" (otmr), it very commonly fails. {status:"fail"}. I feel like there might be reason, but I can't seem to figure it out. I notice that I can poke at the API repeatedly in various ways to get it to not fail. Typically my last ditch attempt is to set the hold to "off" and then set it back again to "on", but I worry that's not ideal for the system.

Any help here?

nebulous commented 9 months ago

Is this the endpoint you're calling?

https://github.com/nebulous/infinitude/wiki/Infinitude-API-calls#set-hold

If you provide examples of what you're trying, what's happening, and what you expect instead, I'll see if I can reproduce.

electricessence commented 9 months ago

Yeah, I'm calling the config API. I'm using the same pathways that the Home Assistant integration is using.

electricessence commented 9 months ago

One sec. I'll get an example.

electricessence commented 9 months ago

/api/config/zones/zone/${zoneId-1} and I POST the variables to that (form encoded):

hold=on
currentActivity=manual
otmr=16:15 (or any 15 minute 24 hour value)

For the most part this end point works, but I've found that it is the least reliable when setting the otmr value. But if I was just turning on and off hold with no otmr, it would work almost all the time. But now, I'm trying to tweak my 'until' times.

electricessence commented 9 months ago

Looks like the hold API you've listed only returns XML?

nebulous commented 9 months ago

That looks reasonable, and is basically what the hold API does under the covers other than some minor conveniences like automatically rounding to the nearest 15m interval. And yes, it looks like the commit which added that feature 9 years ago for some reason returned the XML representation of the config file as the thermostat accepts it rather than the slightly-less-terrible json translation. Seems worth changing the default there, although the return value isn't particularly useful in any case. I just tested it in a quick and dirty fashion by hitting http://192.168.1.3:3000/api/1/hold.json?until=13:15 which seemed to work in my case. Are you using GET or POST? Does the hold endpoint ever fail? What's the load like on the machine running Infinitude?

electricessence commented 9 months ago

I'm using POST for anything that "modifies" a setting. I haven't been using the /api/[zoneId]/hold endpoint, but I'm going to try it.

The load is minimal. A docker container on a Synology that never gets thrashed. I was also gonna ask, do you have any guidelines as to how aggressive the infinitude API can be called? For example, here's what I will commonly do:

a) set the heatpoint, b) immediately after (no delay, effectively in parallel) set the hold mode.

Should I be adding a delay, or waiting when sending commands? What about setting hold, and then setting it again? Does the system need time to process?

electricessence commented 9 months ago

Side note, this is for a Hubitat hub driver set.

nebulous commented 9 months ago

I'm using POST for anything that "modifies" a setting.

👍 the mutating GETs drive me crazy but many legacy home automation devices require it.

I haven't been using the /api/[zoneId]/hold endpoint, but I'm going to try it.

Cool, thanks for testing - I'd be interested in seeing if there's a difference.

The load is minimal. A docker container on a Synology that never gets thrashed. I was also gonna ask, do you have any guidelines as to how aggressive the infinitude API can be called? For example, here's what I will commonly do: a) set the heatpoint, b) immediately after (no delay, effectively in parallel) set the hold mode.

I'd recommend against that for sure. Not because Infinitude can't handle it, but because Infinitude can and the thermostat can't. So a race condition could occur where Infinitude's state gets ahead of/out of sync with, then subsequently overwritten by, the thermostat's state.

Should I be adding a delay, or waiting when sending commands? What about setting hold, and then setting it again? Does the system need time to process?

Yeah - it may make sense to incorporate some queueing/state buffering/rate limiting within Infinitude such that it can quickly accept, then slowly dole out commands to the thermostat - but complexity like that could create more problems than it solves.

Rule of thumb: you can change as much as you like in any single /config call, but wait for the thermostat to update before calling again.

electricessence commented 9 months ago

Ok so first attempt using the ../hold endpoint worked well. I'll make adjustments and keep testing. Thanks for the word of warning on the potential race condition.

electricessence commented 9 months ago

I'll say again. What you've done here has REAL LIFE impact. Thank you!

electricessence commented 9 months ago

Things seem to be working well now. Thanks for the help.

electricessence commented 8 months ago

So I'm running into a strange problem. If I call the API with activity:"manual", until:"forever", but then subsequently call it again with activity:"manual", until:"10:00":

  1. The API status looks correct, with otmr: "10:00" and the thermostat itself shows the until time as "10:00 AM".
  2. But when 10 AM arrives, it simply reverts to "forever" and stays in hold mode.

If hold mode is off and I set activity:"manual", until:"10:00", it properly falls back to the schedule.

So in summary, it appears that setting the hold mode when it's already on, changes what the API sees, but doesn't actually change what the thermostat does?

Any help here would be appreciated. As a work around, it sounds like I need to set the hold to off, and then back on again?

nebulous commented 8 months ago

Interesting. From the description it seems possible that the thermostat stores its running state and reverts back to said state when the timer expires. I wonder if the thermostat UI allows this to happen as well or if it does the equivalent of what you suggest: setting hold to off before pushing a new timer state. The latter is certainly doable on the backend, but I'm not certain if it would be a bug or a feature.

electricessence commented 8 months ago

Yeah, it never screws up from the actual thermostat. That said, this is aligned with my previous observation when attempting to change the hold until without the simplified hold endpoint.

It's quite a pain to get around this, as it seems that you have to wait until the status has changed to off before trying to update the until value.

So I have to stop any updates, ping the hold value until it changes to off, then set the until value again.

electricessence commented 8 months ago

So that did work by the way:

1) look and see if the existing known state is hold=on. 2) if off, proceed as normal regardless of the hold update. 3) else if on and the known until does not match the one requested, continue on to 4: 4) slow down any updates to prevent the state from getting out of sync 5) post [hold:off] 6) ping the /api/status/:zoneId/hold value until it changes to off. 7) once off, proceed as normal and await updates on at a normal schedule after a delay.

electricessence commented 8 months ago

Lemme know your thoughts here. Do you think it's a bug or a feature? Is there a better way?

github-actions[bot] commented 6 months ago

This issue has not been active in a while. It will be automatically closed soon absent further activity.