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.