Message format for RadarPoint

[ghost]

The proposed message type for a RadarPoint.

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

Only two drivers appear to output a 'raw' point format.

Position/Coordinate system
One driver outputs in polar coordinates while the other outputs both polar and Cartesian format

Speed
Both drivers output speed. One is called "velocity" and the other "doppler_alias" (both are float 32)

Intensity
Both drivers output intensity. One driver calls this "intensity" and the other "sensor_nr" (noise ratio)

Additional fields
Drivers also include the following fields: usage, target_format_type, msg_counter

Conclusion
The proposed format appears to be sufficiently in line with the existing drivers.

Questions

• The proposed format is for polar/spherical coordinates (azimuth, elevation, distance) which is what radar will natively produce, however in order to do anything significant with this data (eg filtering based on height), it will usually be converted to Cartesian coordinates (x, y, z). I'd be interested to get opinions of using Spherical coordinates vs Cartesian coordinates as the native points interface for radar data in ROS.

[SteveMacenski]

@radarAaron can you add the analysis for each specific message in the PR description (# packages looked at, what they had in common or what they did otherwise, the stuff you posted in slack)? And our rationale about why "this" over "that"?

That will let me then post these 3 PRs in a grouping on Discourse to get community feedback / buy-in on these messages. We should explain why we did "this" over "that" and how our message is suitable for the vast majority, if not all, radar messages to be the new standard.

[SteveMacenski]

Because this is the "raw data" output from the sensor, i think we should follow the sensor conventions that are also smaller messages which can be beneficial. Folks working with this level of radar data would probably want to have it in this format so you can have some more direct awareness of where in the antenna the return came from, in case that's helpful in filtering/structured algorithms

[SteveMacenski]

I approve as is

[tfoote]

For the low level sticking to the spherical definitely makes sense. There's actually a lot of information baked into that such as the implicit doppler velocity direction.

On the naming I think that Point is not right as it's not just a position in space. I term that I think most accurately reflects the semantics of this is a RadarReturn as this is specifically about capturing the underlying measurements. It could also be Echo a quick search found both used commonly.

Similarly the RadarPoints might make more sense as a RadarScan to parallel our LaserScan. And since that's a common word for a grouping of radar returns.

[SteveMacenski]

RadarReturn

So we at one point were doing RadarRaw, and Aaron rightfully corrected me that this isn't really "raw" for people doing signal processing. I would be OK with RadarReturn but I'm going to yield to Aaron for best practices.

RadarScan to parallel our LaserScan

I really like that, any objections?

[mjcarroll]

I'm definitely 👍 for polar rather than Cartesian.

I was going to see what you thought about potentially including two fields:

• a covariance (3x3 for range/az/el or 4x4 for range/range-rate/az/el) - I'm not familiar with the current hardware offerings, but it's often used in the next step of processing (correlating tracks)
• Radar Cross Section - Same as above. Potentially given by drivers/potentially used in downstream processing.

Disregard, I hadn't started reviewing #2 or #3

[frankeverdij]

The ROS driver for the UMRR-11 and UMRR-96 automotive radars from SmartMicro is not included in the overview. It is CAN based, but there is a JSON file which list 'Range', 'Azimuth', 'Speed_Radial', RCS' and 'Elevation' as fields. I couldn't find any further information on these fields, but the driver is available at smartmicro.de as well as data sheets for the individual radar types.

As for the RadarPoint message proposal, i am in favor of the polar coordinate system for representing a single radar reflection.

The intensity in radar systems we came across, mostly Continental ARS ones but also this UMRR, is given as an RCS (radar cross section) value in dB which is a float32, not uint8. The naming of this field seems to vary between 'intensity', 'snr' and 'RCS'. Since it depends on the specific radar used, its interpretation may vary, but i would recommend it not to be called 'intensity' to avoid mixup with LaserScan sensor measurements. 'snr' or 'RCS' would be more suitable, in my opinion.

I also am in favor of the 'azimuth' naming over 'angle' for the horizontal angle between the radar point vector and the normal vector of the radar.

[SteveMacenski]

@frankeverdij please provide a link to this ROS driver.

Sounds like we're all in agreement on azimuth and RadarReturn changes. More discussion required on the intensity / similar parameter naming and type.

[icolwell-as]

Update comment wording to reflect new message name

[SteveMacenski]

@tfoote @mjcarroll can you take another sweep through? I think this resolved all your concerns and mine.

The only ? is the amplitude and RCS thing which I think is resolved now, but @frankeverdij will need to chime in.

If we get a consensus, then we can merge this and move onto the Detection message

[frankeverdij]

The only ? is the amplitude and RCS thing which I think is resolved now, but @frankeverdij will need to chime in.

The name 'Amplitude' and its type of float32 is fine with me. It is certainly more descriptive than 'RCS' , for it being an acronym.
An issue might be that radars outputting intensity values between 0 and 255 for each detection either requires the interface driver to convert these values in dB, or simply put the values in the 'Amplitude' field as float32. This could be indicated by an extra flag, but i don't know if this is a big issue which merits such an addition.

[SteveMacenski]

@frankeverdij so do you approve of this as is? If so, can you hit the approve check mark so we can get consensus? I'd like at minimum 3-4 approvals from different groups to make sure we have a good cross section of the community.

We appreciate your time in helping work through these issues!

[frankeverdij]

You're welcome. I have submitted my approval for this thread. Do i need to approve the change request below as well? If so, a quick help on how to do this would be most welcome.

[SteveMacenski]

I'm not entirely sure what you mean by approve change request below

[frankeverdij]

I'm not entirely sure what you mean by approve change request below

Disregard my question. I am not too familiar with github's discussion interface so i wasn't sure if i missed a checkmark somewhere. Everything looks good from my side.

[ghost]

I think that is all the suggested changes made. Last call for changes before we commit this.

[shrijitsingh99]

Looks good to me 👍

[SteveMacenski]

That's now 5 independent approvals, merging