-
Notifications
You must be signed in to change notification settings - Fork 6
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
publish a vector of bytes given a topic type and message definition #194
Conversation
roslibrust/src/ros1/subscriber.rs
Outdated
|| responded_header.md5sum == Some("*".to_string()) | ||
|| conn_header.md5sum == responded_header.md5sum { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe if they are both "*"
there shouldn't be a connection? Or is that okay, neither will know anything about what the true md5sum is supposed to be and any byte payload can be sent.
This is pretty interesting... I can't think of anywhere in the ROS ecosystem where a publisher would supply "*" as the md5sum? I think if we could find a practical usage of someone else doing that I'd be pretty comfortable with this, otherwise I'd want to test the crap out of this with rospy, roscpp, and various cli tools to make sure they don't choke on it. I think the "right" thing to do is definitely to calculate the m5sum from the msg definition passed in. Let me take a look today at how hard I think that might be... |
Okay I pushed an example and some documentation updates. I can confirm at least that After looking at what it would take to compute the m5sum it'll take some work, but shouldn't be to bad. We could require the msg_definition that is passed in to be the fully expanded definition that includes the definitions of the sub messages referenced. That is probably the right call, as that is what will be needed for the message to work with something like plot_juggler. It means the user has to have access to the expanded definition (which our message generation can provide to them), I suspect the expanded definition is what is present in a rosbag, but I'm not 100% sure. We can then break that message definition apart and recursively calculate the md5sum off of the message and its sub-definitions which isn't too hard to do. I'd say either:
I'm happy with either. |
Doing the md5sum sounds cleaner but possibly a pain to get just right. rosbag store md5sums in the bag, it reads them out here:
(rosbag/bag.py) I was thinking I should do the same in my mcap recorder then on playback it doesn't need to recompute them (or resort to |
Hrmmm... I'm less familiar with MCAP what does it record? Full message definition with submessages? The fact that rosbag stores the md5sum would be really handy... We should probably just test the "*" thing as quickly as possible with roscpp and rospy, and if it doesn't work I can get fancy with computing the md5sum. |
It turns out I can try out roscpp subscribers to AnyMsg later. Also I've noticed plotjuggler can't publish ros1 messages from an mcap (generated either from the official mcap convert or my mcap_record), the topics don't even show up under rostopic list- maybe not a huge task to correct though. (PlotJuggler/plotjuggler-ros-plugins#90) |
Yes the full message definition exactly like in the connection header is stored in 'schema.data' and the message type under 'schema.name', and schema.encoding is "ros1msg" (or ros2msg or protobuf for those formats). Probably the md5sum could have been squeezed into the data field, or second parallel schema entry just to store the md5sum, and there are also https://mcap.dev/spec#schema-op0x03 update I see now |
I've only tried this python message-definition-to-md5 https://github.com/lucasw/mcap_tools/blob/main/mcap_tools/scripts/msg_def_md5.py for a few messages and so far so good:
There's some ambiguity where the message definition strips off package names (but not always, unless 'Header'?). The package name in field is stripped off when the message being defined is in the same package, and in addition Header is always without std_msgs? I don't think there are going to be very many message with say a |
Here's the python connection header message definition to md5sum converted to rust: lucasw@04bb108 I expect there are more than several str vs. String style/efficiency improvements to be made (and more map & filter to replace much of it) but it appears to work with handful of message test cases, I'll try it out with message publishing next. |
…ls/scripts/msg_def_md5.py to rust (and maybe the python should live in this repo), move the tests into a separate file later
LUCAS! Awesome work. I pulled in your changes adding the seperate md5sum calculation function, and then refactored the code a bit (in part so I could understand it better) and in part to improve readability. I think the last step now is plumbing this through the .advertise_any() call and this will be ready to merge. |
The plumbing part is sitting near the top of my commits over in my development branch- lucasw@1ddebf1 specifically- I'll try out your changes and those together tomorrow. |
…ter cleaning, or missing 'MSG:' in sub-message sections
…d because of that restore rejecting mismatching md5sums
I updated the comment above about mcap #194 (comment) - it turns out there are md5sums stored by mcap convert (but not where I was expected them, in Channels instead of Schemas), which if I'd realized sooner I wouldn't have made the definition-to-md5sum conversion. Having already done it I think the conversion can stay as-is here, though a second Now I'm trying to sort out how to get the same connection header information that is stored in rosbags from roslibrust into mcap_record- in a rosbag there is per message information on which publisher the message came from and the publisher node name is stored, at least sometimes when the callerid is available, but in roslibrust that gets discarded, or is otherwise difficult to access? In my modifications here I store and later query the most recently received publisher connection header, but the older ones are overwritten. I don't strictly have to conform to everything mcap convert does (and if I break compatibility I'll have to make a new mcap convert to use instead, probably in python because support for rosbags in rust I think is incomplete- or foxglove may accept PRs to change the mcap tool). |
Apologies for the delay, life took me out for a bit, this is now merged!
|
Description
Publish a message using a topic type string and message definition string but without having a RosMessageType. One example is loading serialized data out of an mcap or rosbag and publishing it directly.
Maybe the md5sum could be computed from the definition instead of using
"*"
, but the asterisk is the AnyMsg/ShapeShifter method (https://github.com/ros-o/ros_comm/blob/obese-devel/clients/rospy/src/rospy/names.py#L44-L53 & https://github.com/ros-o/ros_comm/blob/obese-devel/tools/topic_tools/include/topic_tools/shape_shifter.h#L115-L131, though I think under some circumstances that is updated to the actual md5sum ergo the 'shape shifter' name). I seecompute_md5sum
over in roslibrust_codegen but the definition needs to go through a reverse ofcompute_full_definition
to be the right format.Fixes
It's the publisher version of #190 but I didn't make an issue for it.
Checklist