-
Notifications
You must be signed in to change notification settings - Fork 125
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
Do not abort current behaviour after emergency is cleared #86
Do not abort current behaviour after emergency is cleared #86
Conversation
When emergency is cleared, mower should continue its last behaviour. This will make the mower to continue mowing when the emergency status is cleared manually.
Also stop enabling mow on continue, will be enabled at the first point
I merged this PR because my I'm still struggling to finish my first full mowing session 🙈 |
@rovo89 can you grab the logs? It will not re-enable the mow motor immediately, drives to the first point in the path and then enables mowing. First point should be relatively close though |
Are there any instructions how to do that? In my case, the "crash" happened while mowing the last round of area outline. With "first point in the path", do you mean that it would finish the outline path without mowing? And as soon as it starts the next path (obstacle outline), it would start mowing again? |
Ah, are you simply referring to the logs in |
Ah, don't think openmower-gui clears the error correctly |
Well, at least it does continue to drive afterwards... and I have no other way to test. |
I think the emergency started at 1716114363, I cleared it after less than 10 seconds. At 1716114392 I enabled the blade via GUI. Here's another one:
Emergency at 1716114407, cleared it after a bit less than 30 seconds. Re-enabled the mower at 1716114443. |
Confirmed it's an openmower-gui issue. Doesn't work when clearing emergency via openmower-gui, work fine if using the cover UI button. |
OK, so I guess I'll have to check what the GUI does and what the official way would be - assuming that there is one. "Clear emergency" should probably be an action in some central service which gets called by the Cover UI and the GUI (and MQTT and...) in the same way. |
I'm now testing and finding likewise-> that it restarts the path correctly after an emergency but doesn't re-enable the mow motow, not even after minute or two off following it's usual little route around. So, nearly there, and it's easy to reenable motor manually at same time as clearning emergency. So vast improvement for me already, thanks Sample log from turning emergency mode off: [ERROR] [1716212826.663697518]: Low Level Emergency. Bitmask was: 1 |
yes, thanks I noticed that and I'm trying to build it now. I'm a bit new to git, but just to check, am i correct in saying the best way to do this is to:
?? Or is there a quicker way to do it by pulling one of your builds directly? thanks |
This maybe? https://github.com/olliewalsh/open_mower_ros/pkgs/container/open_mower_ros I have meanwhile changed my setup to a self-built container with ROS and other dependencies, but not Open Mower itself. Then I cloned the open_mower_ros repo and mount it into the container for compiling, so it can use incremental builds. I use the same container and mount for actually running the robot. So whenever I want to try something new, I just need to pull/do the change, recompile (usually seconds) and then restart the container. WIP is published at https://github.com/rovo89/open_mower_bare_container, but I'm still experimenting and it's not documented yet. |
ah yes that latest build on OW's page looks like it. I looked about four hours ago and that one wasnt there then. Anyway nearly built it myself now so I'll give that a go. 👍 |
That branch wasn't the current version. This one is up to date if you want to try it ghcr.io/olliewalsh/open_mower_ros:releases-no-abort-after-emergency-2m |
(no change required in openmower-gui, the service it was calling should have worked and now does) |
This is a bit offtopic, but still related: I noticed that if emergency is cleared not immediately, but after some time, then after emergency is cleared the mower goes to the IDLE state (I am still running with the code from my older fix branch). Is that just a bug from my faulty bug fix, or is that a normal behaviour? I would like to increase this timeout. |
Don't think that will be an issue with this PR, will stay paused until emergency is cleared. |
I noticed the same. But not a big problem anymore because "Start" continues at the same spot. |
Do you mean with the latest version of this PR? |
I wasn't able to test the last commit yet. I don't see anything related to timeouts in it, but maybe it's fixed indirectly. |
IIRC Originally when emergency is cleared it would abort mowing behaviour, which will switch directly to docking behaviour. With the latest changes in the PR is pausing mowing behaviour on emergency and resuming mowing behaviour when the emergency is cleared, so should fix that issue directly |
I can confirm that with the latest commit, the mower happily continues to mow after turning off emergency mode via Cedric's GUI. I haven't experienced the "timeout" issue. Not 100% sure it's gone, as I had very limited time to test, but the main objective of the PR is fulfilled, so I would suggest to merge it. If it comes up again, I'll report back. |
havent had a chance to check yet as raining in sunny uk! hopefully tomorrow. But sounds good. |
} | ||
} | ||
} else { | ||
currentBehavior->requestContinue(); |
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.
regular pause/continue (non-emergency) no longer works though, this will always continue immediately if there is no emergency
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.
Fixed in latest commit
@ClemensElflein it's not that pretty but I think the logic is correct now so should be good to merge when you have time |
Also tested today. Thanks a lot @olliewalsh !! |
working well for me too 👍🙏 |
Rebased @2m's PR #65 on latest main and added patch to pause/resume during emergency.
Briefly tested it with by triggering/clearing emergency and not noticed any obvious issues.
Also appears to resolve an issue where mower is enabled too early if gps drops/recovers before starting to mow
Fixes #52