Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adapt RadarReturn and RadarScan #14

Conversation

tobiasneumann
Copy link

This PR proposes several changed to the RadarReturn and RadarScan messages.

Summary

Time stamp

The current message for a RadarScan just stores one time stamp for all returns.
And this time stamp is not clearly defined.

Proposed change

  • Add the field stamp_offset to RadarReturn, which will be used relatively to the time stamp of the time stamp in the field header of RadarScan.
  • Define the time stamp of the field header of RadarScan to be of the first return.

Usage of the spherical angles

The current description for the azimuth and elevation angles doesn't directly specify the order of these angles.
From the comment for elevation "For 2D radar, this will be 0." the order can be deducted, but it could be clearer.

Proposed change

  • Add the description that the angles are supposed to be used as "elevation-over-azimuth"

Missing Measurement characteristics

While for LiDAR sensors the measured returns are typically treated as points, this behavior is problematic for radar sensors.

Proposed change

  • Add the field sensor_properties to RadarScan
  • Add the messageSensorProperties to group the information about the device used for the measurement.

Missing definition of amplitude

The description for the field amplitude is missing the relative information of the measurement.

Proposed change

  • Define the field as RCS linear/m^2.
    (Please note that this also changes the logarithmic value to a linear one.)

Tobias Neumann added 4 commits August 22, 2023 14:15
RadarScan: clearify that the stamp in the header should be the first measurement of this scan.
RadarScan: add SensorProperties to message
@SteveMacenski
Copy link
Member

I think this should be split up into PRs of different natures or else this will never get merged since we'll nitpick the different additions at the same time. The changes to the units, sensor properties, and adding offsets are all different subjects.

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