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

fix: modify some tests #1380

Merged
merged 8 commits into from
May 6, 2022
Merged

fix: modify some tests #1380

merged 8 commits into from
May 6, 2022

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
fix: ignore include order for cpplint
5a71322 
Signed-off-by: Daisuke Nishimatsu <border_goldenmarket@yahoo.co.jp>
@scpeters
gazebo_ros: fix ament_flake8 complaints
9bfd464 
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
@wep21 wep21 force-pushed the fix/include-order branch from 90a43f5 to 9bfd464 Compare May 4, 2022 18:48
@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!

@jacobperron jacobperron mentioned this pull request May 4, 2022
Merged
Daisuke Nishimatsu added 2 commits May 5, 2022 04:28
fix clock qos
d2604d1 
Signed-off-by: Daisuke Nishimatsu <border_goldenmarket@yahoo.co.jp>
fix flake8
e0ddbde 
Signed-off-by: Daisuke Nishimatsu <border_goldenmarket@yahoo.co.jp>
Daisuke Nishimatsu added 2 commits May 5, 2022 15:10
fix clock qos
261df1f 
Signed-off-by: Daisuke Nishimatsu <border_goldenmarket@yahoo.co.jp>
fix condition of test sim time
8e314b2 
Signed-off-by: Daisuke Nishimatsu <border_goldenmarket@yahoo.co.jp>
@wep21 wep21 force-pushed the fix/include-order branch from b8788da to 8e314b2 Compare May 5, 2022 08:33
suppress cmake warnings
f35e510 
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

remove test for use sim time logic
1864f19 
Signed-off-by: Daisuke Nishimatsu <border_goldenmarket@yahoo.co.jp>
@jacobperron jacobperron merged commit c39b04c into ros-simulation:ros2 May 6, 2022
@jacobperron jacobperron mentioned this pull request May 6, 2022
Open
@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
@jacobperron
Fix sim time test
abc7b3e 
Partial backport of #1380.

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

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
jacobperron
jacobperron reviewed May 11, 2022
View reviewed changes
gazebo_ros/src/gazebo_ros_init.cpp
@@ -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
@jacobperron
Fix sim time test (#1390)
645e4d6 
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
@jacobperron @afonsofonbraga
Fix sim time test (ros-simulation#1390)
9ca358e 
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
@afonsofonbraga @jacobperron
@MarcelDudek @GerardHH @adityapande-1995
Sync upstream (#3)
8f87c06 
* 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
@wep21 @scpeters @Minipada
Fix test failures (ros-simulation#1380)
f43eae1 
* 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>
@scpeters scpeters mentioned this pull request Mar 5, 2023
Open
5 tasks
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

Assignees

@jacobperron jacobperron

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@wep21 @scpeters @jacobperron