hardillb / TRADFRI2MQTT

MQTT Bridge for IKEA TRÅDFRI Light Gateway
https://www.hardill.me.uk/wordpress/2017/04/06/fist-pass-tradfri-mqtt-bridge/
Apache License 2.0
81 stars 12 forks source link

Fix silent failure when socket name doesn't contain at least 2 spaces #9

Closed bonan closed 7 years ago

bonan commented 7 years ago

Philips Hue lights identify their model with a simple string like LWB004 or LWB010, this causes the code

socket = " " + json.getJSONObject("3").getString("1").split(" ")[2];

to throw a silent ArrayIndexOutOfBoundsException

JSON response for my Hue light:

{
  "9001":"Living room",
  "9002":1498931354,
  "9020":1499042697,
  "9003":65548,
  "9054":0,
  "5750":2,
  "9019":1,
  "3": {"0":"Philips", "1":"LWB004", "2":"", "3":"5.38.2.19136", "6":1},
  "3311":[{"5850":0,"5851":153,"9003":0}]
}

The patch modifies the above line to first check if the model starts with "TRADFRI bulb" before doing a split

r41d commented 7 years ago

Interesting, how did you manage to pair a hue bulb to the Tradfri bridge? When I implemented this line my compiler didn't warn me about this exception, that's pretty much why I forgot to check it :p But I think a better approach than checking with startswith() would be to catch the ArrayIndexOutOfBoundsException.

daenney commented 7 years ago

You can pair a Hue bulb with a trådfri gateway in the same way you pair IKEA bulbs, with the remote. Just hold the remote up to 5cm away from a Hue bulb and follow the regular procedure. Bulb will start blinking and stop blinking once it's paired. You can then control it through the trådfri gateway. Though if you have the RGB bulbs the app and gateway have no way of controlling that and you won't be able to get updates for your bulb without the Hue gateway either.

As an aside, is what type of socket the bulb is useful information at all?

r41d commented 7 years ago

I think this patch doesn't work as intended, because socket.startsWith(" TRADFRI bulb ") will never be true because of the leading space. I would advise to check for a ArrayIndexOutOfBoundsException, like I suggested earlier.

hardillb commented 7 years ago

I've taken the leading space out of the test for now, I'll look at a better fix a little later in the week