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

Addon suggestion finder xml #1908

Merged
merged 9 commits into from Dec 28, 2023
Merged

Conversation

andrewfg
Copy link
Contributor

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

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
@andrewfg
Copy link
Contributor Author

@mherwege for info

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
…penhab.binding.zwave into suggestion-finder

Merge

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
@andrewfg andrewfg changed the title [wip] Addon suggestion finder xml Addon suggestion finder xml Dec 27, 2023
@andrewfg
Copy link
Contributor Author

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

@cdjackson
Copy link
Collaborator

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
Copy link
Contributor Author

andrewfg commented Dec 27, 2023

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
Copy link
Collaborator

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
Copy link
Contributor Author

andrewfg commented Dec 27, 2023

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
Copy link
Collaborator

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
Copy link
Contributor Author

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

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

@cdjackson
Copy link
Collaborator

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
Copy link
Contributor Author

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.

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
@andrewfg
Copy link
Contributor Author

Ok. Done.

@cdjackson see also this openhab/org.openhab.binding.zigbee#821

@cdjackson
Copy link
Collaborator

Thanks.

@cdjackson cdjackson merged commit 329d0b1 into openhab:main Dec 28, 2023
2 checks passed
@cdjackson cdjackson added this to the 4.2 milestone Dec 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants