Tool Path Planner Package Refactor

[marip8]

There are a number of issues with the current structure of the tool path planner package that make it difficult to use effectively. The maintainers of this repository have put together the following plan for improving the usability of the tool path planning package:

Issues with Current Implementation

• Not all planners work correctly (specifically the surface walk and eigen value edge planners)
• Planners of the same general type (i.e. raster, edge) don't produce consistent results
• Not flexible for expanding to new types of planners
• Lots of code duplication
• Duplication of parameters for planners
• Little documentation

Goals of Design Update

• Create architecture and development plan for repository
• Define expected behavior of planners
• Enforce expected behavior of planners
• Write meaningful unit tests
• Fix issues in planner operation
• Reduce code duplication
• Add new features for tool path generation
• Allow for easy configuration and usage of all planners
• Create GUI for configuring and using planners

Design Update Roadmap

• Define expectations and requirements for key interfaces
• Create drawings and diagrams detailing updated design
• Write updated code & packages, re-using and refactoring existing code where possible
• As components and functionality are ported to updated design, rewrite old code as wrappers around new code
• Mark old classes and functions as deprecated as each piece transitions
• Leave all deprecated code in place for a transitionary period (3, 6, or 12 months) after porting is finished
• Remove old code & packages

Architecture Diagram

Key Interfaces & Requirements

By describing the definition of raster plans, edge plans, and plan modifiers, we also create functional guidelines for what a RasterPlanner, EdgePlanner, and PlanModifier should do when implemented. These requirements should form the basis of the unit tests for each of these classes. These three interfaces can be used to encapsulate and organize much of the important functionality in tool_path_planner and path_sequence_planner.

Raster Plans

• Generally >1 raster
• Generally covers a full surface
• Lines are parallel
• Repeating pattern
• All points are on surface
• All points have same orientation
• Consistent ordering
  • Waypoints in rasters all go same direction
  • Segments are in order in the ToolPath
  • ToolPaths are in order in top-level structure

Edge Plans

• Returns list of closed loops
• Segments in sequential order in ToolPath
• ToolPaths ordered by length of closed loop (ex. Cartesian Distance, # of points)
• All loops start near some point (ex. all in top left corner)
• All loops process in same direction

Plan Modifier

• Takes in the output of a ToolPathPlanner (i.e. ToolPaths)
• Outputs the output of a ToolPathPlanner (i.e. ToolPaths)
• There should be a strong preference for constructing modifiers that have no additional effect when run repeatedly, as below. Since some modifications (such as adding an extra raster) will not meet this requirement, it will only apply to a subclass, OneTimeModifier
  • As an example: While a PlanModifier could be implemented via a function that reverses the direction of odd-indexed rasters, running that function twice would undo the desired changes. Instead, a more desirable PlanModifier might set the direction of every even-indexed raster to the direction of the zero-indexed raster, and set the direction of every odd-indexed raster to the opposite. This would result in no additional change if run multiple times.

[DavidMerzJr]

We'll be working on this redesign on my personal fork, here. There may be periodic merges at stable points in the design process - the completion of the initial interfaces, after particular portions of the existing code is ported over and have passed unit tests, etc.

[Levi-Armstrong]

@marip8 and @DavidMerzJr I recommend creating a feature branch under the main noether repository to have everything in one location. I imagine that you will be creating PR's and reviewing them as you implement the proposed plan and it would be nice to have this captured under the main repository, opposed to a personal fork. Let me know what you think.

[marip8]

I think the idea is that we would start the redesign on @DavidMerzJr 's fork to get the new interfaces and package structure set up, and then we'll make a pull request to the master branch to merge it into the repository (hopefully by the end of the week). At that point we can make separate pull requests from our own forks adding capability to the new package as normal.

[Levi-Armstrong]

That sounds good to me.

[dpiet]

We'd like to see full ROS2 support prioritized, or at least a ros2 branch in this repo that can merge bugfixes and new features that are going into main

[marip8]

Noted. Progress on this refactor has been slower than expected, but we're still pushing forward on it as we get projects to support it.

FWIW most of the new features/improvements added to this repository in the last 12-18 months have been to the ROS-agnostic components. If you're looking to use this repo in a ROS2 application today, I might suggest using those components (i.e., noether_gui, noether_tpp, noether_filtering) and building your own simple ROS2 wrapper. Here is some code where we've taken this approach using the new GUI interface that you could use.

I personally am in favor of removing all the ROS interfaces and letting users define their own for their specific application (as in the example above), but I can understand that it adds some value. The ROS2 components won't be added to this repository though; we'll probably set up a noether_ros and noether_ros2 repository in this org to support those components. I could stand up the ROS2 repo without much effort if you have specific contributions that you would like to make.

[marip8]

Following up from this comment from @liamcarlos

I do think that there are going to be a handful popular uses of Noether (which are seen in other ros-industrial repos) that could be packaged more project agnostic rather than ROS agnostic. It was a little difficult to swap out robots and controllers from other projects to just use their panels for generating meshes and tool path plans..

Can you be more specific here? I'm not sure I follow exactly. Part of the plan with this repo refactor is to both consolidate the conceptual aspects of tool path planning in a more intuitive way that supports better modularity for projects that use noether and separate the ROS interfaces to be able to support multiple OS platforms and ROS versions more easily. Hopefully the changes we're making address your concerns, but if there is something specific we should be thinking about we may be able to address it.

[gavanderhoorn]

Could I perhaps suggest to already split out the noether_msgs package and host it in a new repository?

I'm building ros1_bridge instances with support for noether_msgs and the fact it's part of a repository with more than just that package makes it more of a hassle than I believe it absolutely needs to be :)

Having the messages in a separate repository would help with lightweight Noether interfacing in any case, and the fact it makes building just the messages easier is a nice additional benefit.

Thoughts?

[marip8]

Are there specific messages you are using that should be in this new repository? Most of them are obsolete with the current state of the refactor, and I would like to remove (or not port) them if possible. I think only the tool path message definitions and maybe the tool path planning action (which should probably be a service) would still be relevant in a new noether_ros repository