peter-murray / node-hue-api

Node.js Library for interacting with the Philips Hue Bridge and Lights
Apache License 2.0
1.19k stars 145 forks source link

Api Error: The group id '17' is not valid for this Hue Bridge. #123

Closed jens-maus closed 5 years ago

jens-maus commented 5 years ago

I noticed the following error when using your node-hue-api module:

Api Error: The group id '17' is not valid for this Hue Bridge.
    at _errorPromise (/home/damato/node_modules/node-hue-api/hue-api/index.js:1509:21)
    at _setGroupIdOption (/home/damato/node_modules/node-hue-api/hue-api/index.js:1375:24)
    at HueApi.getGroup (/home/damato/node_modules/node-hue-api/hue-api/index.js:520:15)
    at Object.<anonymous> (/home/damato/projekte/linux/ioBroker.hue/test.js:17:5)
    at Module._compile (module.js:653:30)
    at Object.Module._extensions..js (module.js:664:10)
    at Module.load (module.js:566:32)
    at tryModuleLoad (module.js:506:12)
    at Function.Module._load (module.js:498:3)
    at Function.Module.runMain (module.js:694:10)

The code in question causing this error is:

var HueApi = require("node-hue-api").HueApi;

var displayResult = function(result) {
    console.log(JSON.stringify(result, null, 2));
};

var host = "192.168.5.28",
    username = "XXXXX",
    api;

api = new HueApi(host, username);

api.groups()
    .then(displayResult)
    .done();

api.getGroup(17).then(displayResult).done();

Please note that the api.groups() call actually returns a group with id 17:

  {
    "id": "17",
    "name": "discoAGroupEven",
    "lights": [
      "12",
      "13"
    ],
    "sensors": [],
    "type": "LightGroup",
    "state": {
      "all_on": false,
      "any_on": false
    },
    "recycle": false,
    "action": {
      "on": false,
      "bri": 254,
      "hue": 13524,
      "sat": 200,
      "effect": "none",
      "xy": [
        0.5017,
        0.4152
      ],
      "ct": 443,
      "alert": "none",
      "colormode": "xy"
    }
  },

In addition, directly using 17 in e.g. curl also correctly outputs information on group 17:

$ curl http://192.168.5.28/api/XXXXXXX/groups/17/
{"name":"discoAGroupEven","lights":["12","13"],"sensors":[],"type":"LightGroup","state":{"all_on":false,"any_on":false},"recycle":false,"action":{"on":false,"bri":254,"hue":13524,"sat":200,"effect":"none","xy":[0.5017,0.4152],"ct":443,"alert":"none","colormode":"xy"}}

To me this looks like a somewhat border-case bug in node-hue-api, isn't it?

jens-maus commented 5 years ago

After some more investigation I found the following code passage to be the actual reason for the getGroup(17) to fail:

https://github.com/peter-murray/node-hue-api/blob/master/hue-api/index.js#L1539

It seems you added some hardcoded maximum number for the group number which as it seems not be valid anymore.

peter-murray commented 5 years ago

This is related to a change in the bridge software version 1.8:

as of 1.8 the maximum amount of groups is increased from 16 to 64.
peter-murray commented 5 years ago

Version 1.8 was released 01/05/2017, so it is probably fairly safe to make the change to 64 now, I will fix this over the weekend and release an update.

jens-maus commented 5 years ago

Why is there a check for a fixed maximum anyway? why not put a direct check in there that checks if e.g. a group with number XX exists like api.groups() might output?

jens-maus commented 5 years ago

Or simply just return the return code of the Philips API code which might anyway output an error if a certain group/light ID doesn't exist and a query is performed? Just like you can see from here:

$ curl http://192.168.5.28/api/XXXX/groups/20/
[{"error":{"type":3,"address":"/groups/20","description":"resource, /groups/20, not available"}}]
$ curl http://192.168.5.28/api/XXXX/groups/128/
[{"error":{"type":3,"address":"/groups/128","description":"resource, /groups/128, not available"}}]

So, IMHO it should be fairly save to remove the whole group/light check and just return the Philips API result (i.e. "not available").

peter-murray commented 5 years ago

The fixed maximum, is in their API itself. I don’t expose the direct errors from the API as it used to be limited and misleading on the earlier versions of it. This API library has existed from before there was an official API from Phillips. There are still edge case errors that the bridge does not return a nice of useful message to the user. These checks are also present to represent the limits that Phillips publish in their documentation.

I am finishing off a massive refactoring of this library which is more compatible with the updates from the underlying API as well as removing the now completely unnecessary Q promise library and traits library which reveals the age and origin of this library.