MIT-SPARK / config_utilities

Automatic C++ config structs and tools.
BSD 3-Clause "New" or "Revised" License
28 stars 6 forks source link

ROS2 integration #22

Open jessemorris opened 3 months ago

jessemorris commented 3 months ago

Hi,

I want to use this (very awesome ;)) library for a project using ROS2.I just saw you guys are working on a ROS2 branch for this project and was wondering what the timeline is for that work getting integrated into main? Are there plans to add in additional ROS features (such as dynamic reconfigure in ROS1 or the equivalent event handling in ROS2?) If not, I am happy to work on these features myself and make PR to this repo.

Cheers, Jesse

nathanhhughes commented 3 months ago

Hi Jesse! Thanks for your interest in the library! We don't have a definitive timeline for getting the ROS2 features into main (I've been chipping away at it as I find time), but hopefully no more than a month or so. I will throw out there that one of the biggest blockers for the ROS2 features is that the parameters in ROS2 have a more restricted set of types compared to the original XmlRpc design (namely no maps or mixed type sequences). At this point I'm pretty confident in how to implement a workaround, but it will be static and not involve the ROS2 parameters API.

re. dynamic parameters: We're not really considering any more ROS1 features given the end-of-life for ROS1 is coming up soon. We would definitely welcome contributions on the ROS2 side though! I had toyed with the idea of automatically registering parameters for dynamic updates, but didn't get very far. If you have ideas as to how to do this cleanly, we're all ears!

Best, Nathan

Schmluk commented 3 months ago

Hi @jessemorris ,

Thanks a lot for reaching out! Briefly also following up with some thoughts about the dynamic parameters:

If you want to give the design of a nice interface / mechanism a stab we'd be very happy to give feedback and about a contribution!

All the best, Lukas

jessemorris commented 3 months ago

Hi both,

@nathanhhughes I am happy to add features into your ros2 feature branch to move along the deployment - but I did have a look at your branch and it seems 'most' core features are already implemented. Please let me know.

As for dynamic parameters, I have done some automatic registration for dynamic updates, as you said, and have a good idea how to do this for config_utilities. To do a more general external interface for reconfiguration is more challenging. Happy to try and come back in a week or so with an interface design @Schmluk. To provide some context, what were the other existing interfaces that have been requested, other than ROS/ROS2?

Cheers, Jesse

Schmluk commented 3 months ago

Hi @jessemorris ,

That sounds great! I've pushed the draft for the dynamic updates that I had already (fairly incomplete but some core structure is there) here: https://github.com/MIT-SPARK/config_utilities/tree/feature/dynamic_configs

The idea, as for the rest of config utilities, is that the core libraries don't need to know about the implementation of interfaces downstream, so here we provide a DynamicConfigServer that can get/set all registered dynamic configs via YAML. Then e.g. the ROS specialization can tie these to ROS topics, companies that use config_utilities without ROS as a middleware can add TCP or other connections etc.

We will let you know once we have more details ready (at least for this initial draft the plan was to add a (simple) ROS example with a UI but not sure we'll get to this in the next few weeks). Any contributions or suggestions are very welcome!

jessemorris commented 3 months ago

Hi @Schmluk

I made a VERY preliminary pass on a different dynamic config implementation/interface.

Instead of having a separate type DynamicConfig<> for mutable configs I register config instances with a central registry with an associated Interface. The interface is the downstream interface you mention which can be specialized by the user. Ideally the interface just needs to know a key, or namespace, and the type of config/field it wants to update.

I have implemented a very simple example of this idea here: https://github.com/jessemorris/config_utilities/blob/feature/dynamic_config_alt/config_utilities/demos/demo_config.cpp (just in the main). Please take a look at the comment entitled Ideas where I explain how I think we can allow the interface to use namespacing to cleanly update only parts of a config. I think this goes some way to dealing with the last point in your earlier comment, as the interface only needs to deal with namespacing and does need a full understanding/definition of the actual config type. Maybe I have misunderstood your comment. I have not implemented this last part yet as I wanted to first get some feedback on the overall approach.

Cheers, Jesse

Schmluk commented 3 months ago

Hi @jessemorris

Apologies for the late reply, I just saw that my previous response somehow did not get published, so I'll try to replicated it below:

Many thanks for the draft, this looks really interesting! Can you maybe open a draft PR, that will make it easier to comment inline on your specific points.

On a higher-level note, I understand there to be different ideas of what the server is supposed to do. In my idea, the server is supposed to be a base interface that lives in the server-side app with configs (right box below), and that can be extended to communicate with other (custom) applications, i.e. following an architecture along these lines:

config_utilities_dynamic_conf_1

Other applications can then implement their servers, an example of this is shown in our RosDynamicConfigServer. From what I understand you are more interested in developing a C++ Dynamic Config Client? I think there are two important distinctions, if the client lives in the same application as the configs, it might probably be easiest to just have a global singleton that holds the configs and we don't need a complicated server/client interface? It might be useful to provide such a client in the general case though I think (see left box below). My understanding of what you're trying to achieve would be the boxes in orange:

config_utilities_dynamic_conf_2

Let me know what you think and whether this design makes sense. I've also added a bit more context and description to the demo, hopefully that helps, too!

jessemorris commented 3 months ago

Hi @Schmluk

I have made a draft PR. I agree with how my code fits into your design (the yellow boxes) but I had a slightly different view of how the overall server would behave. I'll try and summarize and provide a different perspective:

Maybe this is a minor point but hopefully you find it valid! Let me know what you think and how you would like to proceed

Jesse

Schmluk commented 2 months ago

Hi Jesse,

Thanks for your patience, I was on travel the last two weeks. I think I left some comments on the draft PR. W.r.t. the points above, I think both of these would be rather straight forward to implement in the proposed architecture, where they would be a specialization of the DynamicConfigServer (e.g. a RosParamDynamicConfigServer that binds the dynamic configs to ROS params rather than topics). I think we would very be happy to support this! Let me know what you think!

Edit: A nice feature of the proposed architecture is that mutiple specialized servers that wrap the internal server would work, e.g. one could update the configs via ROS params or via topic, and the servers would update the corresponding other outlets automatically.