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.

Sign up for GitHub

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

Jump to bottom

Consider network settings to avoid creating unexpected JmDNS instances #3978

Merged
merged 1 commit into from
Dec 30, 2023
Merged

Consider network settings to avoid creating unexpected JmDNS instances #3978

merged 1 commit into from
Dec 30, 2023

Conversation

lolodomo
Copy link
Contributor

Fix #3976

Signed-off-by: Laurent Garnier lg.hc@free.fr

@lolodomo lolodomo requested a review from a team as a code owner December 29, 2023 19:17
@lolodomo
Copy link
Contributor Author

@wborn @kaikreuzer : ready for your review.
This fix should then be backported to 4.1.x.

@openhab-bot
Copy link
Collaborator

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/unsuccess-openhab-updated-from-4-0-2-to-4-1-0-on-docker-openhab-unresponsive/152282/28

@lolodomo lolodomo marked this pull request as draft December 29, 2023 19:32
@lolodomo
Copy link
Contributor Author

I have a doubt if getAllInetAddresses could return different IP at each call when the network setting is set to use only one IP per interface. If it is possible, my code is still not perfect.

When the network settings are updated, the MDNSClient service is not informed. So in practice, for a standard user, it will require to restart openHAB. I don't know if this can be easily improved.
MDNSClient service is only informed if there is an IP address added or removed.

@lolodomo
Copy link
Contributor Author

I am not fully satisfied, I will update my proposal...

@lolodomo lolodomo force-pushed the fix_jmdns_instances branch from bbc9caa to 7a4dea2 Compare December 29, 2023 20:44
@lolodomo lolodomo marked this pull request as ready for review December 29, 2023 20:44
@lolodomo
Copy link
Contributor Author

Ok, now that is fine, I believe.

wborn
wborn approved these changes Dec 30, 2023
View reviewed changes
Copy link
Member

@wborn wborn left a comment

Choose a reason for hiding this comment

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

The changes look good to me! 👍
Now I will make a local build and briefly test it. 🙂

@wborn wborn added the bug An unexpected problem or unintended behavior of the Core label Dec 30, 2023
@wborn
Copy link
Member

wborn commented Dec 30, 2023

The unexpected instances I encountered are no longer there with these changes. 👍

@wborn wborn merged commit 63ae134 into openhab:main Dec 30, 2023
3 checks passed
wborn pushed a commit that referenced this pull request Dec 30, 2023
@lolodomo @wborn
Consider network settings to avoid creating unexpected JmDNS instances (
d602f5e 
#3978)

Fix #3976

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
@wborn wborn added the patch A PR that has been cherry-picked to a patch release branch label Dec 30, 2023
@lolodomo lolodomo deleted the fix_jmdns_instances branch December 30, 2023 11:53
@SHU-red SHU-red mentioned this pull request Dec 30, 2023
Closed
@lolodomo
Copy link
Contributor Author

Please add missing milestone 4.2.

1 similar comment
@lolodomo
Copy link
Contributor Author

Please add missing milestone 4.2.

@wborn wborn added this to the 4.2 milestone Dec 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wborn wborn wborn approved these changes

Assignees
No one assigned
Labels
bug An unexpected problem or unintended behavior of the Core patch A PR that has been cherry-picked to a patch release branch
Projects
None yet
Milestone
4.2
Development

Successfully merging this pull request may close these issues.

Openhab 4.1 starts mass of SocketListener and JmDNS Threads in Docker
3 participants
@lolodomo @openhab-bot @wborn