Skip to content

use reinterpret_cast for function pointer conversion.#1919

Merged
fujitatomoya merged 1 commit intoros2:masterfrom
fujitatomoya:bugfix-20220418-function-pointer-callback
Apr 26, 2022
Merged

use reinterpret_cast for function pointer conversion.#1919
fujitatomoya merged 1 commit intoros2:masterfrom
fujitatomoya:bugfix-20220418-function-pointer-callback

Conversation

@fujitatomoya
Copy link
Copy Markdown
Collaborator

@fujitatomoya fujitatomoya commented Apr 18, 2022

address #1915

Signed-off-by: Tomoya Fujita Tomoya.Fujita@sony.com

@fujitatomoya
use reinterpret_cast for function pointer conversion.
174e4e0 
Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
@fujitatomoya
Copy link
Copy Markdown
Collaborator Author

fujitatomoya commented Apr 18, 2022

#1263 subscription callbacks do not have this problem, that has been verified with https://github.com/fujitatomoya/ros2_test_prover/blob/3f8005350d75b0da30bad734252966d7c6868ca1/prover_rclcpp/src/rclcpp_1915.cpp#L27-L31

christophebedard
christophebedard approved these changes Apr 18, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@christophebedard christophebedard left a comment

Choose a reason for hiding this comment

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

LGTM.

@christophebedard
Copy link
Copy Markdown
Member

christophebedard commented Apr 18, 2022

#1263 subscription callbacks do not have this problem, that has been verified with fujitatomoya/ros2_test_prover@3f80053/prover_rclcpp/src/rclcpp_1915.cpp#L27-L31

Yeah, if I remember correctly, function pointers and std::binds all get converted into std::function.

fujitatomoya
fujitatomoya commented Apr 18, 2022
View reviewed changes
Copy link
Copy Markdown
Collaborator Author

@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.

@christophebedard thanks for review, i will start the CI.

@fujitatomoya
Copy link
Copy Markdown
Collaborator Author

fujitatomoya commented Apr 18, 2022

CI:

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

@fujitatomoya
Copy link
Copy Markdown
Collaborator Author

fujitatomoya commented Apr 18, 2022

@sloretz @clalancette could you review this when you have time?

clalancette
clalancette approved these changes Apr 22, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@clalancette clalancette 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 very reasonable to me. Let's wait to merge this until after we branch off for Humble (probably today).

@fujitatomoya fujitatomoya merged commit a198067 into ros2:master Apr 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@clalancette clalancette clalancette approved these changes

@christophebedard christophebedard christophebedard approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@fujitatomoya @christophebedard @clalancette