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

Fix group of lights #4

Merged
merged 2 commits into from
Apr 22, 2018
Merged

Fix group of lights #4

merged 2 commits into from
Apr 22, 2018

Conversation

fmeies
Copy link

@fmeies fmeies commented Apr 16, 2018

A group of lights like this

Group:Switch:OR(ON,OFF) lightGroup ["Lighting"]
Color light1 (lightGroup) ["Lighting"]
Color light2 (lightGroup) ["Lighting"]

is not discovered correctly. In v2, the individual lights as well as the group of lights are discovered. In v3, only the individual lights are discovered.

I guess we have to distinguish between "endpoint" groups like e.g., Alexa.Endpoint.Thermostat or Alexa.Endpoint.Speaker and "simple" groups like a group of lights or rollershutter items.

What do you think?

@digitaldan
Copy link
Owner

Take a look at eclipse-archived/smarthome#4390 (comment) , I'm going to redo using tags for metadata instead. This should not be too different, but I wonder if we can improve the group issue, i have not had time to look at it yet. In any case we will still support v2 tags.

Also I apologize for some sloppiness in the code with things commented out and random log statements, this was my personal branch and was not anticipating others to share, but I definitely welcome the help!

@fmeies
Copy link
Author

fmeies commented Apr 18, 2018

Thank you for your feedback. I'll have a look at the referenced metadata discussion.

var num = parseInt(state);
state = '0,0,' + num;
}

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One of the changes I submitted was to use the latest openhab state opposed to the original requested state in Alexa response messages. If @digitaldan is fine with that, this change wouldn't be necessary.

c7e8a5a

@digitaldan
Copy link
Owner

Thanks! I'm going to merge this now and test, really appreciate the help.

@digitaldan digitaldan merged commit 15a85b8 into digitaldan:v3 Apr 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants