Carter12s / roslibrust

An async first rust client for ROS1 native and rosbridge.
https://docs.rs/roslibrust
MIT License
48 stars 6 forks source link

Deserialize bytes into ros message? #158

Closed lucasw closed 3 months ago

lucasw commented 4 months ago

Given I have raw bytes of a ros message (which happen to come from mcap) and that I have a matching ros message struct from the macro, what do I do to decode?

https://github.com/lucasw/ros_one2z/blob/38ce91d5976d3f362f5ec9f7e92b3237290bfff0/mcap_to_rerun/src/main.rs#L44

I was thinking roslibrust must have an example within it in subscriber code but I haven't seen it so far- also I'm not that familiar with rust,maybe this is a basic serde questionm.

lucasw commented 4 months ago

Think I found it https://github.com/search?q=repo%3ACarter12s%2Froslibrust%20serde_rosmsg&type=code

https://docs.rs/serde_rosmsg/latest/serde_rosmsg/ https://github.com/adnanademovic/serde_rosmsg

Carter12s commented 4 months ago

Yup you got it.

We don't have too much of that in our Repo because we rely on serde_rosmsg for it.

The types that our codegen generates are directly compatible with serde_rosmsg, and ::from_slice() should be what you want.

However, the fact that you called out MCAP has me a bit worried. serde_rosmsg is for ROS1 native messages and very likely will not work with a binary encoded ROS2 message.

If you are working with native ROS2 data you probably want to checkout https://github.com/ros2-rust/ros2_rust

Sorry the ecosystem is so piecemeal at the moment.

lucasw commented 4 months ago

No I'm using ros1msg in mcap which works fine after doing a mcap convert my_ros1.bag my_ros1.mcap (I was looking at https://docs.rs/rosbag/latest/rosbag/ for using bags directly, maybe that could still work but the output looks too raw).

There's an annoying 4 byte length header that appears in some places and not others - serde_rosmsg wants the header, but mcap doesn't deliver an array that contains it. (It would be nice to chase down some of these and add an option not to require it if it already knows the length from elsewhere)

This code is working: https://github.com/lucasw/ros_one2z/issues/10#issuecomment-2185313738

Carter12s commented 4 months ago

Dope! I hadn't seen ros1 w/ mcap yet. Cool that exists.

I've definitely tripped over the various 4-byte length fields before. Note that deserializing to Vec needs the 4 byte header to be there, but [T; #] needs the header to not be there. Note that v0.9.0 of this crate fixed a bug with our message generation where we we're incorrectly generating Vec for fixed length array types which was causing them to fail deserialization with serde_rosmsg.

I "think" the answer to your "it would be nice to chase down some of these and not require the 4-byte length field if already knows the length" comes down to using [T; #] vs Vec, but not entirely sure that addresses what you need.

Carter12s commented 4 months ago

Ahh never mind. Just read your linked thread more.

You are talking about the 4-byte overall message length field documented here: https://wiki.ros.org/ROS/Connection%20Header

Which is technically part of TCPROS/UDPROS specification, but I can see why mcap would omit it.

Let me jump over to serde_rosmsg and see if I can make and PR to add the feature you are talking about. I agree it is reasonable.

Carter12s commented 4 months ago

Okay I reviewed the serde_rosmsg code, and their implementation of a Deserializer is fully dependent on being able to read the correct expected message length up front, and relies on this message length to track deserialization progress, and to know when to stop.

I think it is probably a significant re-write to implement skipping the message length field in a "good" way.

There is an easy "work around" that looks like:

pub fn from_slice_no_length<'de, T>(bytes: &[u8]) -> Result<T>
    where T: de::Deserialize<'de>
{
    let mut data_copy = Vec::new();
    data_copy.reserve(bytes.len() + 4);
    data_copy.extend_from_slice(&(bytes.len() as u32).to_le_bytes());
    data_copy.extend_from_slice(&bytes);
    from_slice(&data_copy)
}

I'm not sure folks will want that to be the canonical solution, but I think you could use this as a pretty reasonable workaround if you're willing to deal with a silly copy.