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

add option to support use_sim_time. #581

Merged
merged 3 commits into from
Feb 17, 2021
Merged

add option to support use_sim_time. #581

merged 3 commits into from
Feb 17, 2021

Conversation

fujitatomoya
Copy link
Collaborator

address #515

@fujitatomoya fujitatomoya mentioned this pull request Jan 7, 2021
Closed
@fujitatomoya
Copy link
Collaborator Author

CC: @clalancette @hidmic

@fujitatomoya
Copy link
Collaborator Author

@hidmic

friendly ping, could you share what you think?

hidmic
hidmic approved these changes Jan 22, 2021
View reviewed changes
Copy link
Contributor

@hidmic hidmic left a comment

Choose a reason for hiding this comment

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

Seems reasonable to me. A test would be ideal.

audrow
audrow reviewed Feb 5, 2021
View reviewed changes
Copy link
Member

@audrow audrow left a comment

Choose a reason for hiding this comment

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

Looks good to me, too. I also would like to see a test for this.

@audrow audrow self-assigned this Feb 5, 2021
@fujitatomoya
Copy link
Collaborator Author

@hidmic @audrow thanks for the review, will try to add test on this.

@fujitatomoya
add option to support use_sim_time.
e558e12 
Signed-off-by: Tomoya.Fujita <Tomoya.Fujita@sony.com>
@fujitatomoya
requirement is delay, not bandwidth.
3a6beee 
Signed-off-by: Tomoya.Fujita <Tomoya.Fujita@sony.com>
@fujitatomoya fujitatomoya force-pushed the topic-20210107-ros2cli-use_sim_time branch from 86d286c to 3a6beee Compare February 15, 2021 08:18
@fujitatomoya
add test for DirectNode Class.
9aa7a1d 
Signed-off-by: Tomoya.Fujita <Tomoya.Fujita@sony.com>
@fujitatomoya
Copy link
Collaborator Author

@hidmic @audrow friendly ping,

@fujitatomoya fujitatomoya added enhancement New feature or request in review Waiting for review (Kanban column) labels Feb 16, 2021
@fujitatomoya
Copy link
Collaborator Author

CI:

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

@audrow
Copy link
Member

audrow commented Feb 16, 2021

It probably also makes sense to run CI tests for ros2topic, since you are modifying ros2topic/ros2topic/verb/delay.py.

@fujitatomoya
Copy link
Collaborator Author

It probably also makes sense to run CI tests for ros2topic, since you are modifying ros2topic/ros2topic/verb/delay.py.

yeah, right. i will do that later.

@fujitatomoya
Copy link
Collaborator Author

CI with ros2cli and ros2topic

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

@audrow
Copy link
Member

audrow commented Feb 17, 2021

@fujitatomoya, I think you have permissions to squash and merge. If not, let me know. Thanks for the PR!

@fujitatomoya fujitatomoya merged commit 89f56e1 into ros2:master Feb 17, 2021
@clalancette clalancette mentioned this pull request Sep 21, 2022
Merged
@clalancette clalancette mentioned this pull request Mar 27, 2023
Merged
@christophfroehlich christophfroehlich mentioned this pull request Oct 10, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hidmic hidmic hidmic approved these changes

@audrow audrow audrow approved these changes

Assignees

@audrow audrow

Labels
enhancement New feature or request in review Waiting for review (Kanban column)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@fujitatomoya @audrow @hidmic