Skip to content

Conversation

@DavidSpielman
Copy link
Contributor

This PR addresses a bug in the PlanToolPathServiceNode Class in snp_bt_ros_nodes.cpp where if toolpaths are present in the service request (!response->tool_paths.empty()) but the toolpaths within tool_paths are empty (response->tool_paths.tool_paths.empty() == true) the behavior tree node will succeed, it will publish an empty toolpath, and it will not provide meaningful feedback to the user.

The following has been added as part of this PR to address this:

  • Added additional checks for partially empty toolpaths
  • Exposed warning messages for BT::NodeStatus::SUCCESS in the text_edit_logger

When a toolpath is partially empty (ex: when two regions are circled with the Polygon Selection Tool and the ROISelection mesh modifier is applied, and one region contains a toolpath and the other region contains an empty toolpath) the behavior tree node will return BT::NodeStatus::SUCCESS, generate the nonempty toolpath, and warn the user in the text_edit_logger that there are other toolpaths that may be empty. This allows the user to more easily see which toolpaths succeeded in generating and which did not.

See below for an example from the text_edit_logger:
Screenshot from 2025-11-25 14-56-37

Warning messages can also be added to accompany BT::NodeStatus::SUCCESS messages to give the developer the ability to generate warnings for BT Nodes that have succeeded but may contain unexpected output that is not severe enough to return a BT::NodeStatus::FAILURE.

* Exposed warning messages for BT::NodeStatus::SUCCESS in text_edit_logger
@marip8
Copy link
Member

marip8 commented Nov 26, 2025

I'm on board with checking that at least one of the tool paths is not empty and returning an error if they're all empty

Wrt the warning message, what value does this provide if we're okay with some tool paths being empty but not all? It seems like it's more directly communicated in the the tool path display. If the user sees some chunk is missing, then they would be able decide whether or not they want to proceed.

If we think that any empty tool path is an error, then I would rather just make the whole BT node return failure and communicate it with the existing infrastructure

@DavidSpielman
Copy link
Contributor Author

DavidSpielman commented Nov 26, 2025

Wrt the warning message, what value does this provide if we're okay with some tool paths being empty but not all? It seems like it's more directly communicated in the the tool path display. If the user sees some chunk is missing, then they would be able decide whether or not they want to proceed.

In my view, the warning message clarifies that the missing chunk may be an error. Without the warning message in this PR, but including the other checks implemented in this PR, the BT Node will succeed because some tool paths are generated. It will not report to the user that there was an error in the tool path pipeline even though a requested tool path failed to generate.

If a user has selected multiple regions that are close together, and the tool path waypoint spacing is dense, it is possible that they may overlook that there was a region that did not generate tool paths inside, and they may only realize this when motion planning which does not feel as efficient.

If the user drew a polygon accidentally during selection and tried to generate the tool paths, this may save the user from redrawing polygons.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants