-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[automower] Status update via Husqvarna WebSocket API #18630
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Michael Weger <[email protected]>
Signed-off-by: Michael Weger <[email protected]>
Signed-off-by: Michael Weger <[email protected]>
…ondition Signed-off-by: Michael Weger <[email protected]>
Signed-off-by: Michael Weger <[email protected]>
Signed-off-by: Michael Weger <[email protected]>
update of timestamp on WSS events 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]>
Ready for review. |
Having recently purchased a newer Automower model, I was just about to start extending the existing binding to add support for working areas. I see this is almost completely included already in the new version - thanks a lot for your great work! There's one thing though, I would consider a great addition, i.e. the "StartInWorkArea" action (which allows you to send the mower directly into a work area). Do you think it's feasible for you to add this? |
As the Automower API supports this command, it will be easy to add. |
One question regarding the channels of group "Work Areas" - in my case these channels don't seem to be updated. |
What does the Thing Property Could the issue be caused by orphan items due to the breaking changes of the update? |
yes
I'm actually running this in a dedicated docker container / separate OH instance - it's the only binding and I generated items just for all channels offered by the binding. I've also noticed that the updates don't seem to happend automatically (I configured 300s). |
Please enable TRACE logging and share the relevant parts |
I triggered the update manually, then received this output and nothing else yet:
|
Signed-off-by: Michael Weger <[email protected]>
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 taking the time to refactor and add this enhancement. Found some minor issues while looking at all files except the handerls and apiclient. Before i do, i think it is good to agree on how the factory, bridge and mower handerls work together. I left a suggestion to make use of a subscription pattern.
bundles/org.openhab.binding.automower/src/main/resources/OH-INF/thing/thing-types.xml
Outdated
Show resolved
Hide resolved
bundles/org.openhab.binding.automower/src/main/resources/OH-INF/thing/thing-types.xml
Show resolved
Hide resolved
bundles/org.openhab.binding.automower/src/main/resources/OH-INF/thing/thing-types.xml
Outdated
Show resolved
Hide resolved
bundles/org.openhab.binding.automower/src/main/resources/OH-INF/update/update.xml
Show resolved
Hide resolved
bundles/org.openhab.binding.automower/src/main/resources/OH-INF/update/update.xml
Show resolved
Hide resolved
Signed-off-by: Michael Weger <[email protected]>
fixed compiler warnings Signed-off-by: Michael Weger <[email protected]>
bundles/org.openhab.binding.automower/src/main/resources/OH-INF/update/update.xml
Show resolved
Hide resolved
Signed-off-by: Michael Weger <[email protected]>
Signed-off-by: Michael Weger <[email protected]>
From my point of view all findings are addressed or commented. |
There is a conflict to solve. |
This reverts commit e46d27f. Signed-off-by: Michael Weger <[email protected]>
When resolving the conflict in the editor, how can I sign it of using a real email? - found it. Conflict resolved. |
Signed-off-by: MikeTheTux <[email protected]>
Signed-off-by: Michael Weger <[email protected]>
…addons into automowerWSS
Signed-off-by: Michael Weger <[email protected]>
@lsiepel thx for all your efforts on this project! Can we get this one now merged as well? As this issue didn't made it into 5.0.0, I have to provide a new upgrade instruction. |
I guess this one shall be flagged as breaking-change. Here is the update info: openhab/openhab-distro#1773 |
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.
Final review. Some questions and suggestions. covered all files and changes.
public static final ChannelTypeUID CHANNEL_TYPE_STAYOUTZONES_DIRTY = new ChannelTypeUID(BINDING_ID, | ||
"zoneDirtyType"); | ||
|
||
public static final Map<Integer, String> ERROR = new HashMap<>() { | ||
private static final long serialVersionUID = 1L; |
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.
Did you get a warning ? Don;t understand why this is added here.
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.
[WARNING] AutomowerBindingConstants.java:[193,58] The serializable class does not declare a static final serialVersionUID field of type long
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.
Wondering what causes this. It is a static class with static constants, it should never be serialized. None of these files of all bindings have this.
....automower/src/main/java/org/openhab/binding/automower/internal/AutomowerHandlerFactory.java
Outdated
Show resolved
Hide resolved
....automower/src/main/java/org/openhab/binding/automower/internal/things/AutomowerHandler.java
Outdated
Show resolved
Hide resolved
public AutomowerHandler(Thing thing, TimeZoneProvider timeZoneProvider) { | ||
super(thing); | ||
this.timeZoneProvider = timeZoneProvider; | ||
this.mowerZoneId = timeZoneProvider.getTimeZone(); // default initializer | ||
} | ||
|
||
@Override | ||
public void handleCommand(ChannelUID channelUID, Command command) { | ||
public synchronized void handleCommand(ChannelUID channelUID, Command command) { |
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.
Similar comment regarding synchronized. I would not expect that here.
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.
The Channel commands are updating the automower status cached in a global variable and then send a request with the updated values. Having several Channel commands in parallel, could lead to inconsistent updates send to the API - e.g. when the user calls:
items.AM430X_Task_Start_01.sendCommand(360);
items.AM430X_Task_Duration_01.sendCommand(480);
I would like to make sure, that the Start
update is fully processes before the Duration
update is executed.
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.
Hard to get a full overview on my phone. But if the lock’s scope can’t be reduced any more, this is what it is.
Implementation details like this can always be further adjusted
...g.automower/src/main/java/org/openhab/binding/automower/internal/bridge/AutomowerBridge.java
Outdated
Show resolved
Hide resolved
...ower/src/main/java/org/openhab/binding/automower/internal/bridge/AutomowerBridgeHandler.java
Outdated
Show resolved
Hide resolved
...r/src/main/java/org/openhab/binding/automower/internal/bridge/AutomowerWebSocketAdapter.java
Outdated
Show resolved
Hide resolved
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]>
If you fix the conflict, we are ready to merge. Please. Check the conflict carefully, as we spend some time in getting the markup right. |
Signed-off-by: MikeTheTux <[email protected]>
Conflict resolved - was just a single empty line in readme |
[automower] Status update via Husqvarna WebSocket API
Implementation of even base status update of Husqvarna automower via WebSocket API in parallel to the cyclic polling via REST API.
Description
This PR resolves 2 issues:
Removed the numbered 1 ... 50
position
andmessage
channels. Motivation: