nebulous / infinitude

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

Fix failure to return correct response payload to t-stat #61

Closed sbrown4 closed 5 years ago

sbrown4 commented 5 years ago

Many of the stash values are reserved. These include: action, app, cb, controller, data, extends, format, handler, inline, json, layout, namespace, path, status, template, text and variant

Setting stash(data) interferes with the processing of render() and causes the payload in the response to be copied from the request. Consequently, POST status from the t-stat never sees a change flag and doesn't request an updated config.

The stash(data) line isn't ever referenced, so just drop it.

This seems to address issue 38.

nebulous commented 5 years ago

Thanks for the PRs. I recently moved houses so can't properly test this stuff right now on my implementation. I'm going to rely on your good will as a contributor that you've tested to ensure this isn't a Chesterton's fence problem.

sbrown4 commented 5 years ago

I've got one more P/R's coming. It fixes the response to the status post.

Changes made to either the t-stat, Bryant site or with the api seem to propagate.

I just got 3 of these systems installed and want to get things working well enough to ditch the Bryant site and just run your code.

Steve

On Thu, 2018-12-06 at 07:05 -0800, nebulous wrote:

Thanks for the PRs. I recently moved houses so can't properly test this stuff right now on my implementation. I'm going to rely on your good will as a contributor that you've tested to ensure this isn't a Chesterton's fence problem. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

sbrown4 commented 5 years ago

I've got one more P/R after the one I just posted.

The current firmware issues many posts for which there are no routes. The 404 error returns cause timeouts in the t-stat and really slow things down.

I wrote some stubs to intercept, log and return success. I'm sure there is a simpler way to do this.

I'd like to see what's involved in handling multiple systems. The t- stat id's are unique, so they could be added to the cache names. I don't have any experience with mojolicious so don't know if there are any concurrency issues. I saw your comment about running multiple copies at different ports.

Steve

On Thu, 2018-12-06 at 07:05 -0800, nebulous wrote:

Merged #61 into master. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

nebulous commented 5 years ago

The simplest way that should always work is to run infinitude on multiple ports, each with its own state directory, an environment variable for the state dir would make sense in this case. You're correct however that all of the information necessary to manage multiple stats in the same interface is available, but it might be a bit of a slog to port the code to expect multiples. For instance /systems.xml will return the systems definition for your system id. We would have to either: 1) only allow /yourid/systems.xml urls 2) designate a default system in those cases

1 is obviously better than 2, but not backwardly compatible (acceptable) and a bit of a pain in the (badly in need of upgrade) frontend.