-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[automower] Implement individual Things for configuration dependent items #19268
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
Conversation
…utomowerCommand.java Signed-off-by: Michael Weger <[email protected]>
Signed-off-by: Michael Weger <[email protected]>
Signed-off-by: Michael Weger <[email protected]>
Signed-off-by: Michael Weger <[email protected]>
Signed-off-by: Michael Weger <[email protected]>
Signed-off-by: Michael Weger <[email protected]>
Signed-off-by: Michael Weger <[email protected]>
Signed-off-by: Michael Weger <[email protected]>
Signed-off-by: Michael Weger <[email protected]>
Signed-off-by: Michael Weger <[email protected]>
Signed-off-by: Michael Weger <[email protected]>
Signed-off-by: Michael Weger <[email protected]>
|
@lsiepel This is the PR for the improvement that I was mentioning in #18630 (comment) Could you please have a look? |
Signed-off-by: Michael Weger <[email protected]>
lsiepel
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for creating the follow-up PR. I left some comments to look at. I have not seen a milestone release date yet, but it should not be long. Let's see if we can get this in.
...automower/src/main/java/org/openhab/binding/automower/internal/actions/AutomowerActions.java
Show resolved
Hide resolved
...g.automower/src/main/java/org/openhab/binding/automower/internal/bridge/AutomowerBridge.java
Show resolved
Hide resolved
...va/org/openhab/binding/automower/internal/rest/api/automowerconnect/AutomowerConnectApi.java
Show resolved
Hide resolved
....automower/src/main/java/org/openhab/binding/automower/internal/things/AutomowerCommand.java
Show resolved
Hide resolved
bundles/org.openhab.binding.automower/src/main/resources/OH-INF/thing/thing-types.xml
Outdated
Show resolved
Hide resolved
...er/src/main/java/org/openhab/binding/automower/internal/things/AutomowerWorkAreaHandler.java
Outdated
Show resolved
Hide resolved
...er/src/main/java/org/openhab/binding/automower/internal/things/AutomowerWorkAreaHandler.java
Outdated
Show resolved
Hide resolved
...er/src/main/java/org/openhab/binding/automower/internal/things/AutomowerWorkAreaHandler.java
Outdated
Show resolved
Hide resolved
...er/src/main/java/org/openhab/binding/automower/internal/things/AutomowerWorkAreaHandler.java
Show resolved
Hide resolved
...src/main/java/org/openhab/binding/automower/internal/things/AutomowerStayoutZoneHandler.java
Show resolved
Hide resolved
I'll try to be reactive in order to bring the changes into next release. This would avoid one additional breaking update. |
Signed-off-by: Michael Weger <[email protected]>
...er/src/main/java/org/openhab/binding/automower/internal/things/AutomowerWorkAreaHandler.java
Outdated
Show resolved
Hide resolved
....automower/src/main/java/org/openhab/binding/automower/internal/things/AutomowerHandler.java
Show resolved
Hide resolved
|
I did some testing from OH5.0.1 to this version. Some channel updates of static channels have been missed and are now added via ab0c995. The binding still throws errors. These come from the channels who have been created with the old binding version and are no longer there / or with changed type in the new binding version. Do I need to write code in the new binding, searching and deleting orphan channels? |
If you have some examples i can offer better guidances. But in general, it will be best to remove channels that are not there anymore, but where missed from previous upgrade instructions. Same applies to changed channel type's, but i would expect if those are part of previosu upgrade instructions that they fail. Do not alter previous ugprade instructions, only if a blocking issue occurs and prevents to complete the instruction at all, remaining the thing stuck upgrading. |
|
In zwavejs binding, i solved this by:
This ensures that i do not need to keep track of versions or whatever. looping those channels is an ultra minor step. I hope we do not need to have 'in code' upgrade instructions that we need to maintain for years. |
That makes sense. I've implemented already something similar in Line 279 in cd639ea
|
|
I'm still struggling with the upgrade from v1 (OH5.0.1) to v3:
There does exist an update channel instructions for the msg channels: |
|
I installed the native binding in the latest snapshot, added the automower, and linked the msg-timestamp channel. Since I don’t have a bridge (or device), it doesn’t go online, but everything looks fine. The thing type version is 2. I then removed the binding, dropped in the jar from this PR, and the thing/channel updated as expected: I noticed your mower thing is still at version 1. The channel type that fails was introduced in v2, so it looks like the thing does not upgrade from v1 to v2. Maybe related to https://github.com/openhab/openhab-addons/pull/18630/files#r2207150423 |
I’m currently testing the upgrade path from v1 to v3, since this is likely the most common scenario for users on the stable release. From my perspective, the update process seems problematic:
How does openHAB core handle multiple sequential updates? If it processes updates in order (1 → 2 → 3), this will always cause issues like this, since intermediate channel types may not exist in the final binding version. While the resulting error could be ignored, it’s tied to a 120s timeout: During this timeout, the Thing remains "Not Yet Ready." Furthermore any channel update in code (via In summary, upgrading from v1 to v3 results in log errors, a 120s timeout, and additional delay (cycle time) before the binding goes ONLINE. After that, everything works as expected. Is this normal and accepted behavior, or am I missing something? |
They are processed in order, not directly to the final version. Exactly to prevent the case you describe. I wonder if you can test the upgrade from v1 to v2 and check if the The 120s timeout is because it cannot find the thing type and waits before it is added to the thingtypeprovider, but you are not using that provider, so it waits until the timout. That is just a side effect of the upgrade failing. |
Signed-off-by: Michael Weger <[email protected]>
Signed-off-by: Michael Weger <[email protected]>
|
We can probably merge this as is, in the end the binding works as expected. Nevertheless i wonder why these upgrades give issues. Maybe @jlaur can share his thoughts on this. |
|
Yes, let's get it merged. @bern77, I compiled an OH5.0.2 version of these latest changes. |
|
Thanks, I'll give it a try! But most likely not before Sunday... I'll keep you posted on any findings! |
Sorry, it's a bit busy at the moment... I tried to take your snapshot for a spin, but ran into the old issue of Obviously, I've not received any response from Husqvarna on my ticket but that would have been quite a suprise anyway. So I fell back to a 5.1.0 snapshot from end of July, which works for me... |
Thanks for giving it a try and sorry that you have to report a similar problem again. Can you please have a second try on this update? |
Signed-off-by: Michael Weger <[email protected]>
Thanks for the fix - I've installed the new snapshot, now bringe + thing go online smoothly. I'll keep an eye on it and report any findings! |
Big thanks! Please be aware, that with this version individual Things for WorkAreas (and Stayout Zones - that you do not have) are needed. |
Thanks, that's a good hint indeed - I've updated my config accordingly! Way better design this way 😊 |
|
I'll monitor this PR, please report when all is green. |
In the log I see the following warning every now and then: |
My explanation is that the issue was caused by the first, failing upgrade attempt of the binding. Can you confirm that ...
|
Yes, I think you're right. The error appeared after the upgrade and since I restarted earlier today is hasn't appeared anymore. |
…tems (openhab#19268) * split automower stayout zones and work areas into individual things/AutomowerCommand.java Signed-off-by: Michael Weger <[email protected]>
[automower] Implement individual Things for configuration dependent items
This PR implements individual openHAB Things for configuration-dependent Automower items (WorkAreas and StayoutZones) to avoid ordering issues when the mower configuration changes. Instead of using consecutive channel naming, each item now gets its own Thing with a unique ID based on the Automower's configuration.
In addition, following improvements are implemented / issues are fixed:
handleCommand()forCHANNEL_WORKAREA_ENABLEDandCHANNEL_WORKAREA_CUTTING_HEIGHTwas mixed up