falk0069 / hue-upnp

Philips Hue UPNP and HTTP emulator (works with the Android Hue app and Logitech Harmony Home Remotes)
30 stars 10 forks source link

hue-upnp issuing multiple DON commands when dimming #8

Open ggiesen opened 8 years ago

ggiesen commented 8 years ago

When my brightening/dimming my lights with my Harmony Elite, hue-upnp issues two DON commands for each brighten/dim command. First a DON with no brightness parameter, then a DON with the brightness parameter. This causes the light to bounce between 100% (the on level) and the requested brightness level when brightening/dimming.

See log (this is the result of pressing the brighten key once):

2016-01-04 03:28:08,234 [DEBUG] device number:0
2016-01-04 03:28:08,234 [DEBUG] In set method. Data={u'on': True, u'bri': 254}
2016-01-04 03:28:08,235 [INFO ] ISY REST: http://192.168.2.148/rest/nodes/38 26 38 1/cmd/DON
2016-01-04 03:28:08,243 [INFO ] Starting new HTTP connection (1): 192.168.2.148
2016-01-04 03:28:08,259 [DEBUG] "GET /rest/nodes/38%2026%2038%201/cmd/DON HTTP/1.1" 200 104
2016-01-04 03:28:08,265 [INFO ] ISY REST: http://192.168.2.148/rest/nodes/38 26 38 1/cmd/DON/254
2016-01-04 03:28:08,274 [INFO ] Starting new HTTP connection (1): 192.168.2.148
2016-01-04 03:28:09,040 [DEBUG] "GET /rest/nodes/38%2026%2038%201/cmd/DON/254 HTTP/1.1" 200 104
2016-01-04 03:28:09,046 [DEBUG] hueUpnp: 192.168.1.61 Sent HTTP Put Response:
HTTP/1.1 200 OK
CONTENT-LENGTH: 43
CONTENT-TYPE: text/xml charset="utf-8"
DATE: Mon, 04 Jan 2016 08:28:09 GMT
EXT:
SERVER: Unspecified, UPnP/1.0, Unspecified
CONNECTION: close
falk0069 commented 8 years ago

Hmm, I seem to recall there being a reason for that. I'll take a closer look later this week. -Andy On Jan 4, 2016 2:36 AM, "ggiesen" notifications@github.com wrote:

When my brightening/dimming my lights with my Harmony Elite, hue-upnp issues two DON commands for each brighten/dim command First a DON with no brightness parameter, then a DON with the brightness parameter This causes the light to bounce between 100% (the on level) and the requested brightness level with brightening/dimming

See log (this is the result of pressing the brighten key once):

2016-01-04 03:28:08,234 [DEBUG] device number:0 2016-01-04 03:28:08,234 [DEBUG] In set method Data={u'on': True, u'bri': 254} 2016-01-04 03:28:08,235 [INFO ] ISY REST: http://1921682148/rest/nodes/38 26 38 1/cmd/DON 2016-01-04 03:28:08,243 [INFO ] Starting new HTTP connection (1): 1921681148 2016-01-04 03:28:08,259 [DEBUG] "GET /rest/nodes/38%2026%2038%201/cmd/DON HTTP/11" 200 104 2016-01-04 03:28:08,265 [INFO ] ISY REST: http://1921682148/rest/nodes/38 26 38 1/cmd/DON/254 2016-01-04 03:28:08,274 [INFO ] Starting new HTTP connection (1): 1921682148 2016-01-04 03:28:09,040 [DEBUG] "GET /rest/nodes/38%2026%2038%201/cmd/DON/254 HTTP/11" 200 104 2016-01-04 03:28:09,046 [DEBUG] hueUpnp: 192168161 Sent HTTP Put Response: HTTP/11 200 OK CONTENT-LENGTH: 43 CONTENT-TYPE: text/xml charset="utf-8" DATE: Mon, 04 Jan 2016 08:28:09 GMT EXT: SERVER: Unspecified, UPnP/10, Unspecified CONNECTION: close

— Reply to this email directly or view it on GitHub https://github.com/falk0069/hue-upnp/issues/8.

jimboca commented 8 years ago

I was about to add this same issue since I just upgraded my fork that is used for ISYHelper to the latest.

jimboca commented 8 years ago

I think the Harmony remembers the last brightness it set on the device so when you press on, it passes on with the last bri. So I think if we see bri, we should just set bri and assume that will also turn the device on?

ggiesen commented 8 years ago

On my Harmony, I confirmed when you turn the light on, it sets the brightness parameter to 0 (which hue-upnp interprets to send a simple DON with no brightness parameter to the ISY. I have my Elite buttons mapped as "Short Press - Turn on/select; Long Press - Turn off" for the actual light button, and am using the rocker switch (+/-) for dimmer control.

2016-01-04 11:13:08,540 [INFO ] hueUpnp: 192.168.1.61: reading http request
2016-01-04 11:13:08,546 [DEBUG] hueUpnp: Header-Length=174 Content-Length=19
2016-01-04 11:13:08,551 [DEBUG] hueUpnp: 192.168.1.61: HTTP Request: PUT /api/lights/1/state HTTP/1.1
connection: close, TE
content-length: 19
user-agent: LuaSocket 2.0.2
te: trailers
content-type: application/json
host: 192.168.1.34

{"on":true,"bri":0}
2016-01-04 11:13:08,552 [DEBUG] hueUpnp: 192.168.1.61 Got PUT request to do something
2016-01-04 11:13:08,552 [DEBUG] hueUpnp: 192.168.1.61 Content data=---
{"on":true,"bri":0}
---
2016-01-04 11:13:08,553 [DEBUG] hueUpnp: 192.168.1.61 Parsed Content data=---
{u'on': True, u'bri': 0}
---
2016-01-04 11:13:08,554 [DEBUG] device number:0
2016-01-04 11:13:08,555 [DEBUG] In set method. Data={u'on': True, u'bri': 0}
2016-01-04 11:13:08,555 [INFO ] ISY REST: http://192.168.2.148/rest/nodes/38 26 38 1/cmd/DON
2016-01-04 11:13:08,564 [INFO ] Starting new HTTP connection (1): 192.168.2.148
2016-01-04 11:13:08,581 [DEBUG] "GET /rest/nodes/38%2026%2038%201/cmd/DON HTTP/1.1" 200 104
2016-01-04 11:13:08,587 [DEBUG] hueUpnp: 192.168.1.61 Sent HTTP Put Response:

HTTP/1.1 200 OK CONTENT-LENGTH: 43 CONTENT-TYPE: text/xml charset="utf-8" DATE: Mon, 04 Jan 2016 16:13:08 GMT EXT: SERVER: Unspecified, UPnP/1.0, Unspecified CONNECTION: close

[{"success":{"/lights/1/state/on":true}}]

2016-01-04 11:13:08,588 [DEBUG] hueUpnp: -------------------------------
2016-01-04 11:13:08,589 [DEBUG] hueUpnp:
ggiesen commented 8 years ago

It also seems to have the same behaviour when you change the color/color temperature through the Harmony Android app (multiple DONs) (yes I realize Insteon doesn't support this, just happened to stumble upon it):

2016-01-04 11:23:37,502 [INFO ] hueUpnp: 192.168.1.61: reading http request
2016-01-04 11:23:37,508 [DEBUG] hueUpnp: Header-Length=174 Content-Length=30
2016-01-04 11:23:37,511 [DEBUG] hueUpnp: 192.168.1.61: HTTP Request: PUT /api/lights/1/state HTTP/1.1
connection: close, TE
content-length: 30
user-agent: LuaSocket 2.0.2
te: trailers
content-type: application/json
host: 192.168.1.34

{"on":true,"ct":291,"bri":194}
2016-01-04 11:23:37,512 [DEBUG] hueUpnp: 192.168.1.61 Got PUT request to do something
2016-01-04 11:23:37,512 [DEBUG] hueUpnp: 192.168.1.61 Content data=---
{"on":true,"ct":291,"bri":194}
---
2016-01-04 11:23:37,513 [DEBUG] hueUpnp: 192.168.1.61 Parsed Content data=---
{u'on': True, u'bri': 194, u'ct': 291}
---
2016-01-04 11:23:37,514 [DEBUG] device number:0
2016-01-04 11:23:37,515 [DEBUG] In set method. Data={u'on': True, u'bri': 194, u'ct': 291}
2016-01-04 11:23:37,515 [INFO ] ISY REST: http://192.168.2.148/rest/nodes/38 26 38 1/cmd/DON
2016-01-04 11:23:37,524 [INFO ] Starting new HTTP connection (1): 192.168.2.148
2016-01-04 11:23:37,541 [DEBUG] "GET /rest/nodes/38%2026%2038%201/cmd/DON HTTP/1.1" 200 104
2016-01-04 11:23:37,548 [INFO ] ISY REST: http://192.168.2.148/rest/nodes/38 26 38 1/cmd/DON/194
2016-01-04 11:23:37,557 [INFO ] Starting new HTTP connection (1): 192.168.2.148
2016-01-04 11:23:38,352 [DEBUG] "GET /rest/nodes/38%2026%2038%201/cmd/DON/194 HTTP/1.1" 200 104
2016-01-04 11:23:38,358 [DEBUG] hueUpnp: 192.168.1.61 Sent HTTP Put Response:
HTTP/1.1 200 OK
CONTENT-LENGTH: 43
CONTENT-TYPE: text/xml charset="utf-8"
DATE: Mon, 04 Jan 2016 16:23:38 GMT
EXT:
SERVER: Unspecified, UPnP/1.0, Unspecified
CONNECTION: close

[{"success":{"/lights/1/state/on":true}}]

2016-01-04 11:23:38,359 [DEBUG] hueUpnp: -------------------------------
2016-01-04 11:23:38,360 [DEBUG] hueUpnp:
jimboca commented 8 years ago

What if you change the brightness of the light all the way up using the rocker, then turn it off and back on? Does that last on send on and 254?

ggiesen commented 8 years ago

Yes.

Here's the log:

2016-01-04 20:04:26,302 [INFO ] hueUpnp: 192.168.1.61: reading http request
2016-01-04 20:04:26,309 [DEBUG] hueUpnp: Header-Length=174 Content-Length=21
2016-01-04 20:04:26,312 [DEBUG] hueUpnp: 192.168.1.61: HTTP Request: PUT /api/lights/1/state HTTP/1.1
connection: close, TE
content-length: 21
user-agent: LuaSocket 2.0.2
te: trailers
content-type: application/json
host: 192.168.1.34

{"on":true,"bri":254}
2016-01-04 20:04:26,313 [DEBUG] hueUpnp: 192.168.1.61 Got PUT request to do something
2016-01-04 20:04:26,313 [DEBUG] hueUpnp: 192.168.1.61 Content data=---
{"on":true,"bri":254}
---
2016-01-04 20:04:26,314 [DEBUG] hueUpnp: 192.168.1.61 Parsed Content data=---
{u'on': True, u'bri': 254}
---
2016-01-04 20:04:26,315 [DEBUG] device number:0
2016-01-04 20:04:26,316 [DEBUG] In set method. Data={u'on': True, u'bri': 254}
2016-01-04 20:04:26,317 [INFO ] ISY REST: http://192.168.2.148/rest/nodes/38 26 38 1/cmd/DON
2016-01-04 20:04:26,326 [INFO ] Starting new HTTP connection (1): 192.168.2.148
2016-01-04 20:04:26,343 [DEBUG] "GET /rest/nodes/38%2026%2038%201/cmd/DON HTTP/1.1" 200 104
2016-01-04 20:04:26,349 [INFO ] ISY REST: http://192.168.2.148/rest/nodes/38 26 38 1/cmd/DON/254
2016-01-04 20:04:26,358 [INFO ] Starting new HTTP connection (1): 192.168.2.148
2016-01-04 20:04:27,125 [DEBUG] "GET /rest/nodes/38%2026%2038%201/cmd/DON/254 HTTP/1.1" 200 104
2016-01-04 20:04:27,131 [DEBUG] hueUpnp: 192.168.1.61 Sent HTTP Put Response:
HTTP/1.1 200 OK
CONTENT-LENGTH: 43
CONTENT-TYPE: text/xml charset="utf-8"
DATE: Tue, 05 Jan 2016 01:04:27 GMT
EXT:
SERVER: Unspecified, UPnP/1.0, Unspecified
CONNECTION: close

[{"success":{"/lights/1/state/on":true}}]

2016-01-04 20:04:27,131 [DEBUG] hueUpnp: -------------------------------
2016-01-04 20:04:27,132 [DEBUG] hueUpnp:
jimboca commented 8 years ago

Thanks, I also noticed that every time you hit the rocker it sets the light to full on, then changes the brightness. I think we need to change the super class to just call the bri method when bri is passed in, and the sub class can decide how to handle it.

falk0069 commented 8 years ago

I'm not 100% following. The main thing for me is if 'on' is in the data and the light's state is currently 'off', I want to make sure the 'on' directive is issued. I currently haven't had a need for the brightness functions yet.

Jim it might be better if you make the needed changes and I'll merge it in. Otherwise I can take a stab at it later this week.

Thanks

jimboca commented 8 years ago

OK I will try when I can, problem is my only harmony home control remote is in our master bedroom, so I can only debug when my wife is awake. :-)

The problem is that ISY devices do not want the on passed, setting bri implies on.

ggiesen commented 8 years ago

Just for clarity, "GET /rest/nodes/38%2026%2038%201/cmd/DON/254" implies turning the device on. There's no need for a second DON command without the brightness parameter.

From: http://wiki.universal-devices.com/index.php?title=ISY_Developers:API:REST_Interface#Commands

Insteon - /rest/nodes//cmd/DON/128 - turn on a scene to 50% (valid parameters = 0 - 255)

Alternatively, I would actually probably make the rocker just issue a BRT/DIM command, since at least on the Harmony you have to select the device before brightnening/dimming it, which turns it on anyways. That may not be possible depending on what you get from the Harmony, though. Either way, the second DON is redundant and makes dimming almost unusable.

jimboca commented 8 years ago

Yes, that's what I was trying to say. I just pushed the merge request with the fix, although it's part of the existing merge request to remove all globals. Sorry, I should have created a new branch but did not have time.

falk0069 commented 8 years ago

I just pulled in Jim's latest code. Things have been working great so far for me. I have yet to try the bind fix I suggested in the other issue, so that isn't fixed yet.

ggiesen commented 8 years ago

I can confirm this definitely works now. Would prefer if I hit the light on button that it did a full on (according to the on level defined in ISY) but I can live with the current behaviour. I'm testing the fix for issue 9 right now but initial indications look like it's not working yet

jimboca commented 8 years ago

Good to hear, thanks for confirming. I think the harmony always sends the last brightness level it had when sending the on. But I think I agree that I would prefer full on when on is pressed. I'll have to review the logs again to see if we can detect a difference.