Skip to content
This repository has been archived by the owner on May 17, 2021. It is now read-only.

Query nodes for the number of groups supported. This Fixes #1452. #1459

Merged
merged 3 commits into from
Sep 26, 2014

Conversation

digitaldan
Copy link
Contributor

I would like @cdjackson to sign off on this one before it gets merged please. The zwave binding queries devices for groups until the device reports back that a group has zero max associations. Some devices like the Honeywell TH8320ZW thermostat simply do not reply at all when queried on a non existent group. This change queries the node for the number of groups first, then queries each one. I checked with the open zwave code, and while they have logic to do it both ways, afaik, only this way is actually used.

@cdjackson
Copy link
Contributor

Nice!

I’d not come across this message - I took the iteration through the groups methods from OZW - not sure why they didn’t do this!

Let’s not merge just yet though. I’d like to have a play for a day or two, and I have one comment (which I’ll comment in the code).

Cheers
Chris=

@@ -56,6 +56,9 @@

@XStreamOmitField
private AssociationGroup pendingAssociation = null;

@XStreamOmitField
Copy link
Contributor

Choose a reason for hiding this comment

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

Why omit this member from the XML?
Given that the number of associations don't change, this should I think be saved into the XML and then we could avoid doing the 'get groupings report' if the value isn't 0.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, I did that without even thinking about it, Ill omit it.

@buildhive
Copy link

openhab » openhab #1219 SUCCESS
This pull request looks good
(what's this?)

@buildhive
Copy link

openhab » openhab #1221 SUCCESS
This pull request looks good
(what's this?)

@teichsta
Copy link
Member

pls just give me sign, when this PR is ready to merge. Thanks, Thomas E.-E.

@cdjackson
Copy link
Contributor

Hey Thomas - I would say this is good to merge...

kaikreuzer added a commit that referenced this pull request Sep 26, 2014
Query nodes for the number of groups supported. This Fixes #1452.
@kaikreuzer kaikreuzer merged commit c1b21a2 into openhab:master Sep 26, 2014
johgoe pushed a commit to johgoe/openhab that referenced this pull request Jan 16, 2017
…nhab#1459)

- Fix setColor(black) action to trigger OFF command for RGB devices

Signed-off-by: Pepijn de Geus <[email protected]> (github: pdegeus)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants