Ability to toggle automatic point collection during recording

[jkaflik]

Draft as it might require further testing. Joy support is 99% broken.

Currently during area recording, mower_logic adds points if the distance to the previous point was below value 0.1. When trying to map a straight line, recording will add a bunch of lines - not necessarily pretty.

This PR introduces the ability to:

• disable automatic point collection
• manual trigger to collect point, so you can fully control your polygon
• changes to UI
  • support for enable/disable auto collection & add single point
  • add skip area button
  • minor UI changes

automatic point collection can be confusing. Any ideas for better naming?

Usage via MQTT:

• mqtt publish /action mower_logic:area_recording/start_recording
• build polygon using a standard way
• mqtt publish /action mower_logic:area_recording/auto_point_collecting_disable
• mqtt publish mower_logic:area_recording/collect_point for every point
• mqtt publish /action mower_logic:area_recording/auto_point_collecting_enable

(cherry picked from commit 4ba6c4d947c01d934872b98ff664bc211ef02e2f)

[ClemensElflein]

Currently the slicer algorithm (especially the switch-between-outlines) part of the algorithm advances a fixed number of points while switching from an inner outline to the next one.

This is done because otherwise, the robot would turn on the spot and try to drive outline_offset cm to the right and then do another turn to follow the path again.

If the points are sparse, then this advancing might cause the robot to skip many meters instead of just skipping a few cm. If unlucky, the robot will traverse diagonally through the lawn ignoring all obstacles.

I.e. before we can do sparse recording, we need to think about the algorithm. The outline swapping logic has some issues in complex outlines anyways and should be looked at.

[jkaflik]

@ClemensElflein I think I have noticed the behavior you mention here, but in general, haven't found any issues with sparse lines. Thanks for the explanation.

What if we feed the slicer and prefill the area with more dense points? https://github.com/ClemensElflein/open_mower_ros/blob/5ee2df56ea9c529506caec338e9e4f0488343f37/src/mower_logic/src/mower_logic/behaviors/MowingBehavior.cpp#L181 Would that work?

[jkaflik]

Today I observed mower behaviour when mowing the area with a few points only. I became more aware of the issue you explained.

I implemented additional logic that makes sure points are enough dense. Please have a look if that does make sense: https://github.com/ClemensElflein/open_mower_ros/pull/56/files#diff-117930a39435148fbbb0ff536dbdd8193034e11b507c640434a338e8ea635773R188

[jkaflik]

@ClemensElflein what do we do about it? I can change this PR to include ROS changes only. We can keep UI not modified and follow up.

[ClemensElflein]

Thank you for the updated function. The logic looks good to me. Does it work stable for you? If so, we can merge to main.

In the beginning you mentioned "Draft as it might require further testing. Joy support is 99% broken." is this still relevant or was this resolved?

[ClemensElflein]

Also regarding the app, does it work? If so I think we can also merge this. Is the default still "auto collection" or manual? We should do auto so that the docs don't need to change

[jkaflik]

I have this code running for a while in my mower. UI is a bit broken on mobile unfortunately :) I will not work on further app updates. Perhaps it's enough to merge.

[rovo89]

I will not work on further app updates.

Could you publish your fork of the app source code though? I would like to test these changes, but meanwhile the app had some other changes upstream.

[rovo89]

I rebased the three commits (except for the app) on main and tested them today. Used these commands to toggle manual mode and add points:

rostopic pub -1 /record_auto_point_collecting std_msgs/Bool false
rostopic pub -1 /record_auto_point_collecting std_msgs/Bool true

rostopic pub -1 /record_collect_point std_msgs/Bool true

Worked quite well! In one part of the map, a few points were missing, but I might have missed to execute the command or maybe had lost the connection or something. For that some feedback would be helpful - not sure if that would be possible with a service instead of messages?

Apart from that, I think it added the first point when I hit "Start recording". But didn't verify that later.

[rovo89]

Here's the (again rebased) branch in case someone else wants to test: https://github.com/rovo89/open_mower_ros/tree/manual_points

[rovo89]

Apart from that, I think it added the first point when I hit "Start recording". But didn't verify that later.

Verified that in the code and will work on a fix in the next days.

Apart from that, I'm wondering whether it would make sense to average the position of manual points over a few seconds, hoping that it will improve accuracy. Not sure though whether that makes sense and whether something like 5 or 10 seconds would be sufficient.

Finally, might consider mowing the dense point logic to slic3r_coverage_planner as suggested by @olliewalsh.

[rovo89]

@jkaflik published his changes here: https://github.com/jkaflik/OpenMowerApp/compare/feature/auto-point-collection I cherry-picked only those related to this PR on top of the latest main branch: https://github.com/ClemensElflein/OpenMowerApp/compare/main...rovo89:OpenMowerApp:manual_points

And updated https://github.com/rovo89/open_mower_ros/tree/manual_points with the compiled version. Will test this tomorrow if the weather agrees.

[rovo89]

Recorded a new area today - most as sparse points, non-straight parts with auto collection, so I toggled the new feature off and on several times and added points using the UI. It worked very well!

However, the plan looked like there were some small glitches in the outline. I'll probably have to check that in the simulator, maybe the function to add additional points has some error.

[rovo89]

I just spent the last 1.5 hours looking at the slicer, because I thought that the proper fix would be to just follow the polygon for a certain distance instead of skipping two points. I found the code already uses line.equally_spaced_points(scale_(0.1)), which should work for sparse polygons as well. Combining that with Point::nearest_point_index() as "safe" transition point, and checking +2 on the equally spaced points for a smoother transition should do the trick.

And we need the same logic also for obstacles.

[olliewalsh]

I just spent the last 1.5 hours looking at the slicer, because I thought that the proper fix would be to just follow the polygon for a certain distance instead of skipping two points. I found the code already uses line.equally_spaced_points(scale_(0.1)), which should work for sparse polygons as well. Combining that with Point::nearest_point_index() as "safe" transition point, and checking +2 on the equally spaced points for a smoother transition should do the trick.

And we need the same logic also for obstacles.

IIRC equally_spaced_points is a method on the the polyline i.e the polygon after it has been split

[rovo89]

Yes - but it's almost trivial to split that polyline (or the points array) again after finding the correct index.

[olliewalsh]

Even better would be to inspect the angle instead of just skipping ahead 2 points, as that isn't necessarily a good path

[rovo89]

Maybe - but I wouldn't really know what to do for that, and there are so many other improvements I'd like to work on in very limited time. Two points is only 20 cm, so I assume it can't go that much wrong?!?

[ClemensElflein]

As far as I remember equally_spaced_points does not interpolate and create points, it just drops points which are closer than whatever the parameter is. Naming of this function is weird. I think we should support sparse points in the map and interpolate inside of the coverage planner.

[rovo89]

As far as I remember equally_spaced_points does not interpolate and create points, it just drops points which are closer than whatever the parameter is.

Looks like it does interpolate: https://github.com/ClemensElflein/Slic3r/blob/026c1380e0ad12176dd2fc8cdf8f6f181deeb071/xs/src/libslic3r/Polyline.cpp#L122-L126

What it doesn't seem to do is ensuring that all original points are returned, so it might cut corners short. But the worst case would be that one point is 5 cm before the corner, so the next one is 5 cm after it, and I think the mower would drive a diagonal line between them. So it will create slightly "round corners" - not dramatic though.

Here's the PR based on my idea: https://github.com/ClemensElflein/slic3r_coverage_planner/pull/11

[ClemensElflein]

this is included in #137