-
Notifications
You must be signed in to change notification settings - Fork 127
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Dashing support #19
Comments
I agree it definitely should be done but I don't think anybody is working on this right now. |
Yes, this would be great. I tried, on a whim, to get it to compile on my ROS2 eloquent install, and this failed. If you are interested in the error I got, I can send it along. |
Finally got some time to migrate back to this issue. It seems like the first set of issues is to adapt to using the IDL interface, as opposed to individual msg, srv, and action files, and is therefore related to #11. For example, I get the following:
Unfortunately, I haven't done much in this realm of ROS (or ROS2), so I wasn't sure how to proceed. Are there some examples you @lelongg may have, or suggestions? |
Quick update on stuff, I have a fork that I have been working on a bit. So far, ignoring the
@esteve and/or @lelongg, Do you have any intuition for what needs to be adjusted here for idl support? I haven't been able to find a good set of documentation about what we need. Maybe I'm just looking in the wrong place. |
It's been a while since I dived into this. One thing I remember is that if you use a message in some package in your workspace, the package containing this message's definition must be in this workspace as well. Otherwise the rust code generation is not triggered. Do you have quick instructions on how your issue can be reproduced ? |
Hi @lelongg, so far I haven't included any other packages in the workspace, just the ros2_rust package (my fork). Therefore, all I am doing is running It seems like |
Nice to see some activity here. I took a stab at updating rclrs to Eloquent a few months ago, but a got bit overwhelmed with the intricacies in the changes to rosidl in Eloquent. If we'd like to invest some time updating everything, perhaps we could just leapfrog to the next LTS with Foxy releasing soon? |
@ruffsl Yeah, I think those intricacies are the same ones that I am seeing here, haha. I think that may make sense. |
Do you know if rosidl can be considered stable yet ? Tracking changes to message generation internal details proved to be painful. I considered bypassing cmake based generation to use rust macro (as rosrust does) or build.rs based generation. How would you feel about this ? |
@lelongg I'm not sure, unfortunately. Maybe @dirk-thomas or some people from OSRF might be able to offer some suggestions in that area? |
In general for at least for Foxy? The Foxy API freeze deadline has passed, so I'd think the change log written in index page for the upcoming LTS is accurate. Newer point releases seem just to pertain to tests, documentation, or bug fixes.
I'd also like to move away from cmake, as it makes using a native cargo workflow more tricky. Although it seems ROS2 is heavily invested in CMake for packaging and IDL macros. Perhaps we could revisit |
@ruffsl Would there be any downsides of going all-in on |
The changes have been introduced for Dashing and are described in the migration section of that release: https://index.ros.org/doc/ros2/Releases/Release-Dashing-Diademata/#rosidl Nothing significant has changed in Eloquent or Foxy (the latter has a few headers moved which should be a simple search-n-replace). I can't predict the future but the
Existing interface packages are using CMake to trigger code generation. So for Rust there still must be a CMake part which is the extension point being triggered by each of these interface packages passing the interface files which should be processed. Everything in the process after this triggering can be written in whatever language you like. The existing code generators use CMake and Python to perform the work (Python logic e.g. converts |
Another alternative that I'm going to use in the Ada client library is linking to the C typesupports and use the introspection to generate the static Ada types. I already use the introspection functions for data access in the Ada library, so it should be relatively straightforward. Honestly, when I looked at the C++/Python templates for their struct generation I found impossible to understand/reuse that. |
Thanks @mosteo for that as a reference. I’ll take a look. |
If I remember correctly and if this has not changed, using the existing rosidl generator process with rust would really pay off if rust generation is included in rosidl_defaults, for the trigger to take place even when message from outside the current workspace are used. Is it an open alternative ? If so what would be the requirements for a new generator to be included in rosidl_defaults ? The other alternative I was referring to would completely bypass the cmake process and the generated code would be a build artifact of the end user package (like rosrust does).
I was not familiar with that step but I agree that duplicating the conversion of .msg to .idl seems like a waste of time. Maybe this can be prevented by having the .idl stored in the ament store, so we would be able to parse the it directly. |
You might want to take a look at the grammar / parser being used in
The logic in the |
I have some experience with templating (although not empy specifically). I can take a look at that as well. |
I see what you mean, however that doesn't seem too involved and should represent a reasonable amount of work with the right parser generator. If rust generator cannot become part of rosidl_default my preference would go to writing something like this as I find working with cmake very difficult (I don't know what tool to use to figure what's going on during a cmake build, so I have a hard time debugging) but it's very personal and probably not a valid technical argument. |
Thanks for these pointers, I will keep that in mind if I have to go back to templates. |
I can make necessary changes to the pipeline to get the build passing on my PR: #24. Which will pull in changes from: #20. The pipeline changes are probably the main thing to resolve here since it's blocking PRs. Afterwards @AndrewJSchoen your QOS changes can be added. |
Looks like switching the build over to foxy was enough to get it passing. |
I think this can be closed now, since we're already on Foxy+. If there is still desire to add Dashing to CI, please reopen. |
Do you have a plan to support Dashing?
Dashing is the first LTS ROS2 distro. I think it is great if ros2_rust supports it!
The text was updated successfully, but these errors were encountered: