self.node.get_clock().now().to_ros_msg().unwrap() doesn't work anymore

[Guelakais]

Some errors have detailed explanations: E0433, E0560.
For more information about an error, try `rustc --explain E0433`.
error: could not compile `lanelet_rs` (bin "lanelet_publisher") due to 3 previous errors; 1 warning emitted
error[E0308]: mismatched types
   --> src/lanelet_subscriber.rs:73:28
    |
73  |                     stamp: self.node.get_clock().now().to_ros_msg().unwrap()
    |                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `builtin_interfaces::msg::Time`, found `Time`
    |
    = note: `Time` and `builtin_interfaces::msg::Time` have similar names, but are actually distinct types
note: `Time` is defined in crate `rclrs`
   --> /ros_ws/install/rclrs/share/rclrs/rust/src/vendor/builtin_interfaces/msg.rs:223:1
    |
223 | pub struct Time {
    | ^^^^^^^^^^^^^^^
note: `builtin_interfaces::msg::Time` is defined in crate `builtin_interfaces`
   --> /ros_ws/install/builtin_interfaces/share/builtin_interfaces/rust/src/msg.rs:185:1
    |
185 | pub struct Time {
    | ^^^^^^^^^^^^^^^

error[E0599]: no method named `sec` found for struct `Time` in the current scope
  --> src/lanelet_path_cross_product.rs:78:34
   |
78 |                         sec: now.sec(),
   |                                  ^^^ method not found in `Time`

error[E0599]: no method named `nanosec` found for struct `Time` in the current scope
  --> src/lanelet_path_cross_product.rs:79:38

How do you imagine how the timestamp should be filled properly and why does this error first occur today? What have you done?

[Guelakais]

Ok, I have a workaround for now. So you can fill the timestamp of a ros2 header correctly:

Time {
          sec: (self.node.get_clock().now().nsec /10_i64.pow(9)) as i32,
          nanosec: (self.node.get_clock().now().nsec %10_i64.pow(9)) as u32,
          }

As you can see right here, your rclrs::Time type collides with the builtin_interfaces::Time type. In my opinion, a good approach would be to translate certain ros2 message types directly into rust crates to make them available for rclrs. I have already asked you how to do this and have not yet received an answer. If you do this, a possible method in the implementation for rclrs::Time could look like this

fn to_ros_msg(&self) -> builtin_interfaces::Time {
builtin_interfaces::Time {
                    sec: (self.nsec /10_i64.pow(9)) as i32,
                    nanosec: (self.nsec %10_i64.pow(9)) as u32,
                }

From my point of view, this issue should definitely remain open, as timestamps are an essential resource for nodes, especially when processing sensor data. I am currently having heated discussions with someone who is using an algorithm for determining angular ranges from sensor_msgs::msg::LaserScan messages in his node and simply refuses to look at the timestamps. The results of his node are complete rubbish. I have now set about doing this in my implementation for precisely this reason. Timestamps are essential and must adapt to the corresponding ros2 message types to ensure proper processing.

[esteve]

@Guelakais do you happen to have the builtin_interfaces repository in your workspace? If so, you'd need to remove it (or maybe even remove the entire workspace and start from scratch). The builtin_interfaces code is now vendorized into rclrs, which from what I gather, conflicts with the repository you have in your workspace.

In my opinion, a good approach would be to translate certain ros2 message types directly into rust crates to make them available for rclrs. I have already asked you how to do this and have not yet received an answer.

Can you paste a link to the ticket where you asked for this feature? Would be a nice addtion, though perhaps using the Into and From traits would be more idiomatic, and if you have time, we'd be very happy to review a PR with these changes if you submit it.

[Guelakais]

#381
I'm beginning to think that I can't avoid a pull request. But then I would have to learn how github works...

[mxgrey]

@esteve it occurs to me that the vendored message packages could create a lot of confusion when building the ros2_rust repo in the same workspace as a ROS distro, which was meant to be supported as of #370.

What would you think if we put some logic into the vendoring script to check whether the Rust bindings for the messages are available in the workspace before we vendor them?

We would then install the vendored bindings in a way that they can be used as if they're the upstream bindings.

[Guelakais]

building the corresponding Ros distribution in the same workspace as ros2 rust is, in my experience, quite extraordinary. Normally, the classic ros2 developer relies on the corresponding .deb dependencies, which he installs via apt install ros-${ROSDISTRO}-<PACKAGENAME>. A logic that checks how ros2 rust gets .msg dependencies is of course always cool.

Something basic: If in doubt, you should include your dependencies in package.xml via <depend>package</depend> instead of any other keyword, as this exact keyword is used when using ros2 pkg create pacakge_name --dependencies package_one package_two. Build and exec dependencies are, in my opinion, more confusing than normal <depend> dependencies.

[esteve]

@mxgrey yeah, I agree, the vendored interfaces can complicate things (this ticket is an example of that). The core issue is that we wouldn't need to this if we could get the generator included in the buildfarm. Or at least, we could do things differently.

What would you think if we put some logic into the vendoring script to check whether the Rust bindings for the messages are available in the workspace before we vendor them?

That's a good idea, it'd make things less confusing and we'd still be able to push rclrs to crates.io

[esteve]

@Guelakais the infrastruture for generating code for messages is rather complex, hence why it's not as streamlined with ros2-rust as with other projects that just use existing message packages in other languages. We know about this issue and the ROS team as well, we've been talking with them on how to improve the user experience since we both believe that Rust will play an important role in robotics in the short future.

However, you'd still need to declare dependencies in Cargo.toml so that both colcon and cargo can find them. C++ doesn't have that problem because there's no packaging system for C++, but given that Rust comes with its own toolchain, the dependencies need to be duplicated. Eventually, we might be able to infer the ROS dependencies from Cargo.toml, but not sure if that's entirely feasible.

[Guelakais]

Ok, I've been dealing with this bug all evening. For some reason rclrs keeps replacing builtin_interfaces::msg::Time with rclrs::vendor::builtin_interfaces::msg::Time when compiling. I have already tried to include the dependency directly in Cargo.toml. Doesn't work either. I have found a script, which probably just renames all possible parts in the code when compiling. It is called vendor_interfaces.py. This file alone probably ensures that neither the Into trait nor the to_ros_msg method can generate the correct type in the return. So the error is simply too deep.

[pmirabel]

Hi @Guelakais ! I am trying to construct a std_msgs::msg::Header and having hard time to figure out how to do that.
I am using

        let stamp = rclrs::Clock::system().now().to_ros_msg().unwrap();
        let header = Header {
            stamp,
            frame_id: "".to_string(),
        };

and get : Time and builtin_interfaces::msg::Time have similar names, but are actually distinct types.
Would you mind to elaborate your workaround please?
Thanks !

[Guelakais]

Hi @Guelakais ! I am trying to construct a std_msgs::msg::Header and having hard time to figure out how to do that. I am using

        let stamp = rclrs::Clock::system().now().to_ros_msg().unwrap();
        let header = Header {
            stamp,
            frame_id: "".to_string(),
        };

and get : Time and builtin_interfaces::msg::Time have similar names, but are actually distinct types. Would you mind to elaborate your workaround please? Thanks !

The functionality of my workaround can be observed in one of my nodes. Normally it works like this:

let header = std_msgs::msg::Header {
                frame_id: "map".to_string(),
                stamp: Time {
                    sec: (self.node.get_clock().now().nsec /10_i64.pow(9)) as i32,
                    nanosec: (self.node.get_clock().now().nsec %10_i64.pow(9)) as u32,
                },

I also have a rough idea of how it could be solved. For example, you can include a trait that provides the method to_ros_msg(). At the same time, I am still not sure if this trait is implemented by all possible ros2 time interface structs. ros2 rust uses a total of 3 different variants of this message interface:

1. builtin_interfaces::msg::Time
2. builtin_interfaces::msg::rmw::Time
3. vendor::builtin_interfaces::msg::Time
   all three variants also look exactly the same:

pub struct Time {
    pub sec: i32,
    pub nanosec: u32,
}

2 of these variants are autogenerated. Therefore, I am not sure whether the corresponding interface has to be incorporated directly into the autogenerator or whether it can somehow be incorporated into the time struct of rclrs. So far I had no success with the latter approach.

[jhdcs]

On the three different variants of the message interface:

1. builtin_interfaces::msg::Time is the standard sort of message that we would like to use; however, due to Rust not being on the buildfarm yet, we tend to need to use the vendorized version below.
2. builtin_interfaces::msg::rmw::Time is the message that's used by the rmw layer. While it's layout is exactly the same as builtin_interfaces::msg::Time, using it might confuse a future programmer into thinking that you're working with the rmw layer for some reason. As such, I'd avoid using this unless you're working on something internal to rclrs.
3. vendor::builtin_interfaces::msg::Time is the vendorized version of builtin_interfaces::msg::Time. The vendorized version of messages were developed as a workaround to build/buildfarm issues, and we are working on eliminating the need for them as soon as we possibly can. This is actually mentioned above.

That's at least how I think about them in my own head. One of the other maintainers can correct me if I'm wrong, but I think that vendor::builtin_interfaces::msg::Time is probably going to be the message type that you want to use for now.

[Guelakais]

Of course you can do this if you only want to transfer already vendorised message types. At the latest when you want to transfer a ros2 std_msg, this is no longer possible, as this does not communicate with the vendor::builtin_interfaces, but with builtin_interfaces.

[romainreignier]

I have faced the same issue. This workaround works but it is not trivial for newcomers.
Is there any solution we can work on to resolve the conflict?

[Guelakais]

not the way ros2 rust is implemented. The problem is that ros2 rust has implemented a completely separate ros2 message stack for testing purposes, which collides with the normal ros2 message stack. In my opinion there are only 2 options:

• either the messages vendorised in rclrs are completely removed. Then ros2 rust will be significantly less testable, because it then has to be tested directly with the ros2 toolchain
• the associated ros2 messages are automatically uploaded to crates.io, in which case the problem disappears completely because the tests can be carried out directly by cargo.

You could also address the messages in ros2 rust directly and work with them. Then you don't have the problem. At the same time, ros2 will then only be usable to a very limited extent.

[romainreignier]

Hi @Guelakais
Thanks for your detailed answer. I see that this cannot be easily solved, I will keep using your workaround for now.

[mxgrey]

In the working group meeting today it was mentioned that we might not need the vendored messages if the build farm would generate the bindings for the message packages that rclrs depends on.

However, IIUC crates.io wouldn't allow us to have dependencies on any crates that are not also published to crates.io. So for rclrs to have a dependency on builtin_interfaces, we would need to publish a crate called builtin_interfaces to crates.io, even if our intention is to fetch that dependency from a crate generated by the buildfarm. crates.io just wouldn't know how to obtain that generated message binding crate.

Does all of that match your understanding @esteve ?

The only way I can think of working around this such that we can keep rclrs on the buildfarm would be to remove all message package dependencies from the core rclrs entirely and introduce a new package named something like rclrs_convert which would define conversion functions from rclrs types into the various standard message structures. We would publish rclrs to crates.io but we would withhold rclrs_convert.

There would likely be a lot of data structures introduced to rclrs that are effectively duplicates of message structures defined in builtin_interfaces and others. I don't love that situation, but I can't think of any other way to escape this problem.

[esteve]

Does all of that match your understanding @esteve ?

Yes. Once we have the generator as part of the buildfarm, we could explore publishing some of the base message packages as crates, I think it might work, but we'd have to test it. In any case, yeah, I agree the situation is not ideal, especially now that we're in a transition period before submitting the message generator to the buildfarm. I'd prefer if we can keep rclrs as a crate on crates.io as it'll play along with the rest of the crates ecosystem (e.g. docs.rs)

[mxgrey]

we could explore publishing some of the base message packages as crates

I'm not particularly against this, but I can imagine us running into conflicts with crate names as generic as builtin_interfaces, std_msgs, test_msgs, and action_msgs. That being said I see that we've already snatched up the ones that we would currently need.

I guess we just need to hope that there aren't others in the crates.io ecosystem that would want to fight us for ownership of those crate names.

I'd prefer if we can keep rclrs as a crate on crates.io as it'll play along with the rest of the crates ecosystem (e.g. docs.rs)

I fully agree with that. I'm trying to brainstorm how to play as nicely as possible with both ecosystems at the same time. Unsurprisingly it's just very tricky to stand with each foot in a different world at once.