Carter12s / roslibrust

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

Subscribe to any topic type like ShapeShifter/AnyMsg and get raw bytes from next() instead of deserializing #190

Closed lucasw closed 2 months ago

lucasw commented 2 months ago

I made a subscribe-to-anything system here: https://github.com/lucasw/roslibrust/tree/subscribe_any (and https://github.com/lucasw/mcap_tools/blob/main/mcap_tools/src/mcap_record.rs makes use of it)- it tries to duplicate the rospy/roscpp approach but also requires a placeholder ros message type to satisfy other code.

Could there be a special ShapeShifter/AnyMsg completely internal to roslibrust that satisfies all the traits of a ros message type as generated by the macro, and even can get passed into serde_rosmsg and just returns Vec (or returns what seems like a regular message but contains only byte[] data).

Maybe something is wrong with my implementation as I get duplicate received messages if the mcap_record node starts before the publisher.

What I'm looking into next is getting information out of the connection header to satisfy writing an mcap record (mostly getting the message definition), looks like I can find the right Subscription in the node handle and access it.

Further future is the publisher equivalent of this (for a rosbag play alike)- be able to give the publisher raw bytes to publish after configuring it with the topic and message definition and md5sum loaded from the mcap.

I haven't tried rebasing on your latest changes, but one thing that stands out is the part you commented out that compares md5sums.

Some notes on where I found the values rospy and roscpp were using: https://github.com/lucasw/mcap_tools/issues/1#issuecomment-2271952018

Carter12s commented 2 months ago

Okay, spent some time thinking on this today.

  1. I think this is a useful feature for the crate to have and pretty easy to support so we should add it.

  2. I think this should happen through a parallel API to subscribe() (much like you've done) and not rely on any "Magic Type" a. Call to subscribe_raw() -> returns a Subscriber<Vec>. We don't need subscribe_raw() to take any type argument. b. Instead of Subscriber having the deserialization logic buried inside .next(), Subscriber holds a "Fn(Vec) -> T". This adds an extra layer of indirection and there will be some runtime performance hit, but not trying to make this crate stupid fast. c. When Subscriber gets constructed either in .subscribe() or in .subscribe_raw() the correct handler function is inserted into it. For a regular subscribe this will perform the deserialization logic. For a raw subscribe this can be a no-op and return the raw Vec

  3. Your branch is relatively close to what I'd want to merge for this feature, but has a bunch of other changes I wouldn't want to merge in it. Do you want to go through a PR yourself on this repo and clean up your branch, or would you prefer me to tackle?

lucasw commented 2 months ago

Do you want to go through a PR yourself on this repo and clean up your branch, or would you prefer me to tackle?

I'd like to give it a try- I'll make a new branch based on what's current and keep it focused on just subscribe_raw without the other hacked in features to make mcap recording work, cleaner versions of those could come later.

Carter12s commented 2 months ago

That sounds awesome! Thank you!