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

Improve timestamp granularity of different measurements within a scan #16

Open
wants to merge 1 commit into
base: ros2
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions msg/RadarReturn.msg
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# All variables below are relative to the radar's frame of reference.
# This message is not meant to be used alone but as part of a stamped or array message.

builtin_interfaces/Duration stamp_offset # Offset to the stamp in the RadarScan message.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Define this better. offset representing what? The time it takes for the total scan to be accumulated? The offset due to time sychronization? etc

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This field should have the same meaning as time_increment of sensor_msgs/LaserScan.
The stamp_offset should be the offset to the time stamp of the first measurement on a collected scan which is defined in the header in the RadarScan message.

When you have a rotating radar scanner, a RadarScan would consist on a collection of RadarReturns that are from different measurements.
With this change the returns can have a precise time stamp.


I wanted to add an alternative description to this reply.
But I have problems finding other words to describe it (at least not in a substantial way), because I think it is already quite clear.
The ambiguity of the meaning of this sentence might be more introduced by that this message is not supposed to be used by itself, so it needs to be seen as a member of radar_msgs/RadarScan which time stamp is referenced in the comment.

What do you think about the following description?
It basically says the same, but might does it in a clearer way?

Suggested change
builtin_interfaces/Duration stamp_offset # Offset to the stamp in the RadarScan message.
builtin_interfaces/Duration stamp_offset # The offset relative to the time stamp set in the header of the outer message
# containing a collection of these returns.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to actually describe what its representing. You mention its the offset from the last point in the measurement for the first (e.g. accumulation time).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't meant to referencing the previous return.
Just the time stamp of the header (of the RadarScan message)

So the time stamp of a RadarReturn would be calculated by

<RadarScan>.header.stamp + stamp_offset

So the RadarScan would store the "base" time stamp.
And every RadarReturn would store the offset to this time stamp.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't explain it to me, explain it in the message precisely :-)

# If the offset is unknown it should be set to 0.
float32 range # Distance (m) from the sensor to the detected return.
float32 azimuth # Angle (in radians) in the azimuth plane between the sensor and the detected return.
# Positive angles are anticlockwise from the sensor and negative angles clockwise from the sensor as per REP-0103.
Expand Down
3 changes: 2 additions & 1 deletion msg/RadarScan.msg
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
std_msgs/Header header
std_msgs/Header header # timestamp in the header is the acquisition time of
tobiasneumann marked this conversation as resolved.
Show resolved Hide resolved
# the first beam in the scan.

radar_msgs/RadarReturn[] returns