Proposed rename of geometry_msgs/PoseStampedArray to nav_msgs/Goals

[tfoote]

We recently merged a PR to add geometry_msgs/PoseStampedArray #262 however I'd like to propose renaming that because I believe that it doesn't carry the necessary semantic information appropriate reuse in the ecosystem. And the PoseStampedArray was already discussed in ros/common_msgs#144

Why should we have Strong Typing

As background early in the ROS ecosystem we created a bunch of basic datatypes of what seemed like they would be useful datatypes. However over the years we learned that these generic datatypes that have a structure but not semantic meaning can actually be counter productive in effective reuse. Counterintuitively by reusing the same datatype for different semantic information we break the systems ability to help the user by validating datatypes match. An example of this is that we originally thought that we could use the https://github.com/ros/std_msgs/blob/kinetic-devel/msg/Float32MultiArray.msg for capturing point clouds. There's a really neat way to define the layout and it's completely generic: https://github.com/ros/std_msgs/blob/kinetic-devel/msg/MultiArrayLayout.msg However with this you now have the problem that you potentially have incompatible versions of MultiArray datatypes running around on the system and it's unclear if any given node will know how to process a MultiArray message because it could be an Image or it could be a PointCloud. They both pack into a MultiArray datatype, and just have some different layout settings. And this means that you can accidentally connect the point cloud source to your image processing pipeline. And vice versa for the camera and the point cloud processing pipeline. But with the distinct datatypes we can be confident that image sources and image processing pipelines will connect and be able to process the data without a runtime error letting you know that the data is incompatible because of the detected layout fields. In that case the layout is at least enough to detect at runtime that the datatypes are incompatible, however an even more pernicious error is if the datatypes are indistinguishable at runtime. For example if we were to have a range sensor and a force sensor on the front bumper both producing float32 values, one in meters, one in newtons. If the configuration accidentially connects these two sensors to the wrong topic on an emergency stop, you can get a robot that will think that it's hitting free space, and that when it has hit something that there is free space ahead. With dedicated types this miss-configuration is impossible. And furthermore with dedicated types, rviz can have semantically meaningful displays for each datatype, showing a ranging cone for the Range measurement automatically, and a symbol for the Force feedback which is proportional to the force applied. Without the datatypes it would be necessary to configure this for each topic with ambiguous datatypes and that's just another place for potential miss-configuration.

Looking at geometry_msgs/PoseStampedArray

Looking at geometry_msgs/PoseStampedArray it is field for field identical to the already existing nav_msgs/Path and as argued in #262 nav_msgs/Path does not have the appropriate semantics to capture the information needed for the nav2 stack. That's completely valid and I think we should support this, however we should be making another semantically specific datatype so that we can again know if we're veering into new territory and the new scope will break existing assumptions.

To this end I would like to propose that we rename the message and provide clear documentation in the message of what it represents and what it doesn't so that it can be unambiguously interpreted by any subscriber.

I've spent some time reviewing the PR to merge this into nav2 ros-navigation/navigation2#4791

It's clear that this datatype is being used to represent goals: https://github.com/ros-navigation/navigation2/pull/4791/files#diff-81bfc646583fcbc8bc1e8478251bb41be1fcff7499e5fccfef817c7d25b7d23cL34

This the name should probably reflect that it is indtended to represent goals.

To that end I would like to propose that we rename it to nav_msgs/Goals which will sit as a peer to nav_msgs/Plan

And we should be filling in comments and documentation to reflechttps://github.com/ros2/common_interfaces/blob/rolling/nav_msgs/srv/GetPlan.srvt this usage.

I have the following outstanding questions about the intent of this message that I'd like to add documentation to the message to cover:

• Is there meaning to the header.frame_id or should it be always blank?
• What is the meaning of the ordering of the array entries?
  • I see a reference to via points being removed which suggests there is an ordering.
  • This is getting into how this is semantically different than the nav_msgs/Path
• What is the meaning of the timestamp on the individual Poses?
  • Is that the time at which it should be traversed? Or is that the time at which it was computed?
  • What validation should be applied to the timestamps?

With respect to the structure:

• If this is capturing goals should we also be capturing tolerances? In space, in orientation, in time? We have this sort of thing in https://github.com/ros2/common_interfaces/blob/rolling/nav_msgs/srv/GetPlan.srv
• As a minor additional I think it would be best to replace the subfield poses with goals so that when you iterate it's more like for goal in mymsg.goals: but that's less important than the top level naming.

I would love to get some feedback from @SteveMacenski and @tonynajjar on how it's being used in practice so we can make sure to capture their full expectations of the semantic datatypes and provide that for future implementers too.

[SteveMacenski]

Nice, you caught me in my late Friday afternoon session of community work, so I can give you fast thoughts now!

Why should we have Strong Typing

💯

I believe that it doesn't carry the necessary semantic information appropriate reuse in the ecosystem.
To that end I would like to propose that we rename it to nav_msgs/Goals which will sit as a peer to nav_msgs/Plan

I don't object on the face of it. However, I think that PoseStampedArray is a core type that is missing from geometry_msgs. There's PoseArray already, this is a natural extension. So IMO PoseStampedArray regardless of nav_msgs/Goals should stay as a useful type.

Adding an additional nav_msgs/Goals with more semantic information I think is OK.

Is there meaning to the header.frame_id or should it be always blank?

We use the stamp for when the goals were computed - as a way to both (a) version control the sets of poses and (b) as the timestamp all goals should be transformed w.r.t. if the individual goal poses dont have their headers populated. While the individual poses themselves have timestamps that may be semantically meaningful for transformations, they may or may not be always populated if they're all the same.

The frame_id I think is useful when the individual poses don't have their frame_id's populated as a fallback (as is commonly done today in the Path msgs, the stamps are typically blank). But I agree in a 'normal' or 'good' system, this shouldn't be necessary. However the header is such a ubiquitous standard, I think its worth keeping for both.

One could imagine most applications would have their goals all in one frame (map, odom, etc), though others may have goals in different frames for some application-specific reason. Like nav_msgs/Path, I think it would be flexibly used -- though not strongly typed.

What is the meaning of the ordering of the array entries?

The order to execute the goals. I suppose we could add a bool for whether or not they're ordered so that an application knows to treat them as a requested-order or a set of goals to accomplish at its choice of ordering. I don't think that is strictly necessary, but an idea.

What is the meaning of the timestamp on the individual Poses?

Curently, the stamp of the goal's computation for use in transformations. This could differ between goals for accurate transform synchronization when goal frames are moving wrt to the robot.

I think its within good conversation to identify if we want stamps also associated with goal achievement (i.e. the time we want this goal completed at). If we're making a Goal message, maybe we add this.

If this is capturing goals should we also be capturing tolerances? In space, in orientation, in time? We have this sort of thing in https://github.com/ros2/common_interfaces/blob/rolling/nav_msgs/srv/GetPlan.srv

Note that we don't use GetPlan in Nav2 - even in move_base it seemed to me the service exposed to get a path was a a 'hack' to get around the system due to move_base's limitations. I don't see any modern project currently using this interface on a ~5 minute google search. I think nav_msgs needs some general improvements / deprecation, but that's another discussion for another time.

I don't know if we can specify tolerances in this manner and be totally portable to all possible use-cases. For example in Nav2, we don't specify any tolerances at the Navigation level, just the pose(s) and behavior to execute. The behavior would contain any appropriate tolerancing. That could be implemented at a BT/FSM level, the planner algorithm level, or the planning server level. The metric could also be dynamic on environment conditions or static per-request as is most basic.

So, I don't have a good answer here. I tend to air on the side of not including it since I can't find a modern example in (Nav2, move base flex, MRPT) whereas it is discussing goals. I can see in a few places path planning tolerances are mentioned, but its with respect to individual planning requests that a nav_msgs/Goals message might be broken down into in the navigation system by its own means. If I can't think of the appropriate field, I would think we'd need a new PlanningTolerance message, which opens a whole new topic 😄

MRPT's version: https://github.com/mrpt-ros-pkg/mrpt_navigation/blob/ros2/mrpt_nav_interfaces/srv/MakePlanTo.srv
Nav2's version: https://github.com/ros-navigation/navigation2/blob/main/nav2_msgs/action/NavigateToPose.action#L2
move base flex version: https://github.com/naturerobots/move_base_flex/blob/master/mbf_msgs/action/MoveBase.action

As a minor additional I think it would be best to replace the subfield poses with goals so that when you iterate it's more like for goal in mymsg.goals: but that's less important than the top level naming.

For the context of nav_msgs/Goals, I agree.

[tfoote]

Thanks for the detailed response @SteveMacenski I'm glad the timing worked out well. I've created #269 to followup.

I've done the minimal change such that doing the migration in nav2 should be minimal.

In particular, I really focused on just the direct renames, and adding stronger comments/documentation into the message to capture your explanations as stated above. I rewored them to be more declarative so please take a close look that I captured your intent correctly.

I think that considering unordered goals might be interesting, but we can leave that for a later discussion. And likewise the tolerances are likely another ball of wax and will change the scope of this so we can defer that. But thanks for the pointers.

And I'd also be happy to consider another pass at nav_msgs and consider a refresh. If no one is using messages or services we should at least clearly deprecate them and point people to what is actually demonstrated as useful.

I don't object on the face of it. However, I think that PoseStampedArray is a core type that is missing from geometry_msgs. There's PoseArray already, this is a natural extension. So IMO PoseStampedArray regardless of nav_msgs/Goals should stay as a useful type.

I need to reply specifically to this one. It is a potentially useful type, but as you can see above there's a lot of ambiguities as to how to interpret it even if it's only for one specific use case. And although it's nice to feel completionist about putting in datatypes to the core like this. If we put things in that are not well defined it's actually detrimental to the community and ecosystem.

Creating a new message is very easy. And we want to have people take the time to generate the semantically meaningful variants rather than just reuse an existing loosely defiend type for a different purpose. The fact that both Path and Goals would fit into the same data type, but that they're not interchangeable, shows that this is too generic. And we don't want to cause confusion in the future when the 3rd and 4th use case for this generic datatype end up colliding.

I agree that being completionist on the datatypes feels nice, however it's been very clear in various places where we over did it, such as std_msgs and we're working to roll it back, but it's almost impossible to roll it back once it's in use already, so I work reallly hard to make sure that we don't add more messages that are too generic to the core.

[jrutgeer]

There's a recent RSE question which shows a use case for a PoseStampedArray message. The question can be summarized as follows:

• The Gazebo Sim PosePublisher plugin outputs a pose_V message, which is a vector of stamped poses,
• The ros_gz_bridge bridges this into a pose_array (i.e. non-stamped poses),
• So on the ROS side, you lose the info about which pose relates to which object,
• How to deal with this?

The 'best answer' is probably that the ros_gz_bridge also supports bridging a pose_V message into a tf2_msgs/msg/TFMessage which is an array of stamped transforms.

However, this is an inconsistency: in ROS 2 it has been chosen to make a distinction between a pose and a transform, so it is inconsistent to bridge pose into tranform, and a PoseStampedArray type would be more appropriate.

Obviously, one can argue that a generic PoseStampedArray is too vaguely defined and a specific message (e.g. SimulationObjectPoses) would be more appropriate. But there's a cost in defining new message types, so it always is a trade-off whether that cost is outweighed by the potential benefits.