Add support for clocks

[DS3a]

just a draft pull request, a huge chunk of the functionality is yet to be added

[nnmm]

Hi @DS3a, sorry for being late to comment. I did take a look a while ago, but didn't finish writing this comment until now.

First, thanks for your enthusiasm and for starting the work on this. However, I think that implementing clocks was a too complex feature to choose for someone relatively new to ROS internals (and maybe Rust as well) such as you. How about you work on adding domain_id to the node/context init options instead? If that one does not interest you, maybe #132 or some other smaller issue?

The reason I'm recommending this is that it seems like you're not very familiar with ROS & Rust idioms yet. For instance, you're creating a Default instance for rcl_allocator_t containing null function pointers, but such an allocator is invalid, so it doesn't make much sense. It gives the impression that your goal here was simply to make things compile, but as a contributor, you should aim to be familiar with the types and functions you're using, how they're used inside rclrs, and why.

On the Rust side, things like the DurationFrom type are clunky and do not fit with established style of the Rust community – you'd probably implement From on the Time type instead. You also implement Clone manually, which is not usually done either, etc. On the lower level, I see lots of code smells that we try to avoid or at least justify as well, like as and unwrap(). There's more, but I think it's not needed to go into detail.

So, to make sure you're able to contribute independently to rclrs, here is what I recommend:

1. Try reading articles/books/talks on Rust best practices, for instance from this list, to get more familiar with idiomatic Rust.
2. Read through the rclrs code base, or at least parts of it, and try to understand why things are done that way.
3. Choose a (smaller) feature to work on.
4. Get really familiar with that feature.
   1. Know what problem it solves. Some features have design docs, which would be good to read.
   2. How is it implemented in rclcpp, and maybe also rclpy. How has it evolved? Is there some
   3. Know the relevant functions and types in rcl, and make sure to read their documentation.
   4. …
5. Think a while about how you'd implement it, start coding, get feedback if needed, and when it's ready open a PR.

I hope this sounds reasonable to you. Please don't be too discouraged, we value every contributor and want to help you find a path to be productive in our code base! We are all learning as we go, it just requires a bit of patience.

[DS3a]

Got it... I did feel like I took a bite larger than I could chew. I shall start working on the simpler issues while getting more familiar with the rust idioms and the internals of ROS, and slowly work my way up.

Thank you for the valuable feedback, and for taking the time to comb through my code. I did catch myself trying to trick the compiler many times. Although I did go back and fixed what I thought was wrong, I should have stuck to the standard procedures from the very beginning itself, as I lost track. I have started reading the embedded rust book and some others to get more familiar with rust.

Now, I couldn't find domain_id in any of the issues mentioned, however there is an rcl_node_get_domain_id in the rcl bindings that have been generated. could point me to the right issue? or open one to explain what exactly is required?
So far, I have understood that you want to be able to specify the domain_id that you want the node to be joining while initiating it. Is that right? or do you want to be able to change domain_ids during the runtime? and this should happen in line 293 of builder.rs where it should be setting the domain_id instead of calling rcl_node_get_default_options()? can you please point the issue out to me? or start a new one with an explanation so that I can start from there?

[nnmm]

Perfect, thanks @DS3a! You're right that there was no issue for it, but I've now created #206. Let me know if it's not detailed enough.

[DS3a]

Perfect, thanks @DS3a! You're right that there was no issue for it, but I've now created #206. Let me know if it's not detailed enough.

Thanks... I've started looking in into it... I shall let you know if there's any queries

[nnmm]

@DS3a can this be closed then?

[DS3a]

@DS3a can this be closed then?

Yes, sure... sorry for the late reply; I'm busy with work... I shall start putting in more time as soon as the load decreases.