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.

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

subscriber callbacks bound to class methods do not compile #173

Closed
tfoote opened this issue Dec 11, 2015 · 9 comments
Closed

subscriber callbacks bound to class methods do not compile #173

tfoote opened this issue Dec 11, 2015 · 9 comments
Assignees

Comments

@tfoote
Copy link
Contributor

tfoote commented Dec 11, 2015

It fails due to an inability to resolve an overloaded function.

/home/tfoote/work/ros2/tf3/install/include/rclcpp/function_traits.hpp:51:85: error: decltype cannot resolve address of overloaded function
       typename function_traits<decltype( & FunctionT::operator())>::arguments>::type;

The full error is here: https://gist.github.com/tfoote/17222805a01e7c05c3f1

@tfoote tfoote self-assigned this Dec 11, 2015
@tfoote tfoote added the in progress Actively being worked on (Kanban column) label Dec 11, 2015
@jacquelinekay
Copy link
Contributor

Can you make this a test case in system_tests/test_rclcpp instead? (I meant to post this comment on the pull request in examples, sorry if that was confusing.)

@jacquelinekay
Copy link
Contributor

Are you working on a fix for this right now? I will take a look if you're not actively working on it.

@tfoote
Copy link
Contributor Author

tfoote commented Dec 11, 2015

@esteve found a workaround to use a lambda function. I'll add a unit test for it too.

@jacquelinekay
Copy link
Contributor

You mean using a lambda function in the example? Or to make bound functions work as the input to create_subscription?

None of our tested rclcpp code so far uses std::bind. Any code that captures arguments does so using lambdas.

Playing around with this, I found another "workaround" that compiles:

std::function<void(const std_msgs::msg::String::SharedPtr)> callback = std::bind(&Chatter::chatterCallback, &chatter, std::placeholders::_1);
  auto sub = node->create_subscription<std_msgs::msg::String>(
    "chatter", callback, rmw_qos_profile_default);

Ideally we would support any valid callback signature for create_subscription, so I think we should do the necessary work to make your example compile.

@esteve
Copy link
Member

esteve commented Dec 11, 2015

@tfoote could you have a look at this branch and see if it works? Thanks.

https://github.com/ros2/rclcpp/tree/std_bind_function_traits

@esteve
Copy link
Member

esteve commented Dec 11, 2015

@tfoote I got your example compiling with my branch, I'll clean up the changes here and submit them for review.

@tfoote
Copy link
Contributor Author

tfoote commented Dec 12, 2015

@esteve I can confirm that your branch works for me, but I see it's not a cross platform solution unfortunately :-(

tfoote added a commit to ros2/system_tests that referenced this issue Dec 15, 2015
@tfoote tfoote removed the in progress Actively being worked on (Kanban column) label Dec 15, 2015
@tfoote tfoote removed their assignment Dec 15, 2015
@esteve esteve self-assigned this Dec 15, 2015
@esteve esteve added the in progress Actively being worked on (Kanban column) label Dec 15, 2015
@esteve
Copy link
Member

esteve commented Dec 16, 2015

@tfoote I got the workaround to compile on all three platforms in #183, I'll see if I can compile the std::bind test in ros2/system_tests#88

tfoote added a commit to ros2/system_tests that referenced this issue Dec 16, 2015
tfoote added a commit to ros2/system_tests that referenced this issue Dec 16, 2015
tfoote added a commit to ros2/system_tests that referenced this issue Dec 16, 2015
tfoote added a commit to ros2/system_tests that referenced this issue Dec 16, 2015
tfoote added a commit to ros2/system_tests that referenced this issue Dec 16, 2015
@tfoote
Copy link
Contributor Author

tfoote commented Dec 17, 2015

fixed in #183

@tfoote tfoote closed this as completed Dec 17, 2015
@tfoote tfoote removed the in progress Actively being worked on (Kanban column) label Dec 17, 2015
nnmm pushed a commit to ApexAI/rclcpp that referenced this issue Jul 9, 2022
ros2#173)

* expand rcl_node_is_valid with allocator to standarize the node check across RCL

Current RCL use very different way for node check in different components. Actually,
node.c already provide a standard way - rcl_node_is_valid. However, it just use
internal allocator, but some context has external allocator. The commit expand
rcl_node_is_valid with one more parameter for allocator. It can be used in all cases.

Signed-off-by: jwang <jing.j.wang@intel.com>

* Add allocator check in rcl_node_is_valid

* Separate argument checks from the concept of what makes a node invalid

* clarify allocator only used for error messages; default if NULL

* shorten parameter description
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants