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

Adapt to core changes (ThingHandlerService) #16107

Merged
merged 183 commits into from
Jan 3, 2024
Merged

Adapt to core changes (ThingHandlerService) #16107

merged 183 commits into from
Jan 3, 2024

Conversation

J-N-K
Copy link
Member

@J-N-K J-N-K commented Dec 24, 2023

@J-N-K J-N-K added the awaiting other PR Depends on another PR label Dec 24, 2023
@J-N-K J-N-K mentioned this pull request Dec 24, 2023
Merged
@J-N-K J-N-K marked this pull request as ready for review January 2, 2024 12:06
@wborn wborn removed the rebuild Triggers Jenkins PR build label Jan 2, 2024
@J-N-K
Copy link
Member Author

J-N-K commented Jan 2, 2024

@openhab/add-ons-maintainers Please make sure that every contribution of thing-bound actions/providers/services follows the new coding scheme. Since this is a breaking change, older and newer versions that make use of ThingHandlerService will no longer work with the old code.

wborn
wborn reviewed Jan 2, 2024
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.

Thanks for your huge work! It nicely cleans up all the workarounds. 👍

I only found these comments:

...main/java/org/openhab/binding/verisure/internal/discovery/VerisureThingDiscoveryService.java Show resolved Hide resolved
...sync/src/main/java/org/openhab/binding/vesync/internal/discovery/VeSyncDiscoveryService.java Show resolved Hide resolved
...le/src/main/java/org/openhab/binding/windcentrale/internal/WindcentraleDiscoveryService.java Show resolved Hide resolved
...inding.wled/src/main/java/org/openhab/binding/wled/internal/WLedSegmentDiscoveryService.java Show resolved Hide resolved
david-pace
david-pace approved these changes Jan 2, 2024
View reviewed changes
Copy link
Member

@david-pace david-pace left a comment

Choose a reason for hiding this comment

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

LGTM for boschshc

@J-N-K
address review comments
c05f29c 
Signed-off-by: Jan N. Klug <github@klug.nrw>
@J-N-K
Copy link
Member Author

J-N-K commented Jan 2, 2024

done

wborn
wborn approved these changes Jan 2, 2024
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.

Thank you very much!

@wborn wborn added this to the 4.2 milestone Jan 2, 2024
@wborn wborn added the enhancement An enhancement or new feature for an existing add-on label Jan 2, 2024
mvalla
mvalla approved these changes Jan 2, 2024
View reviewed changes
Copy link
Contributor

@mvalla mvalla left a comment

Choose a reason for hiding this comment

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

LGTM for OpenWebNet

stefanroellin
stefanroellin reviewed Jan 2, 2024
View reviewed changes
Copy link
Contributor

@stefanroellin stefanroellin left a comment

Choose a reason for hiding this comment

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

If I read the annotation in AbstractThingHandlerDiscoveryService correctly, we can assume that thingHandler is not null and it is not necessary to check for null. Is this correct? If so, some checks in PilightDeviceDiscoveryService can be removed. I could do this in a follow up pull request after this is merged.

.../main/java/org/openhab/binding/pilight/internal/discovery/PilightDeviceDiscoveryService.java Show resolved Hide resolved
wborn
wborn reviewed Jan 2, 2024
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.

There are a few more overridden methods that have this issue:

.../src/main/java/org/openhab/binding/avmfritz/internal/discovery/AVMFritzDiscoveryService.java Show resolved Hide resolved
.../src/main/java/org/openhab/binding/avmfritz/internal/discovery/AVMFritzDiscoveryService.java Show resolved Hide resolved
...ding.asuswrt/src/main/java/org/openhab/binding/asuswrt/internal/AsuswrtDiscoveryService.java Show resolved Hide resolved
...src/main/java/org/openhab/binding/boschindego/internal/discovery/IndegoDiscoveryService.java Show resolved Hide resolved
@J-N-K
address review comments
9440449 
Signed-off-by: Jan N. Klug <github@klug.nrw>
@J-N-K
Copy link
Member Author

J-N-K commented Jan 2, 2024

Done.

stefanroellin
stefanroellin approved these changes Jan 2, 2024
View reviewed changes
Copy link
Contributor

@stefanroellin stefanroellin left a comment

Choose a reason for hiding this comment

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

LGTM for pilight. Thanks for the changes.

MrRonfo
MrRonfo approved these changes Jan 2, 2024
View reviewed changes
Copy link

@MrRonfo MrRonfo left a comment

Choose a reason for hiding this comment

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

LGTM for bticinosmarther

wborn
wborn reviewed Jan 2, 2024
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.

Looks like there are a few more:

...main/java/org/openhab/binding/qolsysiq/internal/discovery/QolsysIQChildDiscoveryService.java Show resolved Hide resolved
...tapocontrol/src/main/java/org/openhab/binding/tapocontrol/internal/TapoDiscoveryService.java
TapoBridgeConfiguration config = bridge.getBridgeConfig();
public void initialize() {
thingHandler.setDiscoveryService(this);
TapoBridgeConfiguration config = thingHandler.getBridgeConfig();
Copy link
Member

Choose a reason for hiding this comment

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

I think it should also call super.initialize(); here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Debatable, because the service handles its background discovery by itself and the activate method didn't call super.activate either. I have refactored that to use the standard mechanism, unfortunately I can't test it.

...tapocontrol/src/main/java/org/openhab/binding/tapocontrol/internal/TapoDiscoveryService.java Outdated Show resolved Hide resolved
...ab.binding.tr064/src/main/java/org/openhab/binding/tr064/internal/Tr064DiscoveryService.java Show resolved Hide resolved
@J-N-K
address review comments
bd4a686 
Signed-off-by: Jan N. Klug <github@klug.nrw>
@J-N-K
Copy link
Member Author

J-N-K commented Jan 3, 2024

done

gdolfen
gdolfen approved these changes Jan 3, 2024
View reviewed changes
Copy link

@gdolfen gdolfen left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM for enigma2

@wborn wborn merged commit 3136425 into openhab:main Jan 3, 2024
3 checks passed
@J-N-K J-N-K deleted the x branch January 3, 2024 11:13
Cybso pushed a commit to Cybso/openhab-addons that referenced this pull request Jan 5, 2024
@J-N-K @Cybso
Adapt to core changes (ThingHandlerService) (openhab#16107)
a55e2d8 
Related to: openhab/openhab-core#3957

Signed-off-by: Jan N. Klug <github@klug.nrw>
@J-N-K J-N-K mentioned this pull request Jan 14, 2024
Merged
@jlaur jlaur mentioned this pull request Jan 15, 2024
Merged
austvik pushed a commit to austvik/openhab-addons that referenced this pull request Mar 27, 2024
@J-N-K @austvik
Adapt to core changes (ThingHandlerService) (openhab#16107)
bb645bd 
Related to: openhab/openhab-core#3957

Signed-off-by: Jan N. Klug <github@klug.nrw>
Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
joni1993 pushed a commit to joni1993/openhab-addons that referenced this pull request Oct 15, 2024
@J-N-K @joni1993
Adapt to core changes (ThingHandlerService) (openhab#16107)
6ef5794 
Related to: openhab/openhab-core#3957

Signed-off-by: Jan N. Klug <github@klug.nrw>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gdolfen gdolfen gdolfen approved these changes

@MrRonfo MrRonfo MrRonfo approved these changes

@bern77 bern77 bern77 approved these changes

@mvalla mvalla mvalla approved these changes

@wborn wborn wborn approved these changes

@EvilPingu EvilPingu EvilPingu approved these changes

@stefanroellin stefanroellin stefanroellin approved these changes

@david-pace david-pace david-pace approved these changes

@clinique clinique clinique approved these changes

@druciak druciak druciak approved these changes

@mhilbush mhilbush Awaiting requested review from mhilbush mhilbush is a code owner

@miloit miloit Awaiting requested review from miloit miloit is a code owner

@zarusz zarusz Awaiting requested review from zarusz zarusz is a code owner

@pavel-gololobov pavel-gololobov Awaiting requested review from pavel-gololobov pavel-gololobov is a code owner

@computergeek1507 computergeek1507 Awaiting requested review from computergeek1507 computergeek1507 is a code owner

@BoBiene BoBiene Awaiting requested review from BoBiene BoBiene is a code owner

@Skinah Skinah Awaiting requested review from Skinah Skinah is a code owner

@marcelrv marcelrv Awaiting requested review from marcelrv marcelrv is a code owner

@tdeckers tdeckers Awaiting requested review from tdeckers tdeckers is a code owner

@jamesmelville jamesmelville Awaiting requested review from jamesmelville jamesmelville is a code owner

@Jamstah Jamstah Awaiting requested review from Jamstah Jamstah is a code owner

@jannegpriv jannegpriv Awaiting requested review from jannegpriv jannegpriv is a code owner

@gs4711 gs4711 Awaiting requested review from gs4711 gs4711 is a code owner

@andrewfg andrewfg Awaiting requested review from andrewfg andrewfg is a code owner

@cedricboon cedricboon Awaiting requested review from cedricboon cedricboon is a code owner

@mgbowman mgbowman Awaiting requested review from mgbowman mgbowman is a code owner

@Hilbrand Hilbrand Awaiting requested review from Hilbrand Hilbrand is a code owner

@cweitkamp cweitkamp Awaiting requested review from cweitkamp cweitkamp is a code owner

@kaikreuzer kaikreuzer Awaiting requested review from kaikreuzer kaikreuzer is a code owner

@roieg roieg Awaiting requested review from roieg roieg is a code owner

@kgoderis kgoderis Awaiting requested review from kgoderis kgoderis is a code owner

@nokyyz nokyyz Awaiting requested review from nokyyz nokyyz is a code owner

@olivierkeke olivierkeke Awaiting requested review from olivierkeke olivierkeke is a code owner

@renescherer renescherer Awaiting requested review from renescherer renescherer is a code owner

@fazioa fazioa Awaiting requested review from fazioa fazioa is a code owner

@octa22 octa22 Awaiting requested review from octa22 octa22 is a code owner

@dalgwen dalgwen Awaiting requested review from dalgwen dalgwen is a code owner

@seime seime Awaiting requested review from seime seime is a code owner

@martinvw martinvw Awaiting requested review from martinvw martinvw is a code owner

@paulianttila paulianttila Awaiting requested review from paulianttila paulianttila is a code owner

@ramack ramack Awaiting requested review from ramack ramack is a code owner

@lolodomo lolodomo Awaiting requested review from lolodomo lolodomo is a code owner

@mlobstein mlobstein Awaiting requested review from mlobstein mlobstein is a code owner

@digitaldan digitaldan Awaiting requested review from digitaldan digitaldan is a code owner

@appzer appzer Awaiting requested review from appzer appzer is a code owner

@hakan42 hakan42 Awaiting requested review from hakan42 hakan42 is a code owner

@lsiepel lsiepel Awaiting requested review from lsiepel lsiepel is a code owner

@niklasdoerfler niklasdoerfler Awaiting requested review from niklasdoerfler niklasdoerfler is a code owner

@pail23 pail23 Awaiting requested review from pail23 pail23 is a code owner

@ecdye ecdye Awaiting requested review from ecdye ecdye is a code owner

@janvyb janvyb Awaiting requested review from janvyb janvyb is a code owner

@mherwege mherwege Awaiting requested review from mherwege mherwege is a code owner

@crnjan crnjan Awaiting requested review from crnjan crnjan is a code owner

@stefan-hoehn stefan-hoehn Awaiting requested review from stefan-hoehn stefan-hoehn is a code owner

@ntruchsess ntruchsess Awaiting requested review from ntruchsess ntruchsess is a code owner

@ccutrer ccutrer Awaiting requested review from ccutrer ccutrer is a code owner

@ssalonen ssalonen Awaiting requested review from ssalonen ssalonen is a code owner

@BjoernLange BjoernLange Awaiting requested review from BjoernLange BjoernLange is a code owner

@thewiep thewiep Awaiting requested review from thewiep thewiep is a code owner

@bobadair bobadair Awaiting requested review from bobadair bobadair is a code owner

@sprehn sprehn Awaiting requested review from sprehn sprehn is a code owner

@fwolter fwolter Awaiting requested review from fwolter fwolter is a code owner

@jsjames jsjames Awaiting requested review from jsjames jsjames is a code owner

@GiviMAD GiviMAD Awaiting requested review from GiviMAD GiviMAD is a code owner

@gerrieg gerrieg Awaiting requested review from gerrieg

@bruestel bruestel Awaiting requested review from bruestel bruestel is a code owner

@Wire82 Wire82 Awaiting requested review from Wire82 Wire82 is a code owner

@matchews matchews Awaiting requested review from matchews matchews is a code owner

@jlaur jlaur Awaiting requested review from jlaur jlaur is a code owner

@maniac103 maniac103 Awaiting requested review from maniac103 maniac103 is a code owner

@alexf2015 alexf2015 Awaiting requested review from alexf2015 alexf2015 is a code owner

@andrew-schofield andrew-schofield Awaiting requested review from andrew-schofield andrew-schofield is a code owner

@rmichalak rmichalak Awaiting requested review from rmichalak rmichalak is a code owner

@lujop lujop Awaiting requested review from lujop lujop is a code owner

@GerdZanker GerdZanker Awaiting requested review from GerdZanker GerdZanker is a code owner

@jofleck jofleck Awaiting requested review from jofleck jofleck is a code owner

@billfor billfor Awaiting requested review from billfor billfor is a code owner

Assignees
No one assigned
Labels
enhancement An enhancement or new feature for an existing add-on
Projects
None yet
Milestone
4.2
Development

Successfully merging this pull request may close these issues.