Skip to content

Make ParameterService and Sync/AsyncParameterClient accept rclcpp::QoS#1978

Merged
sloretz merged 2 commits intoros2:rollingfrom
Barry-Xu-2018:topic-parameter-qos
Sep 9, 2022
Merged

Make ParameterService and Sync/AsyncParameterClient accept rclcpp::QoS#1978
sloretz merged 2 commits intoros2:rollingfrom
Barry-Xu-2018:topic-parameter-qos

Conversation

@Barry-Xu-2018
Copy link
Copy Markdown
Collaborator

@Barry-Xu-2018 Barry-Xu-2018 commented Aug 2, 2022

Address #1967

@Barry-Xu-2018
Copy link
Copy Markdown
Collaborator Author

Barry-Xu-2018 commented Aug 2, 2022

@sloretz @clalancette Please help to review this PR.

@fujitatomoya fujitatomoya requested a review from sloretz August 2, 2022 08:26
fujitatomoya
fujitatomoya approved these changes Aug 2, 2022
View reviewed changes
Copy link
Copy Markdown
Collaborator

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

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

overall, lgtm

@fujitatomoya
Copy link
Copy Markdown
Collaborator

fujitatomoya commented Aug 2, 2022

CI:

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

@Barry-Xu-2018
Copy link
Copy Markdown
Collaborator Author

Barry-Xu-2018 commented Aug 3, 2022

CI:

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

I will check failed test cases.

@fujitatomoya
Copy link
Copy Markdown
Collaborator

fujitatomoya commented Aug 3, 2022

@Barry-Xu-2018 CI parameters are incorrect for test, i will start the new CI. my bad, sorry.

@fujitatomoya
Copy link
Copy Markdown
Collaborator

fujitatomoya commented Aug 3, 2022

CI:

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

@Barry-Xu-2018
Copy link
Copy Markdown
Collaborator Author

Barry-Xu-2018 commented Aug 3, 2022

@Barry-Xu-2018 CI parameters are incorrect for test, i will start the new CI. my bad, sorry.

It's okay. I will check new test result.

@Barry-Xu-2018
Copy link
Copy Markdown
Collaborator Author

Barry-Xu-2018 commented Aug 3, 2022 •
edited
Loading

Linux-aarch64 Build unstable

After checking failed test case, I find all are related to flak8. The next build (https://ci.ros2.org/job/ci_linux-aarch64/11632/) in CI is also the same problem.

Error message:

Traceback (most recent call last):
  File "/home/jenkins-agent/workspace/ci_linux-aarch64/ws/install/ament_flake8/lib/python3.10/site-packages/ament_flake8/main.py", line 182, in parse_config_file
    opts_manager = manager.OptionManager(prog='flake8', version='4.0.0')
TypeError: OptionManager.__init__() got an unexpected keyword argument 'prog'

The current used version of flake8 is 8-5.0.3 which not support prog argument.
https://github.com/PyCQA/flake8/blob/5.0.3/src/flake8/options/manager.py#L315-L324

projectroot.flake8
projectroot.flake8
projectroot.flake8
projectroot.flake8
projectroot.flake8
projectroot.flake8_generated_launch
projectroot.flake8
projectroot.flake8
projectroot.flake8
projectroot.flake8_generated_launch
projectroot.flake8
projectroot.flake8
projectroot.flake8
projectroot.flake8
projectroot.flake8
projectroot.flake8
test_cli.flake8.xunit.missing_result
launch_testing_examples.test.test_flake8.test_flake8
test_tracetools_launch.test_tracetools_launch.test.test_flake8.test_flake8
test_launch_ros.test.test_flake8.test_flake8
ros2bag.test.test_flake8.test_flake8
ros2lifecycle.ros2lifecycle.test.test_flake8.test_flake8
ros2component.ros2component.test.test_flake8.test_flake8
test_cli_remapping.flake8.xunit.missing_result
test_communication.flake8.xunit.missing_result
pendulum_control.flake8.xunit.missing_result
test_tracetools.flake8.xunit.missing_result
rosbag2_py.flake8.xunit.missing_result
composition.flake8_generated_launch.xunit.missing_result
composition.flake8.xunit.missing_result
lifecycle.flake8.xunit.missing_result
interactive_markers.flake8.xunit.missing_result
robot_state_publisher.flake8.xunit.missing_result
tf2_geometry_msgs.flake8.xunit.missing_result
demo_nodes_cpp.flake8.xunit.missing_result
logging_demo.flake8_generated_launch.xunit.missing_result
tf2_sensor_msgs.flake8.xunit.missing_result
dummy_robot_bringup.flake8.xunit.missing_result

@clalancette
Copy link
Copy Markdown
Contributor

clalancette commented Aug 3, 2022

Yes, we had flake8 errors for the last couple of days. It should now be fixed.

@Barry-Xu-2018
Copy link
Copy Markdown
Collaborator Author

Barry-Xu-2018 commented Aug 4, 2022

@fujitatomoya Could you help run CI again ?

@fujitatomoya
Copy link
Copy Markdown
Collaborator

fujitatomoya commented Aug 4, 2022

CI:

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

@Barry-Xu-2018
Copy link
Copy Markdown
Collaborator Author

Barry-Xu-2018 commented Aug 16, 2022

For window test unstable, I find below error log.

14:05:15 [MSBuild] [-ERROR-] - 'C:/Program Files (x86)/Microsoft Visual Studio/2019/BuildTools/VC/Tools/MSVC/14.29.30133/include/xutility', IO exception has been thrown: java.nio.file.NoSuchFileException: C:\Program Files (x86)\Microsoft Visual Studio\2019\BuildTools\VC\Tools\MSVC\14.29.30133\include\xutility

It isn't related to change of this PR. It seems the test environment.
@fujitatomoya Could you help run windows test again ?

@fujitatomoya
Copy link
Copy Markdown
Collaborator

fujitatomoya commented Aug 16, 2022 •
edited
Loading

CI(Windows):

  • Windows Build Status

@Barry-Xu-2018
Avoid deprecated function using another deprecated function
5627160 
Signed-off-by: Barry Xu <barry.xu@sony.com>
@Barry-Xu-2018
Copy link
Copy Markdown
Collaborator Author

Barry-Xu-2018 commented Aug 17, 2022

Maybe test unstable on window is related to deprecated function using another deprecated function.
So I submitted 5627160 and test again on Windows

@iuhilnehc-ynos
Copy link
Copy Markdown
Collaborator

iuhilnehc-ynos commented Aug 17, 2022

  • Windows Build Status

@iuhilnehc-ynos
Copy link
Copy Markdown
Collaborator

iuhilnehc-ynos commented Aug 17, 2022

rerun CI(mainly for Linux and Linux-aarch64) as the source code was updated at #1978 (comment)

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

@Barry-Xu-2018
Copy link
Copy Markdown
Collaborator Author

Barry-Xu-2018 commented Aug 18, 2022

Friendly ping @sloretz

sloretz
sloretz approved these changes Sep 9, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@sloretz sloretz left a comment

Choose a reason for hiding this comment

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

This looks great!

@sloretz
Copy link
Copy Markdown
Contributor

sloretz commented Sep 9, 2022

Thank you for the PR! CI params and results LGTM.

@sloretz sloretz merged commit d0e1e83 into ros2:rolling Sep 9, 2022
@sloretz sloretz mentioned this pull request Sep 9, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sloretz sloretz sloretz approved these changes

@fujitatomoya fujitatomoya fujitatomoya approved these changes

Assignees

@Barry-Xu-2018 Barry-Xu-2018

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@Barry-Xu-2018 @fujitatomoya @clalancette @iuhilnehc-ynos @sloretz