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

Fix startup of background discovery #4323

Merged
merged 1 commit into from
Jul 17, 2024
Merged

Fix startup of background discovery #4323

merged 1 commit into from
Jul 17, 2024

Conversation

lolodomo
Copy link
Contributor

Apply the same logic in AbstractThingHandlerDiscoveryService as in AbstractDiscoveryService to determine if background discovery should be enabled or disabled.

Fix openhab/openhab-addons#17089

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

@lolodomo
Fix startup of background discovery
0c95f11 
Apply the same logic in AbstractThingHandlerDiscoveryService as in AbstractDiscoveryService to determine if background discovery should be enabled or disabled.

Fix openhab/openhab-addons#17089

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
@lolodomo lolodomo requested a review from a team as a code owner July 17, 2024 12:17
@lolodomo
Copy link
Contributor Author

@openhab/core-maintainers : this PR fixes a severe bug leading to background discovery in bindings being always disabled.
This fix, once merged, should be back-ported to 4.1.x.

@J-N-K
Copy link
Member

J-N-K commented Jul 17, 2024

But your fix enables it by default, while the default was disabled before.

@lolodomo
Copy link
Contributor Author

The default was enabled by default until you changed it when you introduced this new class (and great mechanism).
Look at the class AbstractDiscoveryService.
https://github.com/openhab/openhab-core/blob/main/bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/AbstractDiscoveryService.java#L110

Except if you checked and changed all bindings discovery (and just forgot freeboxos), we have to restore the original default.

By the way, the two classes should be aligned.

@J-N-K J-N-K added the bug An unexpected problem or unintended behavior of the Core label Jul 17, 2024
@J-N-K J-N-K added this to the 4.3 milestone Jul 17, 2024
J-N-K
J-N-K approved these changes Jul 17, 2024
View reviewed changes
Copy link
Member

@J-N-K J-N-K left a comment

Choose a reason for hiding this comment

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

Thanks!

@J-N-K J-N-K merged commit 0a5886c into openhab:main Jul 17, 2024
5 checks passed
@lolodomo lolodomo deleted the fix_background_discovery branch July 17, 2024 17:58
J-N-K pushed a commit that referenced this pull request Jul 17, 2024
@lolodomo @J-N-K
Fix startup of background discovery (#4323)
d1c8fd1 
Apply the same logic in AbstractThingHandlerDiscoveryService as in AbstractDiscoveryService to determine if background discovery should be enabled or disabled.

Fix openhab/openhab-addons#17089

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
(cherry picked from commit 0a5886c)
@J-N-K J-N-K added the patch A PR that has been cherry-picked to a patch release branch label Jul 17, 2024
@J-N-K J-N-K mentioned this pull request Jul 19, 2024
Closed
@lsiepel lsiepel mentioned this pull request Aug 19, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@J-N-K J-N-K J-N-K 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.3
Development

Successfully merging this pull request may close these issues.

[freeboxos] Things are not discovered automatically
2 participants
@lolodomo @J-N-K