-
Notifications
You must be signed in to change notification settings - Fork 342
Fix UserCommands services so that they return the actual status the executed command #2999
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
+164
−40
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This removes a lot of repetitive code where a service handler simply copies the message, creates and adds the associated command to a command queue. Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
In `SimulationRunner`, we initialize a worker pool but we never actually use it so it's pure overhead. In `ServerPrivate` we only use the worker pool if there are multiple simulation runners, so we can optimize for the most common use case of one runner. Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
… executed command Previously, the services immediately returned true after adding the command to the pending commands queue which meant that the return statuses did not reflect the success or failure of the subsequent execution of the commands. Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
Otherwise, there will be a lock up because the service will block waiting for the command to be executed, but the command won't be executed until the `Server` is allowed to run. Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
Instead of breaking existing behavior, for each existing service, this adds a new service that is a blocking version of the service. Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
10 tasks
Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
iche033
approved these changes
Aug 20, 2025
Homebrew failures don't seem to be related to this PR. These tests have started failing in other PRs as well (https://build.osrfoundation.org/job/gz_sim-ci-pr_any-homebrew-amd64/1962) from #3005. |
https://github.com/Mergifyio backport gz-sim9 gz-sim8 |
✅ Backports have been created
|
mergify bot
pushed a commit
that referenced
this pull request
Oct 17, 2025
…xecuted command (#2999) Previously, the services immediately returned true after adding the command to the pending commands queue which meant that the return statuses did not reflect the success or failure of the subsequent execution of the commands. --------- Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org> (cherry picked from commit 590e0f4)
mergify bot
pushed a commit
that referenced
this pull request
Oct 17, 2025
…xecuted command (#2999) Previously, the services immediately returned true after adding the command to the pending commands queue which meant that the return statuses did not reflect the success or failure of the subsequent execution of the commands. --------- Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org> (cherry picked from commit 590e0f4)
This was referenced Oct 17, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🦟 Bug fix
Summary
Previously, the services immediately returned true after adding
the command to the pending commands queue which meant that
the return statuses did not reflect the success or failure of
the subsequent execution of the commands.
UserCommands
#2996Needed for gazebosim/ros_gz#790
Checklist
codecheck
passed (See contributing)Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining
Signed-off-by
andGenerated-by
messages.