openhab / org.openhab.binding.zwave

openHAB binding for Z-Wave
Eclipse Public License 2.0
170 stars 202 forks source link

Addon suggestion finder xml #1908

Closed andrewfg closed 6 months ago

andrewfg commented 6 months ago

This PR adds the suggestion finder xml to the addon.xml file in anticipation of the PR in openhab-core being merged.

Signed-off-by: Andrew Fiddian-Green software@whitebear.ch

andrewfg commented 6 months ago

@mherwege for info

andrewfg commented 6 months ago

Note: since https://github.com/openhab/openhab-core/pull/3922 has been merged, this one is now ready for review.

cdjackson commented 6 months ago

So I see that the same products are referenced in both the zigbee and zwave. I've not tried to work out what these products are so it's hard to know if this is an error, uncertainty, or what... I wonder if we shouldn't document this somewhere so that we know what these product IDs relate to.

andrewfg commented 6 months ago

the same products are referenced in both the zigbee and zwave

Yes these are dual function sticks that support both Z-Wave and ZigBee.

For documentation see these

cdjackson commented 6 months ago

Ok, I guess this is the hzusb1...

I'd suggest we should document this somewhere it won't get lost. I think the PR is not a good place - I'd suggest either the XML, or possibly the readme. It will otherwise be a nightmare to work out what all these are later as it's rather verbose (by the nature of how it's been implemented)

andrewfg commented 6 months ago

The two lists of chipId’s are as follows. The 10C4:8A2A is the dual function one common to both.

<regex>0403:8A28|0451:16A8|10C4:89FB|10C4:8B34|10C4:8A2A|1CF1:0030</regex>
<regex>0658:0200|10C4:8A2A|1A86:55D4</regex>

I am happy to add documentation. Would you prefer it to be in the read me, or the finder Java doc, or as you say the xml?

cdjackson commented 6 months ago

I don't think it should be in Javadoc...

Personally, I'd put it directly in the XML since then it's right next to the definitions of the devices, so it's easy to work out what is what.

The other option would be to add a section on discovery in the readme and provide a table there if someone doesn't like to add comments in the XML.

andrewfg commented 6 months ago

I already wrote a chapter in the docs (see below) which I can extend..

https://next.openhab.org/docs/developer/addons/addon.html

cdjackson commented 6 months ago

My point wasn't so much around the functionality - but somewhere that documents what 0658:0200|10C4:8A2A|1A86:55D4 means - ie exactly what devices this covers. Otherwise when someone asks "does this work for XYZ device, each time we have to do a search in some database to find the device IDs for everything to work out what this actually means....

So, I don't think you want to add that to the developper docs that you referenced. For me, this should go in the XML file where these definitions are made so we know exactly what this equates to. Alternatively, my other suggestion was it could. go in readme for each binding.

andrewfg commented 6 months ago

this should go in the XML file

Ok. We have a custom xml reader for those files, so hopefully it can handle comment fields in the xml without falling over. If so then I will simply add those comments. If not then I will need to fix that first.

andrewfg commented 6 months ago

Ok. Done.

@cdjackson see also this https://github.com/openhab/org.openhab.binding.zigbee/pull/821

cdjackson commented 6 months ago

Thanks.