Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[rosidl_adapter] Fix how comments in action interfaces are processed. #632

Merged
merged 10 commits into from
Jan 13, 2022
Merged

[rosidl_adapter] Fix how comments in action interfaces are processed. #632

merged 10 commits into from
Jan 13, 2022

Conversation

ivanpauno
Copy link
Member

Add also tests for msg and srv interfaces.

I also wanted to add a top level srv or action doc section.
e.g.

# top level srv docs

# top level request docs

# field docs
int my_field
---
# top level response docs

int response_field

but there's nowhere where we can put that in the current mapping to DDS idl, so I abandoned that idea.

@ivanpauno
Fix actions feedback/result comments parsing
483cab8 
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
@ivanpauno
Test rosidl_adapter comments in .msg files
9f8c1a8 
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
@ivanpauno
Test rosidl_adapter comments in .srv files
85af543 
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
@ivanpauno
Add extract_file_level_comments() function
0805814 
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
@ivanpauno ivanpauno added the enhancement New feature or request label Nov 15, 2021
@ivanpauno ivanpauno self-assigned this Nov 15, 2021
jacobperron
jacobperron approved these changes Dec 1, 2021
View reviewed changes
Copy link
Member

@jacobperron jacobperron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Two minor comments below.

rosidl_adapter/test/data/action/Test.action Show resolved Hide resolved
rosidl_adapter/rosidl_adapter/parser.py Outdated Show resolved Hide resolved
@ivanpauno
remove unnecessary line
5b42eb0 
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
@ivanpauno
Add extra test case
02ee800 
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
@ivanpauno
Copy link
Member Author

ivanpauno commented Dec 2, 2021 •
edited
Loading

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@jacobperron
Copy link
Member

Looks like some downstream tests in ros2interface are affected by this change.

@ivanpauno
fix corner case
38236c9 
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
@ivanpauno
Copy link
Member Author

Should have been fixed after 38236c9.

I also noticed that ros2interface show is making some wrong assumptions about parse_message_string(), I will fix that in another PR.

@ivanpauno
Copy link
Member Author

CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

hidmic
hidmic reviewed Dec 6, 2021
View reviewed changes
rosidl_adapter/rosidl_adapter/parser.py Outdated Show resolved Hide resolved
rosidl_adapter/rosidl_adapter/parser.py Outdated Show resolved Hide resolved
rosidl_adapter/rosidl_adapter/parser.py Outdated Show resolved Hide resolved
ivanpauno and others added 3 commits January 12, 2022 12:57
@ivanpauno @hidmic
Fix string.replace() call
8cca814 
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

Co-authored-by: Michel Hidalgo <michel@ekumenlabs.com>
@ivanpauno @hidmic
Use more idiomatic slice access ([0:index] -> [:index])
b9710b6 
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

Co-authored-by: Michel Hidalgo <michel@ekumenlabs.com>
@ivanpauno
rename variable other->file_content
32ef5a3 
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
@ivanpauno ivanpauno requested a review from hidmic January 12, 2022 16:04
@ivanpauno
Copy link
Member Author

ivanpauno commented Jan 12, 2022 •
edited
Loading

CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status (unrelated issues)

@ivanpauno ivanpauno merged commit 2efb51f into master Jan 13, 2022
@delete-merged-branch delete-merged-branch bot deleted the ivanpauno/rosidl-adapter-srv-action-comments-parsing branch January 13, 2022 16:40
@ivanpauno ivanpauno mentioned this pull request Jan 13, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jacobperron jacobperron jacobperron approved these changes

@hidmic hidmic Awaiting requested review from hidmic

Assignees

@ivanpauno ivanpauno

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@ivanpauno @jacobperron @hidmic