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

Connect rerun #12

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Connect rerun #12

wants to merge 8 commits into from

Conversation

esteve
Copy link
Collaborator

@esteve esteve commented Sep 19, 2024

Connect rclrs to Rerun.

Converters (copied from #4)

From std_msgs:

  • Int8 -> Scalar
  • Int16 -> Scalar
  • Int32 -> Scalar
  • Int64 -> Scalar
  • UInt8 -> Scalar
  • UInt16 -> Scalar
  • UInt32 -> Scalar
  • UInt64 -> Scalar
  • Float32 -> Scalar
  • Float64 -> Scalar
  • String -> Text

From geometry_msgs:

  • Point -> Points3D (mono)
  • Pose -> Transform3D
  • Transform -> Transform3D
  • Quaternion -> Transform3D (only quaternion field used)
  • Polygon -> SeriesLine

From sensor_msgs:

  • PointCloud/PointCloud2 -> Points3D
  • CompressedImage -> EncodedImage
  • Image -> Image
  • CameraInfo -> Pinhole
  • Imu -> Multiple Scalars

From visualization_msgs:

  • Marker. This is a super flexible type that needs to be translated into one of the appropriate (Arrow, Box, Sphere, Line, Points, Mesh) primitive.

src/converters/traits.rs Outdated Show resolved Hide resolved
cdr_buffer: &mut Cursor<Vec<u8>>,
) -> Result<(), Error> {
let value = cdr::deserialize_from::<_, i8, _>(cdr_buffer, cdr::Infinite)?;
rec.log("scalar", &rerun::Scalar::new(value as f64))?;
Copy link
Member

Choose a reason for hiding this comment

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

All of these should use the entity_path from the Conversion as the log destination.

src/converters/mod.rs Show resolved Hide resolved
esteve and others added 2 commits September 20, 2024 12:55
transforms and quaternions.

Signed-off-by: Esteve Fernandez <[email protected]>
@esteve
Copy link
Collaborator Author

esteve commented Sep 23, 2024

@jleibs can you have another look at this? I've added the logic for dispatching different converters based on the (topic, frame_id) tuple. frame_id is optional in the configuration and by default conversions will just take the topic to create subscriptions. Thanks.

@esteve
Copy link
Collaborator Author

esteve commented Sep 26, 2024

@jleibs I've added a pseudo-converter for TransformStamped, which has a header (with a frame_id). The converter doesn't do anything on the frame_id, but I added some example code that shows how to process the field_id that comes from the CDR blob, see https://github.com/rerun-io/rerun-ros/pull/12/files#diff-e728a4b4964ced4a55d869ea1fd0b415c7821e12c3846b044377b8f8a7ae6578R113-R119

Signed-off-by: Esteve Fernandez <[email protected]>
@esteve esteve marked this pull request as ready for review September 26, 2024 15:21
@jleibs
Copy link
Member

jleibs commented Sep 26, 2024

Makes sense.

Definitely not for this PR, but in the future it would be nice to be able to do that logic, generically, for any Stamped variant of a message, but I recognize there is some complexity in doing that since not all messages have headers.

Does CDR let you generically pull out a header (if you know it's there?)

Alternatively maybe this is something we handle via the trait.

If the converter looked like:

    fn convert(
        &self,
        topic: &str,
        frame_id: &Option<String>,
        entity_path: &str,
        message: &mut Cursor<Vec<u8>>,
    ) -> Result<(Option<Header>, Box<dyn Loggable>), Error>;

Then we could actually move the logging code out out of the convert and up into process. This would keep the converter from needing to know anything about the log stream or entity_paths.

Approximately something like:

let (header, loggable) = converter.convert(rec, topic, frame_id, message)?;

// Find mapping from (topic, frame_id) -> entity_path
let entity_path = lookup_path_routing(topic, header);

// Use the timestamp from the header if there is one
if let Some(header) = head {
  rec.set_time_seconds("ros_time", header.timestamp);
} else {
  rec.set_time_seconds("ros_time", receive_time);
}

rec.log(entity_path, loggable);

@esteve
Copy link
Collaborator Author

esteve commented Sep 27, 2024

Definitely not for this PR, but in the future it would be nice to be able to do that logic, generically, for any Stamped variant of a message, but I recognize there is some complexity in doing that since not all messages have headers.

Does CDR let you generically pull out a header (if you know it's there?)

Alternatively maybe this is something we handle via the trait.

We could try two things:

  • Attempt to read the header anyway and if it fails, process the message as usual, I have to check if the cdr will fail or will just deserialize whatever it's fed.
  • Use the message spec representation we already have to check if there's a header field. This requires the files for the message definitions to be available in the workspace.

I think either are valid approaches, I can work on it when I come back.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Translate basic ROS types to Rerun
2 participants