PhilipsHue / PhilipsHueSDK-Java-MultiPlatform-Android

The Software Development Kit for Philips Hue Java Mulfi-Platform and Android (beta)
273 stars 214 forks source link

Provide example of non-deprecated PHBridge.createGroup #26

Closed J-Swift closed 8 years ago

J-Swift commented 8 years ago

Hi, it appears the 'recommended' method of creating groups, by passing the String name and List of lightIds, is now deprecated. It appears the migration path to the alternate createGroup call is not as simple as I would have assumed. My first inclination would be to translate this old call:

bridge.createGroup( groupName, lightIdentifiers, null );

into something like this:

PHGroup group = new PHGroup( groupName, lightIdentifiers );
bridge.createGroup( group, null );

But that isn't syntactically valid since the group initializer doesn't work on the list of lightIds. It instead probably wants to be used like this:

PHGroup group = new PHGroup( groupName, {some sort of group id} );
group.setLightIdentifiers( lightIdentifiers );
bridge.createGroup( group, null );

But it's not clear to me what I should be passing as the group identifier in this case in order to initialize safely/correctly.

Any insights? Side note, are you planning on updating the javadocs? Thanks!

SteveyO commented 8 years ago

Hi,

I have just checked the code/docs and indeed you are right. This looks a bit messed up and we have a deprecated way of creating groups documented, so apologies for this. I will fix it asap.

We seem to have a bit of a flaw in the SDK as to create a PHGroup you need to pass in the GroupName and the GroupIdentifier, which of course is not known for new groups. Using a dummy value works e.g. I use 0 in below example and it is ignored, the GroupIdentifer of the new group is returned in the onCreated event in the PHGroupListener

PHGroup group = new PHGroup("My Group Name", "0");        
group.setLightIdentifiers(lightIdsString);

bridge.createGroup(group, new PHGroupListener() { . . . 

I will fix the Java SDK asap to at least provide a more sensible constructor for creating Groups, but if you want to use a dummy group identifier for now then this works fine.

Regarding the JavaDocs have you seen anything wrong (apart from this issue?). Am pretty sure I updated these recently.

Steve

SteveyO commented 8 years ago

Update. I will release a new Java SDK version early next week. I have done the change and it works fine, just needs to pass the usual quality checks.
In the next version there will be an empty PHGroup constructor which you can instantiate and add your Light Ids and Group Name. Alternatively you can keep using "0" as described above, up to you. It works exactly the same, but obviously messier. I will update the documentation as soon as the new SDK is released. Steve

J-Swift commented 8 years ago

Thanks Steve. I'll use your workaround for now and keep an eye out for the updated SDK. Appreciate the quick turnaround.

SteveyO commented 8 years ago

Just released 1.10.1 Java SDK. Hopefully this will do the trick. Documentation updated also. Thanks again for pointing this out, seemed to have been overlooked. Steve