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

[danfossairunit] Add instruction for channel removal #14473

Merged

Conversation

jlaur
Copy link
Contributor

@jlaur jlaur commented Feb 21, 2023

Follow-up to #14030
Made possible by openhab/openhab-core#3330
Requires also openhab/openhab-core#3391

@jlaur jlaur added the awaiting other PR Depends on another PR label Feb 21, 2023
@jlaur jlaur requested a review from J-N-K February 21, 2023 22:06
@jlaur jlaur removed the awaiting other PR Depends on another PR label Mar 7, 2023
@jlaur
Copy link
Contributor Author

jlaur commented Mar 7, 2023

I will retest this tomorrow when there is a new snapshot build.

Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
@jlaur jlaur force-pushed the 10621-danfossairunit-update-instruction branch from 07d1f35 to 7e4f1d0 Compare March 8, 2023 19:35
@jlaur
Copy link
Contributor Author

jlaur commented Mar 8, 2023

I have now tested with build 3358, and got it working. First attempt failed though. Dropping the new JAR with the upgrade instructions caused the thingTypeVersion property to be set to 1, but didn't execute the upgrade instructions. Restarting openHAB may have triggered the upgrade, but now version was already 1, so nothing happened (removed channel still there). Finally I went back to old version and recreated thing. Then stopped openHAB, dropped new JAR, cleared cache and started openHAB again. This time the channel was removed.

@J-N-K - can you confirm if this is the intended behavior and/or if my test is invalid?

@jlaur jlaur marked this pull request as ready for review March 8, 2023 19:54
@jlaur jlaur requested a review from pravussum as a code owner March 8, 2023 19:54
@jlaur jlaur requested a review from a team March 9, 2023 09:48
@jlaur
Copy link
Contributor Author

jlaur commented Mar 10, 2023

@J-N-K - I don't know if you saw my question, but let me rephrase: Can you confirm that the upgrade instructions are only executed when openHAB is started? Otherwise I don't understand my test results. This would probably mean that replacing JAR's between releases, or upgrading bindings from Community Marketplace, will prevent the upgrade from being triggered.

@J-N-K
Copy link
Member

J-N-K commented Mar 10, 2023

They are checked when the thing is initialized, so: no, this does not depend on openHAB startup. It may be that it fails if you replace a bundle with bundle:update because the thing is already initialized when this happens. Uninstalling and dropping a .jar in the addons folder should work, also deleting and later (i.e. after uninstall is done) adding it again will also work. Overwriting may result in confusion, just like using bundle:update.

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.

LGTM, if you have a scenario that triggers the faulty behavior I can try to see if there is a bug in core.

@jlaur
Copy link
Contributor Author

jlaur commented Mar 10, 2023

They are checked when the thing is initialized, so: no, this does not depend on openHAB startup. It may be that it fails if you replace a bundle with bundle:update because the thing is already initialized when this happens. Uninstalling and dropping a .jar in the addons folder should work, also deleting and later (i.e. after uninstall is done) adding it again will also work. Overwriting may result in confusion, just like using bundle:update.

Thanks. In my tests I was overwriting the JAR in the addons folder. I will try to remove and drop it instead, and repeat my tests. In any case, I was able to trigger the upgrade by changing my test approach, so the PR is ready. @lolodomo, @kaikreuzer - can be merged.

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

@lolodomo lolodomo merged commit dfb172d into openhab:main Mar 10, 2023
@lolodomo lolodomo added this to the 4.0 milestone Mar 10, 2023
@jlaur jlaur deleted the 10621-danfossairunit-update-instruction branch March 10, 2023 14:30
@jlaur
Copy link
Contributor Author

jlaur commented Mar 11, 2023

@J-N-K - I did some additional testing, both replacing and deleting/creating JAR as well as with and without removed channel being linked to an item. I cannot reproduce the mentioned problem in a reliable way, so it seems to be working.

@jlaur jlaur mentioned this pull request Mar 14, 2023
renescherer pushed a commit to renescherer/openhab-addons that referenced this pull request Mar 23, 2023
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
FordPrfkt pushed a commit to FordPrfkt/openhab-addons that referenced this pull request Apr 20, 2023
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants