8000 Store reference to rcl_clock_t instead of copy by sloretz · Pull Request #797 · ros2/rcl · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Store reference to rcl_clock_t instead of copy #797

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

Merged
merged 1 commit into from
Sep 11, 2020

Conversation

sloretz
Copy link
Contributor
@sloretz sloretz commented Sep 10, 2020

rcl_clock_t can't be trivially copied because adding or removing clock
jump callbacks to a copy will free the pointer held by the original
instance.

Fixes this bug #795 (comment)

Which caused tf2_ros test_buffer_server to fail on all nightly jobs

rcl_clock_t can't be trivially copied because adding or removing clock
jump callbacks to a copy will free the pointer held by the original
instance.

Signed-off-by: Shane Loretz<sloretz@openrobotics.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
@sloretz sloretz added the bug Something isn't working label Sep 10, 2020
@sloretz sloretz requested a review from brawner September 10, 2020 22:44
@sloretz sloretz self-assigned this Sep 10, 2020
@sloretz
Copy link
Contributor Author
sloretz commented Sep 10, 2020

CI (build: --packages-up-to rcl_action tf2_ros test: --packages-select rcl_action tf2_ros)

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

@brawner
Copy link
Contributor
brawner commented Sep 10, 2020

This looks good to me. Let me fix up my rclcpp_action test-coverage PR and then I'll run that again on this PR to check

@sloretz
Copy link
Contributor Author
sloretz commented Sep 10, 2020

CI again now that ros2/rosidl#525 was merged (build: --packages-up-to rcl_action tf2_ros test: --packages-select rcl_action tf2_ros)

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

Copy link
Contributor
@brawner brawner left a comment

Choose a reason for hiding this comment

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

This seems to work just fine with rclcpp_action as well as my new unit tests in ros2/rclcpp#1290

@sloretz sloretz merged commit 139cc45 into master Sep 11, 2020
@delete-merged-branch delete-merged-branch bot deleted the action_server_store_clock_reference branch September 11, 2020 00:51
@brawner
Copy link
Contributor
brawner commented Sep 11, 2020

This will probably need backporting with #795

sloretz added a commit that referenced this pull request Sep 18, 2020
rcl_clock_t can't be trivially copied because adding or removing clock
jump callbacks to a copy will free the pointer held by the original
instance.

Signed-off-by: Shane Loretz<sloretz@openrobotics.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
sloretz added a commit that referenced this pull request Sep 23, 2020
) Store reference to rcl_clock_t instead of copy (#797) (#805)

* Use valid clock in case of issue in rcl_timer_init (#795)

* Use valid clock in case of issue

Signed-off-by: Stephen Brawner <brawner@gmail.com>

* Cleanup

Signed-off-by: Stephen Brawner <brawner@gmail.com>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>

* Store reference to rcl_clock_t instead of copy (#797)

rcl_clock_t can't be trivially copied because adding or removing clock
jump callbacks to a copy will free the pointer held by the original
instance.

Signed-off-by: Shane Loretz<sloretz@openrobotics.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>

Co-authored-by: brawner <brawner@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0