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.

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

Fix for actions subfolder introduction in ros2 message bridge #143

Merged
merged 5 commits into from
Nov 15, 2018

Conversation

sservulo
Copy link
Contributor

Relates to the change ros2/rosidl@7d79172 pointed out by @mjcarroll and @dirk-thomas which introduced packaging failures in ros1_bridge.

Deprecates ros2/rosidl#319 .

@tfoote tfoote added the in progress Actively being worked on (Kanban column) label Nov 13, 2018
@mjcarroll
Copy link
Member

mjcarroll commented Nov 13, 2018

Linux packaging: Build Status

@mjcarroll
Copy link
Member

Looks like the tests are still timing out on the farm.

@dirk-thomas
Copy link
Member

dirk-thomas commented Nov 13, 2018

@sservulo Please build the branch locally and run the tests, e.g. the linters fail which can easily be tested locally.

@mjcarroll
Copy link
Member

Yes, there are some flake8 errors, and it looks like service pairs aren't being correctly generated.

@sservulo
Copy link
Contributor Author

Sorry, I did a small amend and didn't rerun tests before pushing it. Lint errors fixed now. I'm digging the issue in ros1_bridge for similar issues as the one pointed out in message naming, like in service mapping. Any other particular points of interest I should be aware of?

ros2_msg.prefix_path, 'share', ros2_msg.package_name, message_location,
ros2_msg.message_name + '.msg')
if os.path.isfile(message_path):
break
Copy link
Member

Choose a reason for hiding this comment

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

To different type of interfaces with the same name can exist. So only allowing one here seems wrong.

Same below.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just to double check, this PR is specifically about stripping subfolders from interface files, not to provide any support for bridging actions, right? If that's the case, then it seems to me we don't need to expose messages generated to implement services nor messages and services generated to implement actions.

@apojomovsky
Copy link

Linux packaging: Build Status

@apojomovsky
Copy link

Linux packaging: Build Status

@mjcarroll
Copy link
Member

mjcarroll commented Nov 14, 2018

For good measure:

  • Windows packaging: Build Status
  • MacOS packaging: Build Status
  • aarch64 packaging: Build Status

@sloretz
Copy link
Contributor

sloretz commented Nov 14, 2018

Windows packaging job has warnings.

I don't think the warning script is catching this one, not sure how long it's been present.

warning: install_lib: 'C:\J\workspace\ci_packaging_windows\ws\build\test_launch_ros\build\lib' does not exist -- no Python modules to install

and

c:\j\workspace\ci_packaging_windows\ws\install\include\urdf_model\color.h(79): warning C4244: 'argument': conversion from 'double' to 'const float', possible loss of data [C:\J\workspace\ci_packaging_windows\ws\build\robot_state_publisher\robot_state_publisher_solver.vcxproj

which might be caused by ros/urdfdom_headers#47/

@mjcarroll
Copy link
Member

I don't think either of those are a blocker, since they are from unrelated packages.

@sservulo
Copy link
Contributor Author

@mjcarroll Is this ready to move forward?

@mjcarroll mjcarroll merged commit e302141 into ros2:master Nov 15, 2018
@mjcarroll mjcarroll removed the in progress Actively being worked on (Kanban column) label Nov 15, 2018
dhananjaysathe pushed a commit to rapyuta-robotics/ros1_bridge that referenced this pull request Aug 22, 2019
* Fix for actions subfolder introduction in ros2 message bridge

* Updating service_name for action srvs for compatibility

* Extended changes on ros1_bridge

* Undo of action specific locations to ros1_bridge

* lint fixup

Signed-off-by: Dhananjay Sathe <dhananjay.sathe@rapyuta-robotics.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants