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: modify some tests #1380

Merged
merged 8 commits into from May 6, 2022
Merged

Conversation

wep21
Copy link

@wep21 wep21 commented May 3, 2022

modify some header include orders and change some brackets to double quotes to pass the cpplint test.

@wep21
Copy link
Author

wep21 commented May 3, 2022

@jacobperron @j-rivero Could you review this PR?

@scpeters
Copy link
Member

scpeters commented May 3, 2022

so the .hh headers need to be enclosed in "" instead of <>?

@jacobperron
Copy link
Collaborator

This overlaps with #1355

Also, I'm proposing a change to ament_cpplint to fix a bug where .hh files are considered C system headers (which may affect this PR): ament/ament_lint#374

@jacobperron
Copy link
Collaborator

so the .hh headers need to be enclosed in "" instead of <>?

I think this is only a workaround, to trick cpplint into thinking they are not C system headers. After ament/ament_lint#374, using double-quotes shouldn't be necessary.

Daisuke Nishimatsu and others added 2 commits May 5, 2022 03:36
Signed-off-by: Daisuke Nishimatsu <border_goldenmarket@yahoo.co.jp>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
@jacobperron jacobperron self-assigned this May 4, 2022
@wep21 wep21 changed the title fix: include order fix: modify some tests May 4, 2022
@wep21
Copy link
Author

wep21 commented May 4, 2022

How about once ignoring cpplint include order until ament/ament_lint#374 is merged?

@jacobperron
Copy link
Collaborator

How about once ignoring cpplint include order until ament/ament_lint#374 is merged?

Sounds good to me. Thanks for working on it!

Daisuke Nishimatsu added 2 commits May 5, 2022 04:28
Signed-off-by: Daisuke Nishimatsu <border_goldenmarket@yahoo.co.jp>
Signed-off-by: Daisuke Nishimatsu <border_goldenmarket@yahoo.co.jp>
Daisuke Nishimatsu added 2 commits May 5, 2022 15:10
Signed-off-by: Daisuke Nishimatsu <border_goldenmarket@yahoo.co.jp>
Signed-off-by: Daisuke Nishimatsu <border_goldenmarket@yahoo.co.jp>
Signed-off-by: Daisuke Nishimatsu <border_goldenmarket@yahoo.co.jp>
@wep21 wep21 requested a review from jacobperron May 5, 2022 13:20
@jacobperron
Copy link
Collaborator

@ros-pull-request-builder retest this please

@jacobperron
Copy link
Collaborator

Looks like there are some, unrelated, flaky tests. I'll try triggering CI one more time.

@jacobperron
Copy link
Collaborator

@ros-pull-request-builder retest this please

Signed-off-by: Daisuke Nishimatsu <border_goldenmarket@yahoo.co.jp>
@jacobperron jacobperron merged commit c39b04c into ros-simulation:ros2 May 6, 2022
@wep21 wep21 deleted the fix/include-order branch May 6, 2022 22:09
jacobperron added a commit that referenced this pull request May 11, 2022
Partial backport of #1380.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
jacobperron added a commit that referenced this pull request May 11, 2022
Partial backport of #1380.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
@@ -162,7 +162,7 @@ void GazeboRosInit::Load(int argc, char ** argv)
// the simulation is paused), late subscribers can receive the previously published message(s).
impl_->clock_pub_ = impl_->ros_node_->create_publisher<rosgraph_msgs::msg::Clock>(
"/clock",
rclcpp::QoS(rclcpp::KeepLast(10)).transient_local());
rclcpp::ClockQoS());
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm having second thoughts about this change; I didn't realize the durability setting for rclcpp::ClockQoS is volatile (instead of transient local). I think it might be better to revert this change so we keep the same behavior for anyone depending on the behavior from early versions.

jacobperron added a commit that referenced this pull request May 11, 2022
Partial backport of #1380.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
afonsofonbraga pushed a commit to afonsofonbraga/gazebo_ros_pkgs that referenced this pull request Aug 4, 2022
Partial backport of ros-simulation#1380.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
GB255 pushed a commit to brisa-robotics/gazebo_ros_pkgs that referenced this pull request Aug 5, 2022
* fix: limits of steering speed control

* gazebo_plugins: GPS sensor plugin publishing velocity (ros-simulation#1371) (ros-simulation#1386)

* GPS sensor publishing velocity

Added velocity publisher to GPS sensor plugin. Velocity is published as Vector3Stamped message in ENU coordinates.

Since Foxy supports Gazebo 11 by default, GPS sensor implements VelocityEast, VelocityNorth and VelocityUp methods
that allow for velocity simulation.

* GPS sensor velocity tests

Testing velocity messages provided by GPS plugin.

Co-authored-by: Marcel Dudek <43888122+MarcelDudek@users.noreply.github.com>

* gazebo_ros: Add inline keyword to template definitions (ros-simulation#1367)

* gazebo_ros: Add inline keyword to template definitions

* Uncrustify

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

Co-authored-by: Gerard Heshusius <gHeshusius@lely.com>
Co-authored-by: Jacob Perron <jacob@openrobotics.org>

* Use shell=False in when launching gazebo (ros-simulation#1384)

Signed-off-by: Aditya <aditya050995@gmail.com>

* Fix sim time test (ros-simulation#1390)

Partial backport of ros-simulation#1380.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

Co-authored-by: Jacob Perron <jacob@openrobotics.org>
Co-authored-by: Marcel Dudek <43888122+MarcelDudek@users.noreply.github.com>
Co-authored-by: Gerard Heshusius <gh.heshusius@gmail.com>
Co-authored-by: Gerard Heshusius <gHeshusius@lely.com>
Co-authored-by: Aditya Pande <aditya050995@gmail.com>
Minipada pushed a commit to brisa-robotics/gazebo_ros_pkgs that referenced this pull request Nov 13, 2022
* fix: ignore include order for cpplint

* gazebo_ros: fix ament_flake8 complaints

* fix clock qos

* fix flake8

* suppress cmake warnings

* remove test for use sim time logic

Co-authored-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Daisuke Nishimatsu <border_goldenmarket@yahoo.co.jp>
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

3 participants