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

Rmw preallocate #160

Merged
merged 9 commits into from
May 2, 2019
Merged

Rmw preallocate #160

merged 9 commits into from
May 2, 2019

Conversation

mjcarroll
Copy link
Member

Connects to #159

@mjcarroll mjcarroll added the in progress Actively being worked on (Kanban column) label Nov 21, 2018
gonzodepedro pushed a commit to ros2/rcl that referenced this pull request Feb 7, 2019
Signed-off-by: Gonzalo de Pedro <gonzalo@depedro.com.ar>
gonzodepedro pushed a commit to ros2/rcl that referenced this pull request Mar 28, 2019
Signed-off-by: Gonzalo de Pedro <gonzalo@depedro.com.ar>
@mjcarroll mjcarroll force-pushed the rmw_preallocate branch 2 times, most recently from 9cbe774 to e6a1577 Compare April 30, 2019 14:22
mjcarroll pushed a commit to ros2/rcl that referenced this pull request Apr 30, 2019
Signed-off-by: Gonzalo de Pedro <gonzalo@depedro.com.ar>
Copy link
Member

@wjwwood wjwwood left a comment

Choose a reason for hiding this comment

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

Should the rmw_*_serialized_message() functions be affected as well? (not rhetorical)

rmw/include/rmw/rmw.h Outdated Show resolved Hide resolved
rmw/include/rmw/rmw.h Show resolved Hide resolved
rmw/include/rmw/rmw.h Show resolved Hide resolved
rmw/include/rmw/rmw.h Show resolved Hide resolved
rmw/include/rmw/rmw.h Show resolved Hide resolved
@mjcarroll mjcarroll added in review Waiting for review (Kanban column) and removed in progress Actively being worked on (Kanban column) labels Apr 30, 2019
Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

This one looks generally OK, though I agree with most of @wjwwood 's comments that are still open about parameter order and documentation.

rmw/include/rmw/rmw.h Show resolved Hide resolved
@mjcarroll
Copy link
Member Author

Addressed feedback and added documentation, please take another look.

Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

Lots of little nits in the documentation. Otherwise, I think this is looking good (with the knowledge that the upper layers will have to change because of the change in API).

rmw/include/rmw/rmw.h Outdated Show resolved Hide resolved
rmw/include/rmw/rmw.h Outdated Show resolved Hide resolved
rmw/include/rmw/rmw.h Outdated Show resolved Hide resolved
rmw/include/rmw/rmw.h Outdated Show resolved Hide resolved
rmw/include/rmw/rmw.h Outdated Show resolved Hide resolved
rmw/include/rmw/rmw.h Outdated Show resolved Hide resolved
rmw/include/rmw/rmw.h Outdated Show resolved Hide resolved
rmw/include/rmw/rmw.h Outdated Show resolved Hide resolved
rmw/include/rmw/rmw.h Outdated Show resolved Hide resolved
rmw/include/rmw/rmw.h Outdated Show resolved Hide resolved
@mjcarroll mjcarroll force-pushed the rmw_preallocate branch 2 times, most recently from c718a57 to 680c578 Compare May 1, 2019 15:23
Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

Lots of documentation nits, all but one for consistency. The last one is just a missing field that is not documented. Since this is all small potatoes, I'll approve anyway.

rmw/include/rmw/rmw.h Outdated Show resolved Hide resolved
rmw/include/rmw/rmw.h Outdated Show resolved Hide resolved
rmw/include/rmw/rmw.h Outdated Show resolved Hide resolved
rmw/include/rmw/rmw.h Outdated Show resolved Hide resolved
rmw/include/rmw/rmw.h Outdated Show resolved Hide resolved
rmw/include/rmw/rmw.h Outdated Show resolved Hide resolved
rmw/include/rmw/rmw.h Outdated Show resolved Hide resolved
rmw/include/rmw/rmw.h Outdated Show resolved Hide resolved
rmw/include/rmw/rmw.h Outdated Show resolved Hide resolved
rmw/include/rmw/rmw.h Show resolved Hide resolved
@mjcarroll
Copy link
Member Author

CI Status (all rmw_preallocate branches)

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

mjcarroll pushed a commit to ros2/rcl that referenced this pull request May 2, 2019
Signed-off-by: Gonzalo de Pedro <gonzalo@depedro.com.ar>
@mjcarroll
Copy link
Member Author

Once more after some rebases:

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

mjcarroll and others added 2 commits May 1, 2019 23:05
Signed-off-by: Michael Carroll <michael@openrobotics.org>
Signed-off-by: Gonzalo de Pedro <gonzalo@depedro.com.ar>
Signed-off-by: Michael Carroll <michael@openrobotics.org>
Gonzalo de Pedro and others added 7 commits May 1, 2019 23:05
Signed-off-by: Michael Carroll <michael@openrobotics.org>
Signed-off-by: Michael Carroll <michael@openrobotics.org>
Signed-off-by: Michael Carroll <michael@openrobotics.org>
Co-Authored-By: mjcarroll <michael@openrobotics.org>
Signed-off-by: Michael Carroll <michael@openrobotics.org>
Additionally updates documentation accordingly.

Signed-off-by: Michael Carroll <michael@openrobotics.org>
Co-Authored-By: mjcarroll <michael@openrobotics.org>
Signed-off-by: Michael Carroll <michael@openrobotics.org>
Signed-off-by: Michael Carroll <michael@openrobotics.org>
@wjwwood
Copy link
Member

wjwwood commented May 2, 2019

There are warnings on Windows, but they're in the generated IDL code.

I looked at the CI job and it looks like it is pulling in changes from more repositories than I expected:

21:03:18 ==> vcs custom . --args checkout -b rmw_preallocate --track origin/rmw_preallocate
21:03:19 EEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEE....EE..EEEEEEE.E.EEEEEEEEEEE..
21:03:19 === .\src\ament\ament_cmake (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ament\ament_index (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ament\ament_lint (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ament\ament_package (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ament\googletest (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ament\uncrustify_vendor (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\eProsima\Fast-CDR (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\eProsima\Fast-RTPS (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\osrf\osrf_pycommon (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\osrf\osrf_testing_tools_cpp (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros-perception\laser_geometry (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros-planning\navigation_msgs (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros-visualization\python_qt_binding (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros-visualization\qt_gui_core (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros-visualization\rqt (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros-visualization\rqt_console (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros-visualization\rqt_msg (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros-visualization\rqt_plot (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros-visualization\rqt_publisher (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros-visualization\rqt_py_console (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros-visualization\rqt_service_caller (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros-visualization\rqt_shell (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros-visualization\rqt_srv (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros-visualization\rqt_top (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros2\ament_cmake_ros (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros2\common_interfaces (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros2\console_bridge_vendor (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros2\demos (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros2\example_interfaces (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros2\examples (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros2\geometry2 (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros2\kdl_parser (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros2\launch (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros2\launch_ros (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros2\libyaml_vendor (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros2\message_filters (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros2\orocos_kinematics_dynamics (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros2\poco_vendor (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros2\rcl (git) ===
21:03:19 Branch 'rmw_preallocate' set up to track remote branch 'rmw_preallocate' from 'origin'.
21:03:19 Switched to a new branch 'rmw_preallocate'
21:03:19 === .\src\ros2\rcl_interfaces (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros2\rcl_logging (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros2\rclcpp (git) ===
21:03:19 Branch 'rmw_preallocate' set up to track remote branch 'rmw_preallocate' from 'origin'.
21:03:19 Switched to a new branch 'rmw_preallocate'
21:03:19 === .\src\ros2\rclpy (git) ===
21:03:19 Branch 'rmw_preallocate' set up to track remote branch 'rmw_preallocate' from 'origin'.
21:03:19 Switched to a new branch 'rmw_preallocate'
21:03:19 === .\src\ros2\rcpputils (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros2\rcutils (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros2\realtime_support (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros2\rmw (git) ===
21:03:19 Branch 'rmw_preallocate' set up to track remote branch 'rmw_preallocate' from 'origin'.
21:03:19 Switched to a new branch 'rmw_preallocate'
21:03:19 === .\src\ros2\rmw_connext (git) ===
21:03:19 Branch 'rmw_preallocate' set up to track remote branch 'rmw_preallocate' from 'origin'.
21:03:19 Switched to a new branch 'rmw_preallocate'
21:03:19 === .\src\ros2\rmw_fastrtps (git) ===
21:03:19 Branch 'rmw_preallocate' set up to track remote branch 'rmw_preallocate' from 'origin'.
21:03:19 Switched to a new branch 'rmw_preallocate'
21:03:19 === .\src\ros2\rmw_implementation (git) ===
21:03:19 Branch 'rmw_preallocate' set up to track remote branch 'rmw_preallocate' from 'origin'.
21:03:19 Switched to a new branch 'rmw_preallocate'
21:03:19 === .\src\ros2\rmw_opensplice (git) ===
21:03:19 Branch 'rmw_preallocate' set up to track remote branch 'rmw_preallocate' from 'origin'.
21:03:19 Switched to a new branch 'rmw_preallocate'
21:03:19 === .\src\ros2\robot_state_publisher (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros2\ros1_bridge (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros2\ros2cli (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros2\rosidl (git) ===
21:03:19 Branch 'rmw_preallocate' set up to track remote branch 'rmw_preallocate' from 'origin'.
21:03:19 Switched to a new branch 'rmw_preallocate'
21:03:19 === .\src\ros2\rosidl_dds (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros2\rosidl_defaults (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros2\rosidl_python (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros2\rosidl_typesupport (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros2\rosidl_typesupport_connext (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros2\rosidl_typesupport_fastrtps (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros2\rosidl_typesupport_opensplice (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros2\rviz (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros2\sros2 (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros2\system_tests (git) ===
21:03:19 Branch 'rmw_preallocate' set up to track remote branch 'rmw_preallocate' from 'origin'.
21:03:19 Switched to a new branch 'rmw_preallocate'
21:03:19 === .\src\ros2\test_interface_files (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros2\tinydir_vendor (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros2\tinyxml2_vendor (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros2\tinyxml_vendor (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros2\tlsf (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros2\unique_identifier_msgs (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros2\urdf (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros2\urdfdom (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros2\yaml_cpp_vendor (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros\class_loader (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros\pluginlib (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros\resource_retriever (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros\ros_environment (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it
21:03:19 === .\src\ros\urdfdom_headers (git) ===
21:03:19 fatal: 'origin/rmw_preallocate' is not a commit and a branch 'rmw_preallocate' cannot be created from it

Specifically ros2/rosidl, which may be the cause of the compiler warnings on Windows. So, I'm just not sure, so I'll leave this until tomorrow morning.

@mjcarroll
Copy link
Member Author

That looks to be the correct set of repositories. Were there any in there that you weren't expecting?

@mjcarroll
Copy link
Member Author

mjcarroll commented May 2, 2019

Looking at the CI, I don't believe that those (edit: MSBuild warnings) are related to anything that I would have changed, it's in the actual generated message code. @jacobperron could this be related to the changes you made yesterday?

@dirk-thomas
Copy link
Member

These CI warnings https://ci.ros2.org/job/ci_windows/6715/warnings43Result/ should be addressed by ros2/rosidl#375.

@mjcarroll mjcarroll merged commit 2a30008 into master May 2, 2019
@delete-merged-branch delete-merged-branch bot deleted the rmw_preallocate branch May 2, 2019 16:30
@mjcarroll mjcarroll removed the in review Waiting for review (Kanban column) label May 2, 2019
mjcarroll added a commit to ros2/rcl that referenced this pull request May 2, 2019
* Proposola of changes for RMW_Preallocate. Related /ros2/rmw#160

Signed-off-by: Gonzalo de Pedro <gonzalo@depedro.com.ar>

* Changed RCL interface

Signed-off-by: Gonzalo de Pedro <gonzalo@depedro.com.ar>

* Updates for allocation in serialize methods.

Signed-off-by: Michael Carroll <michael@openrobotics.org>

* Fix tests for new APIs.

Signed-off-by: Michael Carroll <michael@openrobotics.org>
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.

None yet

4 participants