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

[resol] Fix handling of WeekTime fields on vbus #13753

Merged
merged 3 commits into from
Dec 3, 2022
Merged

[resol] Fix handling of WeekTime fields on vbus #13753

merged 3 commits into from
Dec 3, 2022

Conversation

ramack
Copy link
Contributor

@ramack ramack commented Nov 20, 2022 •
edited
Loading

Controllers (for example the SKSR 1/2/3) on the VBUS transmitting fields of type WeekTime did not work as decribed in #13447 which is fixed by this PR.

The resol-vbus-java library used by this binding was also improved for this controller in the meantime, that's why this PR integrates an upgrade to the latest version of this lib.

A compiled version of this code change can be found here and @paicl01 tested it successfully.

@ramack ramack added the bug An unexpected problem or unintended behavior of an add-on label Nov 20, 2022
@lolodomo
Copy link
Contributor

This looks like a breaking change to be mentioned in the release notes.

@ramack
Copy link
Contributor Author

ramack commented Nov 25, 2022

In a sense I agree, but as it breaks only for devices using the WeekTime type before it doesn't matter, as those did not work at all 😄

@lolodomo
Copy link
Contributor

But users will have to change their item type to make it work. So it is better to inform them.

@ramack
Copy link
Contributor Author

ramack commented Nov 25, 2022

yea why not. Where do I need to put it to make it occur in the release notes? The Readme in the binding doesn't seem to be the best place - on https://www.openhab.org/docs/developer/bindings/ I did not find anything and I am simply not contributing often enough to know it...

@lolodomo
Copy link
Contributor

yea why not. Where do I need to put it to make it occur in the release notes? The Readme in the binding doesn't seem to be the best place - on https://www.openhab.org/docs/developer/bindings/ I did not find anything and I am simply not contributing often enough to know it...

No problem, I will guide you once the PR is merged. This will be added in a second step, outside the current PR.

@ramack
rework after review
438da89 
Signed-off-by: Raphael Mack <ramack@raphael-mack.de>
@ramack ramack requested a review from lolodomo November 28, 2022 20:56
@ramack
rework 1 after review
e4c10fa 
Signed-off-by: Raphael Mack <ramack@raphael-mack.de>
lolodomo
lolodomo approved these changes Dec 3, 2022
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 lolodomo merged commit d777aa4 into openhab:main Dec 3, 2022
@lolodomo lolodomo added this to the 3.4 milestone Dec 3, 2022
@lolodomo
Copy link
Contributor

lolodomo commented Dec 3, 2022

@ramack : please create a PR to explain the breaking change.
Here is an example of what to do (only one line to add in the same file):
openhab/openhab-distro#1420

@ramack
Copy link
Contributor Author

ramack commented Dec 4, 2022 •
edited
Loading

@ramack : please create a PR to explain the breaking change. Here is an example of what to do (only one line to add in the same file): openhab/openhab-distro#1420

like this? openhab/openhab-distro#1427

@wborn wborn mentioned this pull request Dec 4, 2022
Merged
morph166955 pushed a commit to morph166955/openhab-addons that referenced this pull request Dec 18, 2022
@ramack @morph166955
[resol] Fix handling of WeekTime fields on vbus (openhab#13753)
ed39b23 
* correct handling of WeekTime fields from and update vbus library, fixes openhab#13447

Signed-off-by: Raphael Mack <ramack@raphael-mack.de>
Signed-off-by: Ben Rosenblum <rosenblumb@gmail.com>
andrasU pushed a commit to andrasU/openhab-addons that referenced this pull request Dec 24, 2022
@ramack @andrasU
[resol] Fix handling of WeekTime fields on vbus (openhab#13753)
567dde4 
* correct handling of WeekTime fields from and update vbus library, fixes openhab#13447

Signed-off-by: Raphael Mack <ramack@raphael-mack.de>
Signed-off-by: Andras Uhrin <andras.uhrin@gmail.com>
borazslo pushed a commit to borazslo/openhab-mideaac-addon that referenced this pull request Jan 8, 2023
@ramack @borazslo
[resol] Fix handling of WeekTime fields on vbus (openhab#13753)
1f33dc6 
* correct handling of WeekTime fields from and update vbus library, fixes openhab#13447

Signed-off-by: Raphael Mack <ramack@raphael-mack.de>
psmedley pushed a commit to psmedley/openhab-addons that referenced this pull request Feb 23, 2023
@ramack @psmedley
[resol] Fix handling of WeekTime fields on vbus (openhab#13753)
ae14a7e 
* correct handling of WeekTime fields from and update vbus library, fixes openhab#13447

Signed-off-by: Raphael Mack <ramack@raphael-mack.de>
nemerdaud pushed a commit to nemerdaud/openhab-addons that referenced this pull request Feb 28, 2023
@ramack @nemerdaud
[resol] Fix handling of WeekTime fields on vbus (openhab#13753)
498d0c1 
* correct handling of WeekTime fields from and update vbus library, fixes openhab#13447

Signed-off-by: Raphael Mack <ramack@raphael-mack.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lolodomo lolodomo lolodomo approved these changes

Assignees
No one assigned
Labels
bug An unexpected problem or unintended behavior of an add-on (potentially) not backward compatible
Projects
None yet
Milestone
3.4
Development

Successfully merging this pull request may close these issues.
2 participants
@ramack @lolodomo