-
Notifications
You must be signed in to change notification settings - Fork 272
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
Replace RCUTILS_ with RCLCPP_ for logging #62
Conversation
25c34b0
to
76d5366
Compare
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.
Thanks for PR. There are few nitpicks around alphabetical sort.
However, the github action fail with some uncrustify errors:
6: --- src/robot_hardware.cpp
6: +++ src/robot_hardware.cpp.uncrustify
6: @@ -83 +83,2 @@
6: - RCLCPP_ERROR(rclcpp::get_logger("joint state handle"),
6: + RCLCPP_ERROR(
6: + rclcpp::get_logger("joint state handle"),
6: @@ -97 +98,2 @@
6: - RCLCPP_ERROR(rclcpp::get_logger("joint state handle"),
6: + RCLCPP_ERROR(
6: + rclcpp::get_logger("joint state handle"),
6: @@ -111 +113,2 @@
6: - RCLCPP_ERROR(rclcpp::get_logger("joint cmd handle"),
6: + RCLCPP_ERROR(
6: + rclcpp::get_logger("joint cmd handle"),
6: @@ -125 +128,2 @@
Running ament_uncrustify locally gives no error, what could be the reason for this situation .? is there a config file used for uncrustify in the CI system .? I'll fix the other alpha sort comment today, Thanks! |
I suspect that you’re using eloquent but the current master branch is on par with the latest ros2 master and thus the uncrustify version used has changed.
That is annoying, but for this particular problem I believe you don’t have to build your ros2 version from scratch but solely rely on the linked github actions or travis.
… On Feb 24, 2020, at 6:16 AM, Jafar Abdi ***@***.***> wrote:
However, the github action fail with some uncrustify errors:
Running ament_uncrustify locally gives no error what could be the reason .?
I'll fix the other alpha sort comment today, Thanks!
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
aca19dd
to
27a4a0d
Compare
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.
This looks good to me. I would ask you though to once more iterate over it and see if we can get rid of the rcutils
dependency all together if not being used.
test_robot_hardware/include/test_robot_hardware/test_robot_hardware.hpp
Outdated
Show resolved
Hide resolved
@JafarAbdi Any updates on this? |
27a4a0d
to
c336ad4
Compare
@Karsten1987 Sorry, I just saw your last review, I addressed the latest review and pushed the changes |
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.
one outstanding comment, but looks good to me.
c336ad4
to
d342488
Compare
…olated (ros-controls#62) * port over jtc interpolation formulas * update comments * account for 0 duration between points, fix tests accordingly * interpolate() and sample() functions now returns velocities and accelerations * now aborting due to outside_goal_tolerance * update comment * add pos, velocity and acceleration tests to test_trajectory * fix action tests, disable velocity goal tolerance due to uncommitted velocities * lint and uncrustify fixes * Fix rebase hiccup Co-authored-by: Bence Magyar <bence.magyar.robotics@gmail.com>
Fix: #59