rosrust icon indicating copy to clipboard operation
rosrust copied to clipboard

Common ROS message types don't work between crates

Open neachdainn opened this issue 5 years ago • 11 comments

I've been using this library a fair amount the last few weeks and I've found that I have to have a crate that is nothing more than a rosmsg_include otherwise the message types are incompatible between crates. That is to say, anything like the following will not work:

/// ROS Crate A
rosmsg_include(std_msgs/Float64);

/// ROS Crate B
rosmsg_include(std_msgs/Float64);

/// Binary crate
fn main() {
    let val = crate_a::std_msgs::Float64::default();
    let moved: crate_b::std_msgs::Float64 = val; // Won't compile
}

This is problematic for trying to produce ROS crates that are project agnostic, as every crate will have to independently create the standard ROS types and they won't be compatible.

neachdainn avatar Jun 25 '19 20:06 neachdainn

A potential solution to this would be to have a common definition for all of these types that the ROS+Rust community can start utilizing. I know you've been against making assumptions about the format of the messages but, because Cargo builds dependencies from source, it would be possible to utilize a build script and rosmsg list to generate the messages. This wouldn't assume anything about the user's ROS install other than it has that command.

neachdainn avatar Jun 25 '19 20:06 neachdainn

Ideally, of course, ROS would have crates generated in their build system, which is something I considered, but the work of integrating that feels overwhelming, and once it's out there you're locked from making breaking changes until the next ROS release! But, with that solution we are locked out of using crates.io, and if I'd have to choose one or the other, I'd prefer the latter.

That being said, I am aware of that issue, and I've spent hours trying to figure out a proper solution for this. The inherent nature of crates makes this really hard.

In my usage of this library so far, I've encapsulated the message implementations to libraries that use them. For example, the work in progress actionlib implementation just has its own structures that can be casted to the message type, like this.

adnanademovic avatar Jun 25 '19 21:06 adnanademovic

Btw, your rosmsg list idea is really good and has a lot of merit. I never even thought about the fact that ROS allows you to just list all the messages that exist overall!!

This could work similarly to how bindgen crates work.

There could be a single crate, that depends on rosrust - let's call it rosrust-messages. All it does is run a build.rs script that reads rosmsg list on the machine, and generates a rosmsg_include!(...) with all the messages as the contents of its lib.rs file.

Since crates.io only stores the source, not the actual build result, every time this crate is depended upon it will correctly build, right?

adnanademovic avatar Jun 25 '19 21:06 adnanademovic

Ideally, of course, ROS would have crates generated in their build system, which is something I considered, but the work of integrating that feels overwhelming, and once it's out there you're locked from making breaking changes until the next ROS release! But, with that solution we are locked out of using crates.io, and if I'd have to choose one or the other, I'd prefer the latter.

I hadn't thought about that issue. That really is getting stuck between a rock and a hard place.

In my usage of this library so far, I've encapsulated the message implementations to libraries that use them. For example, the work in progress actionlib implementation just has its own structures that can be casted to the message type, like this.

That works as long as there is a clear owner of the type but issues will start happening once/if people start trying to share crates. Right now, if I were to try and bring your actionlib implementation into my project, I would have to manually write a function to convert between crate::std_msgs::String to actionlib::msgs::std_msgs::String.

There could be a single crate, that depends on rosrust - let's call it rosrust-messages. All it does is run a build.rs script that reads rosmsg list on the machine, and generates a rosmsg_include!(...) with all the messages as the contents of its lib.rs file.

I really like that idea.

Since crates.io only stores the source, not the actual build result, every time this crate is depended upon it will correctly build, right?

Every time it is depended upon, it will build all of the messages available on the user's system, yes. If all the required messages are present, it will build correctly, yes. It will make things more unreliable since the Cargo.lock won't fully preserve the dependencies but I think the Rust community has decided that is acceptable (via Bindgen).

One thing to consider is what to do when rosmsg list isn't available. It could be as simple as documenting that the command must exist and, as long as the build script still respects the message search path, that will be fine by me. Alternatively (additionally?) there could be a set of message definitions that are "backup" definitions, such as std_msgs+sensor_msgs+geometry_msgs, that get used if the build script isn't able to find any messages.

neachdainn avatar Jun 25 '19 21:06 neachdainn

It's a tradeoff. Either have ROS installed, or manually create a ROSRUST_MESSAGE_TYPES or whatever named environment variable that will contain what you would have wanted rosmsg list to print if it existed.

Having ROS installed is a default for most users, and while a big focus of rosrust is not making it a requirement, making a tradeoff of "ok, then tell me yourself if you don't want ROS on your build machine" seems acceptable to me.

We could also have the library scour all provided paths for .msg and .srv files if rosmsg list doesn't exist.

adnanademovic avatar Jun 25 '19 23:06 adnanademovic

Implementing this is on my docket, but I might not get to it for a bit.

ROSRUST_MESSAGE_TYPES should be a list of messages and, if present, should override the rosmsg list, right?

neachdainn avatar Jul 03 '19 01:07 neachdainn

That's the idea, yes. That allows you to not have ROS installed and still depend on crates that depend on rosrust-messages. There is a lot to consider though with this crate, regarding triggers of rebuilds. I've made some considerations, and generally have a good idea of what needs to be done for the crate to interact nicely with the compiler. But, I'll have to do some looking into how some factors of build scripts actually work (regarding triggers on updates). Generally I was away or busy these couple of weeks, but I'll start look into this again.

adnanademovic avatar Jul 17 '19 16:07 adnanademovic

There is a lot to consider though with this crate, regarding triggers of rebuilds.

Ironically, this is very easy to do with the environment variable. I am not sure what the best approach for the "regular" way would be. The rerun-if-changed=PATH would work well if the user has sourced devel/setup.sh but that isn't always the case (e.g., the first time the project is built).

Maybe it needs to attempt to discover messages in child directories manually? That would make ROSRUST_MESSAGE_TYPES unnecessary.

EDIT: Manually discovering the messages won't really work for system installed packages.

neachdainn avatar Jul 17 '19 18:07 neachdainn

All the triggers seem to work nicely, and I've added support for a comma separated list of message types to be provided in the discussed env variable.

I'll look a bit more into it, but the current WIP seems to work nicely. I'll transition as much of the code as possible to it, and see what the results end up being.

adnanademovic avatar Nov 26 '19 21:11 adnanademovic

I've removed support for the env variable. Instead we just crawl for all messages in the path manually.

adnanademovic avatar Nov 27 '19 20:11 adnanademovic

What's the status of this? I'm not using ROS much these days but now that Corrosion exists, I think this is the last missing piece for a ROS+Rust workflow.

neachdainn avatar Oct 02 '20 23:10 neachdainn

I believe this was solved with the introduction of rosrust_msg and I should have closed it a while ago.

neachdainn avatar Mar 09 '23 21:03 neachdainn