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

[harmonyhub] Use AbstractStorageBasedTypeProvider #14507

Merged
merged 3 commits into from
May 13, 2023
Merged

[harmonyhub] Use AbstractStorageBasedTypeProvider #14507

merged 3 commits into from
May 13, 2023

Conversation

J-N-K
Copy link
Member

@J-N-K J-N-K commented Feb 26, 2023

This is the second of two reference implementations for the use of dynamic type providers.

@J-N-K J-N-K added enhancement An enhancement or new feature for an existing add-on awaiting other PR Depends on another PR labels Feb 26, 2023
@J-N-K J-N-K mentioned this pull request Feb 26, 2023
Merged
J-N-K added 2 commits March 4, 2023 13:07
@J-N-K
[harmonyhub] Use AbstractDynamicTypeProvider
0906a3c 
Signed-off-by: Jan N. Klug <[email protected]>
@J-N-K
fix
cd16e4a 
Signed-off-by: Jan N. Klug <[email protected]>
@J-N-K J-N-K force-pushed the harmonyhub-channel-type branch from dc39874 to cd16e4a Compare March 4, 2023 12:10
@J-N-K J-N-K changed the title [harmonyhub] Use AbstractDynamicTypeProvider [harmonyhub] Use AbstractStorageBasedTypeProvider Mar 4, 2023
@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/openhab-4-0-milestone-discussion/145133/18

@J-N-K
address review comment
f80424a 
Signed-off-by: Jan N. Klug <[email protected]>
lolodomo
lolodomo reviewed Apr 30, 2023
View reviewed changes
Copy link
Contributor

@lolodomo lolodomo 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 easy to apply.
Just one question.

...hub/src/main/java/org/openhab/binding/harmonyhub/internal/HarmonyHubDynamicTypeProvider.java Show resolved Hide resolved
@lolodomo
Copy link
Contributor

lolodomo commented Apr 30, 2023 •
edited
Loading

@digitaldan ! can you please have a look to this PR and the related PR in core framework to tell us if the change looks good to you for the harmonuhub binding ?

@kaikreuzer kaikreuzer removed the awaiting other PR Depends on another PR label May 7, 2023
@lolodomo lolodomo added rebuild Triggers Jenkins PR build and removed rebuild Triggers Jenkins PR build labels May 8, 2023
@lolodomo lolodomo mentioned this pull request May 8, 2023
Open
30 tasks
@J-N-K J-N-K marked this pull request as ready for review May 9, 2023 08:58
@J-N-K J-N-K requested a review from digitaldan as a code owner May 9, 2023 08:58
@J-N-K J-N-K marked this pull request as draft May 9, 2023 10:38
@J-N-K J-N-K marked this pull request as ready for review May 12, 2023 12:09
lolodomo
lolodomo approved these changes May 12, 2023
View reviewed changes
Copy link
Contributor

@lolodomo lolodomo left a comment

Choose a reason for hiding this comment

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

LGTM, thank you

@lolodomo
Copy link
Contributor

@digitaldan : would you like to test and review this PR before we merge it ?

@digitaldan
Copy link
Contributor

@digitaldan : would you like to test and review this PR before we merge it ?

Thanks, i'll test this out today and get back to everyone

@digitaldan
Copy link
Contributor

digitaldan commented May 13, 2023 •
edited
Loading

I compiled and updated this on my local instance , the binding stays offline for 2 mins as before, then comes backs online and works normally from there. I have not had time to run through the core changes, i might be able to look on sunday, is there something else i need to be doing, like removing the thing and adding it back in ? (i assume its suppose to not have the 2 min delay now)

edit:
I'm also on the latest nightly version as of May 12th , so i think the core changes should be there.

@J-N-K
Copy link
Member Author

J-N-K commented May 13, 2023

The 2min delay will be there for old things on first startup (because the binding can only create the dynamic types after/during first initialization). Ok subsequent starts it should not be there.

@digitaldan
Copy link
Contributor

Yep , after restarting the binding it now loads up right away.

digitaldan
digitaldan approved these changes May 13, 2023
View reviewed changes
Copy link
Contributor

@digitaldan digitaldan left a comment

Choose a reason for hiding this comment

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

LGTM!

@lolodomo lolodomo merged commit 36024a8 into openhab:main May 13, 2023
@lolodomo lolodomo added this to the 4.0 milestone May 13, 2023
@J-N-K J-N-K deleted the harmonyhub-channel-type branch May 13, 2023 16:08
@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/openhab-4-0-snapshot-discussion/142322/357

tb4jc pushed a commit to tb4jc/openhab-addons that referenced this pull request Jun 19, 2023
@J-N-K @tburri-alstom
[harmonyhub] Use AbstractStorageBasedTypeProvider (openhab#14507)
8ab995c 
* [harmonyhub] Use AbstractDynamicTypeProvider

---------

Signed-off-by: Jan N. Klug <[email protected]>
Signed-off-by: Thomas Burri <[email protected]>
matchews pushed a commit to matchews/openhab-addons that referenced this pull request Aug 9, 2023
@J-N-K @matchews
[harmonyhub] Use AbstractStorageBasedTypeProvider (openhab#14507)
436e371 
* [harmonyhub] Use AbstractDynamicTypeProvider

---------

Signed-off-by: Jan N. Klug <[email protected]>
Signed-off-by: Matt Myers <[email protected]>
austvik pushed a commit to austvik/openhab-addons that referenced this pull request Mar 27, 2024
@J-N-K @austvik
[harmonyhub] Use AbstractStorageBasedTypeProvider (openhab#14507)
6b98886 
* [harmonyhub] Use AbstractDynamicTypeProvider

---------

Signed-off-by: Jan N. Klug <[email protected]>
Signed-off-by: Jørgen Austvik <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jlaur jlaur jlaur left review comments

@lolodomo lolodomo lolodomo approved these changes

@digitaldan digitaldan digitaldan approved these changes

@ccutrer ccutrer ccutrer approved these changes

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

Successfully merging this pull request may close these issues.
7 participants
@J-N-K @openhab-bot @lolodomo @digitaldan @ccutrer @jlaur @kaikreuzer