Message format for RadarTrack

[ghost]

Proposed message format for RadarTrack

Prior to proposing this format, research was done into the radar data message formats that exist out in the wild. The details of this analysis can be found here: https://github.com/radarAaron/radar_msgs/blob/master/ROS%20Message%20format%20comparison.xlsx

Summary

Position in 3D Space
All position appears to be represented in Cartesian coordinates, there is no consensus as to the format however or how to represent uncertainty.

The following formats were used:

• geometry_msgs/Polygon (no uncertainty)
• geometry_msgs/PoseWithCovariance
• x,y coordinates [also has a second message type which use distance, bearing] (no uncertainty)
• x,y,z (no uncertainty)

Velocity
There is no standard for velocity. All formats could be converted to Vector3 however.

The following formats were used:

• geometry_msgs/Vector3 (no uncertainty)
• geometry_msgs/TwistWithCovariance (covariance)
• x,y velocities (no uncertainty)
• float32 speed (no uncertainty)

Acceleration
Two drivers output acceleration.

• geometry_msgs/Vector3 linear_acceleration
• geometry_msgs/AccelWithCovariance

Size
There is no standardization on object size.

• geometry_msgs/Polygon (no uncertainty)
• length, width (no uncertainty)
• Horizontal and vertical cross section (no uncertainty)

Classification
Only one format outputs any form of classification

Additional fields
The following fields are also present in some formats: orientation_angle, msg counter, target_format_type, quality, speed_trigger_flag, Lane, Track_packet_counter

Conclusion

• Using the Cartesian coordinates system appears to be most standard.

• Velocity could be reconciled into a Vector3 type with the velocity components which are not supported by a driver being set to zero.

• Acceleration is probably best represented as a Vector3

There is no consensus about how the size of a shape is reported, with the only option being RCS. The call was made to represent 'size' using an ovaloid shape based on the distribution of points in 3D space.

• Only one driver outputs classification but this is still probably a useful addition to the standard format.

• It is unclear how uncertainty should be presented

Notes

Object tracks are based on usually based on a point cloud. The point cloud shape is somewhat object specific and does not necessarily cover the full size of the object (eg. points tend to concentrate towards the center). The 'size' format chosen here is based on the distribution of points in 3D space. An alternative approach for expressing the size would be as a covariance matrix.

Questions

• Should size be expressed as 'size' or as a covariance matrix?
• Is expressing the uncertainty of the velocity and acceleration components important?

[tfoote]

Should size be expressed as 'size' or as a covariance matrix?

What is the underlying measurement that's trying to be expressed here? If it's a clustering of points then a size vector of the bounding box makes sense. The alignment of the bounding box should be explicit too. Covariance is semantically very different.

Is expressing the uncertainty of the velocity and acceleration components important?

Uncertainty is probably worth expressing. Since these are the results of an estimator there should exist some estimates of the uncertainty.

In the recent message API review it was determined that it would be valuable to improve our support for covariance based uncertainty. https://github.com/ros2/common_interfaces/blob/master/docs/APIReviewFoxy.md A new 3x3 Upper Triangle covariance representation would go well here for these measurements.

[tfoote]

Without the header the RadarTrack isn't really useful on it's own so the RadarTracks is a little bit odd that the smallest unit you can subscribe to would be plural. This is presumably some collection of tracks segregated by something like time. I'd suggest considering if there's a better name but could be convinced otherwise.

[RonaldEnsing]

How radar specific is this message type? What about a generic (sensor agnostic) track msg?

[gavanderhoorn]

I'm only an interested external observer (with no experience with radar or radars), but would it be helpful to design these messages with a bit more of a "future radar" in mind, next to the devices (and drivers) which exist today and which have been investigated?

Future devices might be significantly improved, making this:

UUID seems like a complete overkill for a numbering system that will seldomly go above 100 and usually a lot less.

a statement which may prove to become untrue in the (near?) future (obligatory reference to "640KB ought to be enough for everybody").

UUID also implies numbers will be universally unique. I suspect most drivers will output an integer ID and the numbering will reset when the module does. ID at least does not imply the numbers are universal unique

re: drivers have internal IDs which are not globally unique: this is most likely true, but there could still be value in using a UUID as it would avoid conflicting IDs between drivers (both multiple instances of the same driver/sensor and multiple different sensors each with their own driver). Consumers of "radar messages" in general could then rely on the fact that no two detections would ever get the same ID, which seems to greatly simplify bookkeeping for such consumers.

A similar discussion took place over in vision_msgs: ros-perception/vision_msgs#25, ros-perception/vision_msgs#18 and ros-perception/vision_msgs#19.

The question there was whether a globally unique ID should be included in all messages (also detections) and whether it should be a uuid_msgs/UniqueID or whether a string should be used instead.

As @RonaldEnsing writes above, there seem to be quite some parallels between "detections" here and in vision_msgs. Perhaps the discussion there could be helpful here.

[SteveMacenski]

How radar specific is this message type? What about a generic (sensor agnostic) track msg?

@RonaldEnsing agreed, this could at some point be moved into a more generic tracking messages. You're about 2 steps ahead of us. Vision messages is vision specific and unfortunately some of the assumptions they made don't really fit outside of pointcloud processing for 3D items. We'll likely make a detection_msgs or tracking_msgs package that handles generic detections/tracks based on the lessons learned from this. We also need to suppose more generic detection types in navigation2, but we're a bit away from that still. First step is radar, second step is everything else.

between drivers (both multiple instances of the same driver/sensor and multiple different sensors each with their own driver).

Excellent point, in fact that would only mean UUID is the way to go because otherwise you can't reasonably promise uniqueness of the tracks.

[ghost]

@SteveMacenski I'm happy to merge this request as is. Any last suggestions before I do?

[SteveMacenski]

@peci1 @tfoote @gavanderhoorn Can you review again and let us know your thoughts / approvals?

We need to get general buy in @radarAaron so we need to have multiple approvals to merge

[peci1]

I generally agree with all unresolved @tfoote's comments.

Utilizing RadarDetection type here and only adding id, acceleration and classification would make sense to me. Or, rather check my understanding: the track is created by a process that takes a detection, finds its corresponding track, and updates the track with data from the new detection. This basically creates something that could be described as "the detection that best suits the observations", or "if I had an oracle, I'd see the object exactly as this detection". The use of RadarDetection would help making this clearer from the Track definition. On the other hand, it might (wrongly) imply to someone that the exact values stored in the RadarDetection subfield are some actual data measured by the radar at some time (which would be wrong because the data are the result of a tracker/estimator).

I vote for adding covariances everywhere where it makes sense - as @gavanderhoorn stated, we should look into the future. If some vendor doesn't want to compute this information, let's allow them to publish an invalid (zero) covariance. It should be explicitly stated in a comment that downstream code should be prepared for the case of zero covariances.

The size field is still pretty vague. Is it an axis-aligned bounding box? Or is it an oriented bounding box? But that would require an orientation field, and it should also specify whether it is a minimal volume OBB or of its main axes are determined by PCA or whatever... Or is it an oriented bounding box that's always facing the radar frame? The last case would probably make sense given the method how radars work, but then the size would change as the object would be moving around the radar... I'd vote for the PCA-derived OBB or bounding ellipsoid.

I see that the previous paragraph about the size field is quite a stream of thoughts, and I hope you'll be able to extract ideas from it. Maybe a good start would be to agree on the desired properties of the size. Should it remain unchanged when the object/radar moves around (given that the object has already been seen from all sides so that its 3D size can be correctly estimated)? Or is it okay if it "fluctuates", e.g. the way that happens when you rotate an irregular object and observe its axis-aligned bounding box? And, again, even the size field should implement some kind of uncertainty or covariance (whatever it means for boxes). To bring in more complications, an OBB can have uncertainty in its dimensions, orientation and position, and all three of them are probably correlated...

Another thought is about the position, velocity and acceleration fields - they're all missing the rotational components. Going back to @gavanderhoorn's comment, isn't it (theoretically) possible that some future radars would be able to return e.g. rotational velocity of the object (and thus its rotation and angular acceleration would also be available)? Again, not all radars would have to support it - all unsupported "fields" would be set to zero.

And the very last thought is inspired by the camera info messages. It is apparent that each radar (and its driver) has a different set of capabilities. I think it might be useful for downstream code if it could determine the "capabilities" of the driver. For me, it'd be important to know if the acceleration is zero or unobservable. So I suggest creating a RadarInfo message that could be published latched on a defined topic and would apply to all radar track messages published after this info message (this would be inconsistent with CameraInfo, but it seems more appropriate to me here as the capabilities would be constant opposed to camera intrinsics which can change over time). This message would describe the capabilities using boolean flags, e.g. acceleration_available, or even more granular acceleration_x_available etc. It could also contain some textual descriptions of the classification values, which would nicely bridge it back to the originally proposed string representation. Alternatively, no info message would have to be created if there were a convention that all "unavailable" fields would be set to NaN. I know working with NaNs is tricky, but at least you'd see that your computations rely on unobserved values.

[SteveMacenski]

That's a long post - can you submit a specific recommended change in the form of a suggested message(s)?

[peci1]

As I don't work with radars, I don't know the capabilities they have now or could gain in the future, and thus I can't decide whether my requests are worth it or not. If all of them were valid, we'd end up with something like:

RadarDetection:

geometry_msgs/PoseWithCovariance position
geometry_msgs/TwistWithCovariance velocity
geometry_msgs/Vector3 bounding_box_size
float64[9] bounding_box_size_uncertainty  # something like covariance
geometry_msgs/Pose bounding_box_pose

RadarTrack:

uuid_msgs/UniqueID uuid
radar_msgs/RadarDetection detection
geometry_msgs/AccelWithCovariance acceleration
uint8 classification

RadarCapabilities:

string frame_id  # to specify which radar does it describe? maybe not needed?
bool three_dimensional # false would automatically "disable" all 3rd dimension fields
bool orientation_available
bool angular_velocity_available
bool linear_acceleration_available
bool angular_acceleration_available
... # mayb more
string[] classification_categories # index in list corresponds to classification value

But maybe would, in the end, need different definition of size for detection (2D) and for track (2D or 3D), and that would prevent using RadarDetection as the subfield.

It could also make sense to not specify orientation in RadarDetection because it's not clear to me ATM what should it mean. But a track can have it, e.g. relative orientation of the object to its first detection...

Also, for radars working only in 2D, the orientation as a quaternion would not make sense...

This suggestion needs more thinking and polishing, I just wanted to show the ideas. I'm not insisting on any of them (but not adding covariances would need more convincing).

[peci1]

Maybe the whole Radar* API could be duplicated - once with covariances, once without them. As is Pose and PoseWithCovariance. That would allow vendors to start with simple and smaller messages and as they progress with development, covariances could be added still using a standard message. It's a pity that ROS 1 doesn't support memory-efficient optional message fields...

Also, I think it's insufficient to review the implementations of current ROS drivers to determine whether covariances are widely used or not. For that, it seems to me more suitable to look at the low-level drivers of the radars and look there if some covariances can be read out or not (because ROS drivers are often just wrappers around some custom low-level drivers, and the low-level driver might be much more versatile than the ROS wrapper).

[SteveMacenski]

I don't think anyone except @radarAaron is arguing to not have covariances. I think they should be outright added and we can move on with this PR.

On your Capabilities message; I'm deriving info from this based on the IMU and PC2 messages. The IMU has a bunch of fields that some vendors don't provide (orientation, specifically) and when its not implemented, its on the users to know that and not use tools the depend on it. I think that's no different here. If you don't fill out the covariance, that's on the end user to see and deal with appropriately. If it truly isn't common to use it, we can remove it later or applications just won't be built using it.

There are plenty enough drivers and resources that exist to make an intelligent judgement call on. Aaron and other people in these discussions are also radar users and manufacturers. Making a statement like think it's insufficient to review the implementations of current ROS drivers to determine whether covariances are widely used or not. is not productive and will largely set back our efforts to at least do something. I believe we have as much experience and knowledge on radars as could be gathered in these discussions already.

[ghost]

@peci1 Radar does not provide a high resolution of information. I don't think it will ever be possible to get the rotational velocity etc out of radar data. Radar can identify where there is a high probability of an object and its rough size and direction. The data is more of a point cloud of data which you have to make inferences about.

In terms of the size and coordinates: These is intended to the frame of reference of the radar sensor. Oriented Bounding Box using something like Principal Component Analysis is really something that makes a lot of sense in terms of radar data, the data is just not suited to that kind of analysis, and is therefore not something that should be part of a common message format. The RadarPoint message format is available to anyone who wants to try and implement such methods.

This RadarTrack message will come out of some higher-level algorithm on the radar hardware (something like a Kalman filter with k-means clustering) but that algorithm will be vendor specific and I expect there to be a lot of variation in what each piece of hardware is capable of, what the vendors implement, and what makes sense for a specific application, to the point where I think the idiosyncrasies are best explained the modules manual and that a trying to condense these down into a standardized message not really doable.

[ghost]

@SteveMacenski I've added a covariance field to the message type. Does what I've added make sense from an ROS perspective? If not can you suggest an alternative. As we have mentioned previously, I certainly wouldn't expect this to be populated by many vendors.

[SteveMacenski]

See IMU message: http://docs.ros.org/melodic/api/sensor_msgs/html/msg/Imu.html I think each of the covariances should be separated by quantity measured (e.g. a few 9 entry matrices not 1 big one. Also more compact)

How do you feel about @peci1's suggestion for having the Track message include the Detection message (having that stuff) + a classification field?

Also, this + detection should have a Header per @tfoote early comments

[ghost]

Thanks @SteveMacenski. I had used the PoseWithCovariance.msg as my reference (which only had one covariance field). Covariance fields are now adjusted.

[JWhitleyWork]

This discussion seems to have died down a few months ago. However, it looks like all issues with this particular message were resolved. @SteveMacenski - would you be willing to merge this?

[SteveMacenski]

@peci1 @tfoote any blocking suggestions? I want to make sure we have near-universal 'OK' on this. I'm 100% fine iterating on this over time, but so far from Aaron's expertise as the CEO of a radar company, it seems like this represents the viable set of things a radar can provide and would be able to replace all the radar message uses we highlighted in the survey results from the first comment.

I know we haven't looked at this in awhile, and that's probably good so we have fresh eyes. @JWhitleyWork any adjustments you see you'd want to have made from your autonomous driving experience?

---

I agree with @peci1's sentiment that XYZWithCovariance would be nice to use, but none of those provided make sense in this context. If radar's can't meaningfully provide angular velocities, Twist is not right. If pose isn't giving orientation Pose is not right. Ultimately, we have precedence for doing separated covariances from IMU messages (and probably others).

[peci1]

Perfect! Thanks for patience.

[SteveMacenski]

Are the static / dynamic typical enum information a radar SDK / sensor would provide to populate those specific items? Settings those in the message will standardized those values for the message so we need to be clear that this is not just an example, but creating a standard.

---

@JWhitleyWork do you approve this?

[JWhitleyWork]

LGTM

[JWhitleyWork]

@tfoote I think you're the last set of eyes.

[tfoote]

Thanks for iterating this looks a lot stronger to me. It's ok as it is, but I have a few suggestions to consider for improving.

[tfoote]

I generally don't like leaving these open. But definitely don't suggest the usage of 3. This should be expanded to maybe a uint16 and reserve the upper half for additional vendor specific classifications and leave the lower half for potential future standardization.

On the scale of this message optimizing 8 bit vs 16 bit is not worth it.

[SteveMacenski]

Agreed. @radarAaron I think the action items here are:

• change uint8 to uint16 for all pre-defined classifications & classification.
• Keep car but remove 3 = since that implicitly sets 3 as a car, which I don't think was the intent.

I would suggest we could reserve the first 100 for further standardization rather than 32,000 - that seems excessive. Thoughts?

[gavanderhoorn]

I would suggest we could reserve the first 100 for further standardization rather than 32,000 - that seems excessive.

In my experience you end up never having enough of anything. Since you're already going to uint16, really future-proof it and do what @tfoote suggests.

Users won't care whether OEM ids start at 100 or 32000, it's just a number.

[JWhitleyWork]

I agree with @gavanderhoorn - Let's start vendor-specific values soemwhere around 32000.

[ghost]

Changes made as requested.

[SteveMacenski]

@radarAaron one last part of that - you need to update the uint8's for static, uknown, and dynamic.

[ghost]

done

[tfoote]

This is the current standard for covariance matrices from REP 103 but it is very large.

In the message API review it was deemed that a proposal to cut these down to lower resolution and upper triangular to cut the data size would be valuable: ros2/common_interfaces#91

Would people be interested in trying that out here. It's likely we can cut the covariance data size by two thirds just dropping to 32bit and upper triangular.

In this case it will save 192 of 288 bytes of covariance and will make this message overall about half the size per track. Since the current one has ~288 bytes of covariance and ~104 of position and uuid and one more for the classification.

This would be a good amount of bandwidth savings.

[SteveMacenski]

I'll yield to what you think is best on this front. The only criteria I think is important is consistency, so if other standard interfaces are going to move to this relatively soon, we can do that here first. If its not necessarily sure when that change would be made, I'd rather stick with tried and true.

[JWhitleyWork]

Agreed. I'm fine with making this an example of the new standard. Apex.AI made this change internally for similar reasons.

[ghost]

I have updated the covariance fields to float32's. I couldn't find any examples to copy about the wording to use for the upper triangle covariance matrix. Please review and suggest changes if required.

outdated

[JWhitleyWork]

I just noticed that geometry_msgs and uuid_msgs are new dependencies for this repository so they need to be added to package.xml and CMakeLists.txt in this pull request.

[ghost]

good catch @JWhitleyWork changes made.

[JWhitleyWork]

@SteveMacenski Can you and/or @tfoote address the one remaining item about the covariance fields? I think we're then ready for a merge!

[SteveMacenski]

Waiting on @tfoote to respond, I re-asked for a review to have him take a look. I'm happy as is, but I want to make sure everyone is OK with this since its a standard that impacts more than just I (and Tully's brought up really good points)

[JWhitleyWork]

Ping to @tfoote.

[icolwell-as]

Once this PR is merged we will be able to a noetic release right? Currently the lack of radar_msgs package in noetic is blocking us from releasing our radar driver. We aren't in any rush, just wanted to double-check that this is the last piece of the puzzle.

Thanks again for putting theses messages together in a standardized way!

[JWhitleyWork]

@SteveMacenski Given that it's literally been a couple of months and the fact that this message hasn't even been used and so is likely to have some modification in the future anyway, I vote for going ahead with the merge for now and making updates as required in the future. Better to get it out there and have people use it (including Autoware.Auto) than wait for it to be perfect.

[SteveMacenski]

Lets give it another week, but then yeah I agree. I would like to let @tfoote have another go at it since he's our supreme interface expert.

[JWhitleyWork]

@SteveMacenski Ping for merge.

[SteveMacenski]

Fair enough - I welcome to continue this discussion if there are additional requirements not present here, but I agree its time to ship it

[SteveMacenski]

@radarAaron thanks for baring with us after these many months! Your work and others in this thread have made a great impact here. Thanks to all!