search

gazebosim / gz-transport

Transport library for component communication based on publication/subscription and service calls.
https://gazebosim.org
Apache License 2.0
29 stars 36 forks source link

Oneway service request from the command line #477

Closed caguero closed 3 months ago

caguero commented 5 months ago

🦟 Bug fix

Partially fixes #97

Summary

This patch makes it possible to request a one-way service request from the command line. Before, it was possible passing --reptype gz.msgs.Empty but always getting the message Service call timed out as an output (because the service callback wasn't answering anything, which was expected).

How to test it?

You can compile the examples, and then:

  1. Launch the one-way responser:

    ./responser_oneway

  2. Request the service using the two-way request and verify that you don't get the timeout:

    gz service -s /oneway --reqtype gz.msgs.StringMsg --reptype gz.msgs.Empty --req 'data: "Hello"' --timeout 1000

  3. Use the new one-way request and verify that you don't get the timeout message:

    gz service -s /oneway --reqtype gz.msgs.StringMsg --req 'data: "Hello"'

Note that both requests should reach the responser:

Request received: [Hello]
Request received: [Hello]

Checklist

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

codecov[bot] commented 5 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 87.67%. Comparing base (eac2e69) to head (687edcc). Report is 3 commits behind head on gz-transport13.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## gz-transport13 #477 +/- ## ================================================== - Coverage 87.69% 87.67% -0.03% ================================================== Files 59 59 Lines 5704 5710 +6 ================================================== + Hits 5002 5006 +4 - Misses 702 704 +2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

azeey commented 5 months ago

Would it be possible to detect that the response type is Empty instead of relying on the timeout to decide that the request is one way? I like the idea of making some of the arguments optional (see https://github.com/gazebosim/gz-transport/issues/475, https://github.com/gazebosim/gz-transport/issues/474, https://github.com/gazebosim/gz-transport/issues/473), but there is a lot of existing documentation that uses the --reptype gz.msgs.Empty and --timeout arguments. It would be nice if those commands didn't timeout as well.

caguero commented 4 months ago

26d669b

Sounds good to me. Then, now both:

gz service -s /oneway --reqtype gz.msgs.StringMsg --reptype gz.msgs.Empty --req 'data: "Hello"' --timeout 10

and

gz service -s /oneway --reqtype gz.msgs.StringMsg --req 'data: "Hello"'

shoudn't generate timeouts.

caguero commented 4 months ago

@j-rivero , do you know why the same job (Ubuntu CI / Ubuntu Jammy CI) can pass and fail at the same time? It seems that GZ_PATH and g_gzVersion is undefined.

azeey commented 3 months ago

There have been some changes in https://github.com/gazebosim/gz-transport/pull/481 that affect this PR. I suggest merging from gz-transport13.

caguero commented 3 months ago

Good to go again!