-
Notifications
You must be signed in to change notification settings - Fork 7
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
Add ROSIDL enum support #20
Conversation
Signed-off-by: Roman Sokolkov <roman.sokolkov@apex.ai>
@audrow could you please take a look? |
This pull request has been mentioned on ROS Discourse. There might be relevant details there: https://discourse.ros.org/t/need-reviewers-for-rosidl-related-change/26285/1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine to me, but let's make sure than this and the related PR don't break things downstream.
@audrow thanks for review, I fixed header names in this PR. I was able to reproduce the problem locally, seems I renamed
Locally I was able to compile successfully whole ros2 workspace with my forks. |
@audrow do you know may be some ROS working group , where I can present this feature and may be get some feedback? |
Maybe the Middleware Working Group? @wjwwood or @clalancette, any thoughts? |
Thanks @audrow. Could you please rerun CI like you did here? #20 (comment) |
I suspect the job failing is happening because this PR hasn't been merged yet. If CI comes up happy we can merge this and 🤞 |
@methylDragon thanks, we discovered issue with ROS Services support and currently working on it. I'll ping you, folks, as soon as it will be ready. |
Closing as I have no plans to work on this in the near time (see more details here). |
Signed-off-by: Roman Sokolkov roman.sokolkov@apex.ai
Main PR ros2/rosidl#685
Related issue ros2/rosidl#260