Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using dimmable light (LWB006) will cause JSON parse error #166

Closed
nojunpark opened this issue Mar 8, 2016 · 4 comments
Closed

Using dimmable light (LWB006) will cause JSON parse error #166

nojunpark opened this issue Mar 8, 2016 · 4 comments

Comments

@nojunpark
Copy link

Following is the JSON info of the dimmable light sent from the philips hub:
"1":{
"state": {
"on":false,
"bri":0,
"alert":"none",
"reachable":false
},
"type": "Dimmable light",
"name": "Hue white lamp 1",
"modelid": "LWB006",
"manufacturername": "Philips",
"uniqueid":"00:17:88:01:10:34:95:dd-0b",
"swversion": "",
"pointsymbol": {
"1":"none", "2":"none", "3":"none", "4":"none", "5":"none", "6":"none", "7":"none", "8":"none"
}

And this is the JSON info of a color light:
"1": {
"state": {
"on": true,
"bri": 254,
"hue": 14910,
"sat": 144,
"effect": "none",
"xy": [
0.4596,
0.4105
],
"ct": 370,
"alert": "none",
"colormode": "ct",
"reachable": false
},
"type": "Extended color light",
"name": "Hue color lamp 1",
"modelid": "LCT007",
"manufacturername": "Philips",
"uniqueid": "00:17:88:01:10:41:87:79-0b",
"swversion": "66014919"
}

because the dimmable light is missing some fields compared to the color light, it will cause foxbox to have a JSON parse error since it appears that there is only one SettingsLightState in src/adapters/philips_hue/hub_api/structs.rs

@cr
Copy link
Contributor

cr commented Mar 9, 2016

I was expecting something like this to happen at some point. Unfortunately it's hard to work around reliably, because we don't have adaptive JSON parsing in Rust at this point. You either have to know the data returned beforehand, breaking parsing on every unexpected field – be it by discovering new hardware or by firmware update –, or you have to employ dynamic parsing on every access. That would require turning all the easily-maintable structs into much harder to grasp and maintain parsers.

Not sure yet if there's a good middle ground. For now I'll add the fields as optional, but that process will have to continue for a while. Perhaps white-listing of supported types would be a viable mid-term solution.

@nojunpark
Copy link
Author

Just a thought, since the "modelid" field would be present in all types of lightbulb, can we get the modelid value first, and determine which field values to collect based on it? like, of it is LCT007, then we seek for the color values, and if it LWB006, then we seek for dimmer values, and so on. Probably need a separate JSON struct for each type of the bulb I suppose.

Another thing would be that foxbox should not send incorrect commands to the philips hub - for example, currently the on command also sends hue values as well, which the non-color bulbs won't process. (if they simply ignore them, then it should be okay, but the user would be confused if the UI app shows option to change colors for a white dimmer bulb) Maybe keeping track of which type of bulbs are configured could be a good way to avoid this?

but perhaps this bug is outside the scope of the upcoming demo I suppose?

cr pushed a commit to cr/foxbox that referenced this issue Mar 9, 2016
@cr
Copy link
Contributor

cr commented Mar 9, 2016

Fixed by pull request #174 which implements whitelisting of "Extended color light".

@cr cr closed this as completed Mar 9, 2016
@cr
Copy link
Contributor

cr commented Mar 9, 2016

The long term solution with support for multiple light types will be implemented for the rewrite on top of the new taxonomy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants