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.