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

[discovery.mdns] Devices may apply a grace period for removal from the Inbox #2635

Merged
merged 4 commits into from
Dec 22, 2021

Conversation

andrewfg
Copy link
Contributor

@andrewfg andrewfg commented Dec 21, 2021

BACKGROUND

Some types of OpenHAB bindings have devices that can sometimes be a bit late in sending their mDNS renewal announcements, which means that such devices are repeatedly removed from, and (re)added to, the InBox. This leads to confusion in the UI, and repeated logger messages. This is an exact analog to the UPNP Discovery issue that was solved by #2144

SOLUTION

The solution is an exact analog to the solution in #2144
Resolves #1835

DOCUMENTATION

The binding developer guide will also be updated via this PR openhab/openhab-docs#1708

Signed-off-by: Andrew Fiddian-Green [email protected]

Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
@andrewfg
Copy link
Contributor Author

andrewfg commented Dec 21, 2021

Resolves #1835 in general.
And in combination with openhab/openhab-addons#11834 it resolves the specific Miele issue.

@jlaur
Copy link
Contributor

jlaur commented Dec 21, 2021

@andrewfg - link #1835 to this PR?

@andrewfg
Copy link
Contributor Author

^
Yes. See my post above yours.

@jlaur
Copy link
Contributor

jlaur commented Dec 21, 2021

^ Yes. See my post above yours.

Ah, well, I meant reference it here to be able to directly cross-navigate and have issue auto-closed when PR is merged:

image

@andrewfg
Copy link
Contributor Author

AFAIK the keyword “Resolves” (or “Fixes”, “Solves” etc.) followed by the issue number should indeed cause Git to auto close the issue.

@cweitkamp cweitkamp added the enhancement An enhancement or new feature of the Core label Dec 21, 2021
@jlaur
Copy link
Contributor

jlaur commented Dec 21, 2021

AFAIK the keyword “Resolves” (or “Fixes”, “Solves” etc.) followed by the issue number should indeed cause Git to auto close the issue.

Yes, if you write it like "Fixes #1835" in the PR description, it will be linked correctly.

@jlaur
Copy link
Contributor

jlaur commented Dec 22, 2021

I have tested this PR in combination with openhab/openhab-addons#11834 and it works as expected - no more constant removal and readding of the Miele XGW 3000 gateway. From code perspective it also looks fine to me. Thanks again for this fix @andrewfg. Now will leave it to core maintainers to review.

Copy link
Member

@kaikreuzer kaikreuzer left a comment

Choose a reason for hiding this comment

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

lgtm, thanks!

@kaikreuzer kaikreuzer merged commit 459dac9 into openhab:main Dec 22, 2021
@kaikreuzer kaikreuzer added this to the 3.3 milestone Dec 22, 2021
@cweitkamp
Copy link
Contributor

I am afraid we missed one place in the manual scan where we have to cancel the removal task.

private void scan(boolean isBackground) {
for (MDNSDiscoveryParticipant participant : participants) {
long start = System.currentTimeMillis();
ServiceInfo[] services;
if (isBackground) {
services = mdnsClient.list(participant.getServiceType());
} else {
services = mdnsClient.list(participant.getServiceType(), FOREGROUND_SCAN_TIMEOUT);
}
logger.debug("{} services found for {}; duration: {}ms", services.length, participant.getServiceType(),
System.currentTimeMillis() - start);
for (ServiceInfo service : services) {
DiscoveryResult result = participant.createResult(service);
if (result != null) {
final DiscoveryResult resultNew = getLocalizedDiscoveryResult(result,
FrameworkUtil.getBundle(participant.getClass()));
thingDiscovered(resultNew);
}
}
}
}

I will provide a follow-up PR with a fix and some minor code refactoring.

@andrewfg
Copy link
Contributor Author

Ok. Thanks.

@cweitkamp
Copy link
Contributor

See #2647

splatch pushed a commit to ConnectorIO/copybara-hab-core that referenced this pull request Jul 12, 2023
@andrewfg andrewfg deleted the mdns-grace-period branch August 25, 2024 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement or new feature of the Core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[MDNSDiscoveryService] potential bug deleting items from Inbox??
4 participants