adding IDs to geometry_msgs/Polygon, PolygonStamped

[SteveMacenski]

Implementing #230

Polygons are often used to represent specific objects (e.g. robot footprints, obstacles in the scene, other agents) but are difficult to rectify currently without any kind of specific identification - especially when you have N of the same object resulting in several identical object instances.

This PR adds an ID field analog to visualization_msgs/Marker to disambiguate polygons for use in the nav2_collision_monitor and throughout future Costmap2D planned work. I strongly believe this is a good addition worth the change to a standard interface with both minimal overhead & not breaking any current users' behavior.

[peci1]

Is this really needed? Wouldn't it be better to add a PolygonCollision to nav_msgs? I always understood the geometry_msgs as very general. E.g. I have a node that publishes the supporting polygon of the robot. What would the ID be in that case?

And I'm strongly against adding ID to the non-stamped polygon. Can't you actually use Frame ID in the stamped polygon as ID?

[SteveMacenski]

Polygons are used to represent numerous things that having some kind of ID would be useful for.

Examples:

• N homogenous robots in a room, representing each in the global frame wouldn't tell me which polygon belongs to what robot (collision use; non collision use; fleet use; ...)
• I have a bunch of fields I want to plow and/or regions I want to represent in a warehouse. I want to be able to know which field / zone this polygon belongs to correlate some metadata
• I want to represent some virtual information like sensor frustum bounds of many instances of the same sensor in the global frame.
• Something something, detection box polygons from AI 😉

I disagree that this should be some navigation specific message type or particular to collision. I think the general polygon message is missing key information to be able to distinguish between polygons sharing the same frame_id. Other messages in geometry_msgs have unique frame IDs as unique sensor source interfaces or are not application-wise necessary to distinguish them. Polygons are more generic and do, I feel, require some kind of information to distinguish them from one another for metadata correlation reasons in many unique applications. Its also super small overhead and doesn't break anyone's current behavior.

Edit: I think that is one of the motivating reasons why visualization_msgs/Marker has an ID field: the desire to be able to distinguish multiple points from each other in the same global reference frame. I think Polygon as a message contains that same core need - in fact you run into precisely the same issue in that case representing a polygon with Marker messages.

[peci1]

• I have a bunch of fields I want to plow and/or regions I want to represent in a warehouse. I want to be able to know which field / zone this polygon belongs to correlate some metadata

Ehh, really should this be supported by a core message type? This sounds to me like a cartesian version of geographic_msgs.

• Something something, detection box polygons from AI 😉

Sounds more like vision_msgs.

• N homogenous robots in a room, representing each in the global frame wouldn't tell me which polygon belongs to what robot (collision use; non collision use; fleet use; ...)

• I want to represent some virtual information like sensor frustum bounds of many instances of the same sensor in the global frame.

These two seem equivalent. Usually each robot/sensor will have its own topic or frame_id, so you could disambiguate based on that. If you're building a list part by part by listening for single elements on a topic, why couldn't the elements come in their original (unique) frame and you couldn't convert them inside the node?

I think that is one of the motivating reasons why visualization_msgs/Marker has an ID field

But Marker is a special-purpose message type, while Polygon is a general-purpose message type. Marker's purpose dictates that having an id is essential. You can't say that generally about polygons.

Regarding type of the id field:

• Discussion about UUIDs ros-perception/vision_msgs#47 settled on string
• Message format for RadarTrack ros-perception/radar_msgs#3 settled on uuid

Do you really want to introduce yet another id type? :)

---

All the thinking around leads me to proposing a new message type instead of editing the existing one. I'm not sure about the name, but something along PolygonWithIdentity, which would correspond to your proposed edit of PolygonStamped. A small helper library could be provided that would automatically take original PolygonStamped and add the id based e.g. on the topic name, or that would take the local polygon, transform it to global frame and assign an ID based on the local frame.

[SteveMacenski]

Usually each robot/sensor will have its own topic or frame_id, so you could disambiguate based on that.

Not when they're already in the global frame (or base frame, etc). That's the exact need here, to disambiguate with many instances of similar / same polygons. The number of applications of that are numerous

I'm not sure about the name, but something along PolygonWithIdentity

I personally think that's overkill for what complexity we're talking about, but I wouldn't object to that. The biggest thing is that this is a generic problem that needs a generic geometry_msgs solution (I feel) so that it can be leveraged in many places. I don't think its a strict need that it is in precisely Polygon base message.

How about PolygonInstance and PolygonInstanceStamped?

[peci1]

How about PolygonInstance and PolygonInstanceStamped?

These names sound good to me ;)

[SteveMacenski]

Maintainers et al of this repository: Any objection to this course of action?

[clalancette]

Maintainers et al of this repository: Any objection to this course of action?

We discussed this at the triage meeting this week, and we think this is a fine course of action. We do have a couple of comments and questions:

1. It seems like the ID would be better as a uint64. That way you could use e.g. nanosecond timestamps as the ID if you wished. Another alternative would be a string (at which point you could put a full UUID in there), but that may be overkill for what you want.
2. Speaking of the ID, do you have a way to generate it in mind? In other words, what are you expecting to put in that field?

[SteveMacenski]

@clalancette uint64 sounds good, I'll make that update when I make the new PolygonInstance and PolygonInstanceStamped messages. I'll get on that once its all settled, I suppose just your next question and we're good to go?

do you have a way to generate it in mind?

For any particular application, there's a way to do this. So if the polygons are robots, using a serial number seems reasonable. If this is the output of AI or some other thing, UUID generated from the tracking software seems reasonable. That to me is a potential argument for using a string, but I don't feel strongly about it. Correlating a UUID to a ID from a tracker isn't difficult.

The difficulty would come if you had N publishers of these PolygonInstance messages that don't know about each other and have their own index-from-0 schema for generating IDs. When based on something physical or annotated, then the numbering schema from serial numbers or something makes sense, but if each publisher is generating virtual polygons, UUID is the most reasonable way to go to avoid conflicts. I don't think having this field as a UUID is required, but a string would do that job.

I leave that choice of string vs uint64 to maintainers. I lean slightly towards string personally for the above reasons, but using int IDs has heritage in other messages and I do value consistency

[clalancette]

@clalancette uint64 sounds good, I'll make that update when I make the new PolygonInstance and PolygonInstanceStamped messages.

I think uint64 is good enough; if we need the additional resolution that an e.g. string would give us, we can get that in different ways (separate topics, a higher-level message that includes this message + robot_id, etc).

[SteveMacenski]

@clalancette done :-)

[sloretz]

Polygons are often used to represent specific objects (e.g. robot footprints, obstacles in the scene, other agents) but are difficult to rectify currently without any kind of specific identification - especially when you have N of the same object resulting in several identical object instances.

It sounds like there's support for adding the ID to the message. I disagree a little about that. It seems like an id isn't needed to define a polygon. I would instead recommend a higher level message type for a robot footprint, or an obstacle, or an agent such that it has both an id and a polygon.

[SteveMacenski]

What is a 'standard message' but a message that can be used to describe common and innumerable-situation things for many applications. I think a way to identify a shape against other shapes in the same global frame has enough independent applications that this is worthwhile. Especially when many different types of representations that can be generalized to this (agent, obstacle, footprint) have some relationship that some applications may want to ingest all of them and don't need to specifically differentiate between the semantics of each of those 3 example types, as much as it just needs to be able to disambiguate between instances as updates are made.

I think strongly that this is very relevant and I have ~3 places within Nav2 and broader tooling around it that this would get use within a couple of months and one that would be used immediately. Polygon still exists, these are now new PolygonInstance messages in addition to. I don't think any other messages in geometry_msgs would benefit from IDs the way that Polygons specifically would

[peci1]

It sounds like there's support for adding the ID to the message. I disagree a little about that. It seems like an id isn't needed to define a polygon. I would instead recommend a higher level message type for a robot footprint, or an obstacle, or an agent such that it has both an id and a polygon.

I thought the consensus was to add a new PolygonInstance message type.

[SteveMacenski]

It is/was.

@clalancette @peci1 can you rereview? This is blocking a user ros-navigation/navigation2#3885

[peci1]

You might want to squash the commits so that unrelated files are not changed by this PR.

Otherwise LGTM!

[SteveMacenski]

Squash merge should handle :-)

Great!

[SteveMacenski]

Done!

[SteveMacenski]

Failure looks like networking related, not PR related

[SteveMacenski]

Quibble away, my friend 😉

[mjcarroll]

I'm happy with this, it has been sufficiently bike-shedded.

[SteveMacenski]

Got a few +1’s, can we merge this?

[clalancette]

Two really small changes to the wording, then I'll go ahead and run CI on this.

[clalancette]

CI:

• Linux
• Linux-aarch64
• Windows

[SteveMacenski]

I dont believe the windows linting is from this

[clalancette]

I dont believe the windows linting is from this

No, it's not. It timed out in rclcpp, which is likely due to ros2/rclcpp#2303 . We'll deal with that separately.

[ros-discourse]

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros-2-jazzy-jalisco-released/37862/1