Skip to content

basic async service #1576

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

Closed
wants to merge 4 commits into from
Closed

basic async service #1576

wants to merge 4 commits into from

Conversation

rpaaron
Copy link
Contributor

@rpaaron rpaaron commented Mar 9, 2021

#491

naive prevention of handle_request from calling send_response. Instead let the client do it, when the result is available.

@rpaaron
Copy link
Contributor Author

rpaaron commented Mar 9, 2021

example client would look something like:

auto service = node.create_service_async<AddTwoInts>(
    "/add_two_ints",
    [&](const std::shared_ptr<rmw_request_id_t> request_header,
        const std::shared_ptr<AddTwoInts::Request> request,
        const std::shared_ptr<AddTwoInts::Response> response)
    {
        std::thread t([=]() {
            /* something that takes a long time */
            response->sum = request->a + request->b;
            service->send_response(request_header, response);
        });
        t.join();
    }
});

rpaaron added 4 commits May 10, 2021 08:40
Signed-off-by: Aaron Lipinski <aaron.lipinski@roboticsplus.co.nz>
Signed-off-by: Aaron Lipinski <aaron.lipinski@roboticsplus.co.nz>
Signed-off-by: Aaron Lipinski <aaron.lipinski@roboticsplus.co.nz>
Signed-off-by: Aaron Lipinski <aaron.lipinski@roboticsplus.co.nz>
@ivanpauno
Copy link
Member

I don't think we need more API for this, we can already do it using #1709.
@rpaaron Could you check that PR?

@ivanpauno
Copy link
Member

I don't think we need more API for this, we can already do it using #1709.

Unfortunately, I didn't see this PR when working on #1709, but the implementation is pretty similar.

@adityapande-1995
Copy link
Contributor

@rpaaron should we close this PR, now that #1709 has been merged ?

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

Successfully merging this pull request may close these issues.

3 participants