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

[kaleidescape] Switch to SDDP discovery for Strato and Alto #17508

Merged
merged 1 commit into from
Oct 5, 2024

Conversation

mlobstein
Copy link
Contributor

@mlobstein mlobstein commented Oct 5, 2024

Regression of #17371. UPnP discovery does not work due to an invalid deviceType field in the device description.xml. Accordingly, this PR changes the discovery participant to use SDDP instead.

@mlobstein mlobstein added the regression Regression that happened during the development of a release. Not shown on final release notes. label Oct 5, 2024
Signed-off-by: Michael Lobstein <michael.lobstein@gmail.com>
@mlobstein mlobstein added rebuild Triggers Jenkins PR build and removed rebuild Triggers Jenkins PR build labels Oct 5, 2024
Copy link
Contributor

@lsiepel lsiepel left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM.
As this is a regression to switch to SSDP, does this affect the discovery of any device (like the added support for Strato V) ? There is no functional downside to this?

Regarding the release notes. As this partly overrwrites the previous PR we need to remove the regression label as otherwise it won't show up in the release notes leaving the user to think UpnP support is added, while it is actually SSDP.

@lsiepel lsiepel added bug An unexpected problem or unintended behavior of an add-on and removed regression Regression that happened during the development of a release. Not shown on final release notes. labels Oct 5, 2024
@lsiepel lsiepel merged commit 8fcf18f into openhab:main Oct 5, 2024
5 checks passed
@lsiepel lsiepel added this to the 4.3 milestone Oct 5, 2024
@mlobstein mlobstein deleted the kaleidescape_sddp branch October 5, 2024 14:55
@mlobstein
Copy link
Contributor Author

Thanks, LGTM. As this is a regression to switch to SSDP, does this affect the discovery of any device (like the added support for Strato V) ? There is no functional downside to this?

I was just able to test with an older Strato and discovered the bug with UPnP. All devices in the Strato range are said to run the same kOS version so it is fairly safe to assume that the bug and also this fix will apply to all Strato devices.

@mlobstein mlobstein changed the title [kaleidescape] Switch to SDDP discovery [kaleidescape] Switch to SDDP discovery for Strato and Alto Oct 5, 2024
@mlobstein
Copy link
Contributor Author

mlobstein commented Oct 5, 2024

UPnP on this device is failing to work with jUPnP because the <deviceType> element is missing urn: before schemas...
image

joni1993 pushed a commit to joni1993/openhab-addons that referenced this pull request Oct 15, 2024
Signed-off-by: Michael Lobstein <michael.lobstein@gmail.com>
matchews pushed a commit to matchews/openhab-addons that referenced this pull request Oct 18, 2024
Signed-off-by: Michael Lobstein <michael.lobstein@gmail.com>
cipianpascu pushed a commit to cipianpascu/openhab-addons that referenced this pull request Jan 2, 2025
Signed-off-by: Michael Lobstein <michael.lobstein@gmail.com>
Signed-off-by: Ciprian Pascu <contact@ciprianpascu.ro>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An unexpected problem or unintended behavior of an add-on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants