fix Philips Hue and clean up code #2109
Conversation
Philips Hue was not working, or sometimes would only work for a single light. This refactors the update cycle to correctly handle Philips Hue API calls. Tested with 5 lights and a variety of colors/effects, played for 7 hours and worked well (as well as Hue can, that is - it certainly isn't DMX). Cleaned up old code and debugging comments/logs.
| auto resp = setLights.request("PUT", APIEndpoint, postData); | ||
| if (resp.status == 200) // OK | ||
| { | ||
| lights[n].dirty = false; |
There was a problem hiding this comment.
By moving this here, and removing the lock, you created a race condition, which is likely why you needed the 2nd dirty check construct.
|
|
||
| if (lights[n].dirty) | ||
| { | ||
| sp::io::http::Request setLights(ip_address, port); |
There was a problem hiding this comment.
tabs vs spaces. Please use 4 spaces instead of tabs.
|
|
||
| if (lights[n].dirty) | ||
| { | ||
| sp::io::http::Request setLights(ip_address, port); |
There was a problem hiding this comment.
By re-creating the sp::io::http::Request for each call, it makes a new tcp connection per call. If you reuse the same object it should keep a single connection open, which is better for performance.
Did you run into some kind of bug why you needed to move this?
There was a problem hiding this comment.
Yes, that is actually what sent me down this whole rabbit hole... it would sometimes work for the first light, but not additional lights. Wireshark packet captures showed that after the first light call, it would malform all the subsequent calls, putting the json payload before the api call. Like:
{"on":true, "sat":254, "bri":254,"hue":19660, "transitiontime": 1}put /api//lights/3/state
instead of (when Wireshark capturing Postman):
PUT /api//lights/2/state HTTP/1.1
Content-Type: application/json
User-Agent: PostmanRuntime/7.38.0
Accept: /
Postman-Token: 7780f271-cf4f-40c3-8bd4-220a074b80ba
Host: 192.168.1.104
Accept-Encoding: gzip, deflate, br
Connection: keep-alive
Content-Length: 45
{"on": true, "bri": 254, "transitiontime": 1}
So I pulled it out to make a separate connection each time and that did the trick, no more malformed API packets after that. I'm not sure why that is the case, I was on a bit of a time crunch when I was working on it (had a game scheduled) so I was certainly in "just make it work" mode.
I don't know how many people even use the Hue feature, I just happen to already have them in my game space so might as well. I'll tinker with and clean it up some more but I can't promise any kind of timeline for it, so feel free to reject for now if you'd like and I can open a new PR if I make it any better.
Philips Hue was not working, or sometimes would only work for a single light. This refactors the update cycle to correctly handle Philips Hue API calls. Tested with 5 lights and a variety of colors/effects, played for 7 hours and worked well (as well as Hue can, that is - it certainly isn't DMX). Cleaned up old code and debugging comments/logs.
Separated from my voice fix PR so daid can decide whether to include it or not.