feat: added support for spacecraft thrusters

[Pedro-Roque]

🎉 New feature

Summary

Adds support for Spacecraft thrusters controlled with a duty cycle signal (such as PWM). Method ported from PX4 Gazebo-Classic SITL implemented by @Jaeyoung-Lim

Test it

Checklist

• Signed all commits for DCO
• Added tests
• Added example and/or tutorial
• Updated documentation (as needed)
• Updated migration guide (as needed)
• Consider updating Python bindings (if the library has them)
• codecheck passed (See contributing)
• All tests passed (See test coverage)
• While waiting for a review on your PR, please help review another open pull request to support the maintainers

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.

[Pedro-Roque]

Thanks @ahcorde for the inputs! I had not yet started cleaning the branch as I had just finished the feature. I will address this in the coming days. Thanks!

[Pedro-Roque]

@ahcorde should be good for a review check. Let me know what else I should improve on it

[arjo129]

Thanks for this really awesome contribution. I've marked out some things that need changing. It would be nice if you had an example SDF and some unit tests also. These are useful as they allow us to make sure we don't introduce regressions when we make changes to the core simulation platform.

[Pedro-Roque]

Thanks for this really awesome contribution. I've marked out some things that need changing. It would be nice if you had an example SDF and some unit tests also. These are useful as they allow us to make sure we don't introduce regressions when we make changes to the core simulation platform.

Thanks @arjo129 ! Unfortunately right now I'm a bit swamped, so might take a bit more time before I add an example SDF and unit tests. Would this be a hard requirement for now, or is it ok if we can merge this in and then I add them?

On another note: I need this plugin to be available on gz-sim7 (since we are using gazebo garden for now). Should I do a PR targetting that branch too?

[arjo129]

I would say unit tests can go in later but we should have an example. At the very least thatll allow me to test it.

[Pedro-Roque]

@arjo129 I Just added a spacecraft world with the DART spacecraft (mass and inertia were severely reduced for faster visualization. Also updated the tutorial.

Moreover, I think that the mesh file should not be there but hosted on fuel, but I couldn't find my way to upload it there. Do I need extra permissions?

[arjo129]

You do need to sign in to upload files to fuel. Tbh you can just use a box if you're strapped for time.

[Pedro-Roque]

You do need to sign in to upload files to fuel. Tbh you can just use a box if you're strapped for time.

I'll try to do it now so that it's done properly at once. Standby,

[Pedro-Roque]

@arjo129 I've added my model in Fuel, but I'm getting this error even though I believe I have the file paths correctly set:

gz sim spacecraft.sdf
Warning: Ignoring XDG_SESSION_TYPE=wayland on Gnome. Use QT_QPA_PLATFORM=wayland to run on Wayland anyway.
[GUI] [Err] [SystemPaths.cc:425] Unable to find file with URI [file://dart/meshes/dart.dae]
[GUI] [Err] [SystemPaths.cc:525] Could not resolve file [file://dart/meshes/dart.dae]
[GUI] [Err] [MeshManager.cc:211] Unable to find file[file://dart/meshes/dart.dae]
[GUI] [Err] [SceneManager.cc:425] Failed to load geometry for visual: base_link_visual

Model is available here: https://app.gazebosim.org/proque/fuel/models/dart

Any idea of why this could be a problem?

[arjo129]

I wont be able to test this till tomorrow. Ill let you know how to fix it.

[Pedro-Roque]

I wont be able to test this till tomorrow. Ill let you know how to fix it.

No worries, let me know and I'll fix it over the weekend.

[arjo129]

Regarding the Fuel model. I had a look. You just need to remove the file:// prefix inthe mesh tag.

Also you need to re-sign off your commits: https://github.com/gazebosim/gz-sim/pull/2431/checks?check_run_id=25943650220

The automated linter also has a few complaints: https://github.com/gazebosim/gz-sim/actions/runs/9417757870/job/26011958069?pr=2431

[Pedro-Roque]

Thanks @arjo129 , just fixed the above items!

Also, to target gz garden, should I make a PR to that branch?

Best,
Pedro

[arjo129]

Also, to target gz garden, should I make a PR to that branch?

We try to avoid adding new features to older versions. If it really is needed we do do backports from time to time. Lets first merge it then it can be cherry picked later.

[Pedro-Roque]

Sounds good @arjo129 !

Let me know if I should do anything else for this to be merged. Thanks!

[arjo129]

Its more or less looking good to me just handle the two changes I mentioned.

[arjo129]

Your DCO needs fixing also are you basing your work on gz-sim7 so you should target that branch else the abi checker wont be happy.

[Pedro-Roque]

Seems like you are not building tests. Please use:

colcon build --merge-install

NOT:

colcon build --cmake-args -DBUILD_TESTING=OFF --merge-install

Consider deleteing build and install folders in your workspace for clean builds.

Hi @arjo129 the command I was running is

colcon build --cmake-args -DCMAKE_BUILD_TYPE=Coverage --merge-install --packages-select gz-sim7

I'll do a clean build and come back to you.

[arjo129]

Remove the coverage do a vanilla build.

[Pedro-Roque]

Remove the coverage do a vanilla build.

Ok, the vanilla build seems to be building the tests. Maybe this is something we should update the docs with. I'll fix the test and push changes later in the week.

[Pedro-Roque]

@arjo129 @bperseghetti I've added a unit test now that checks for motion of the model upon setting a non-zero input. The test works as expected.

Let me know if there is anything else needed before merging this in.

@ahcorde @azeey just tagging to check if this is good to merge.

[arjo129]

LGTM. I think @bperseghetti, @azeey and @ahcorde should also take a look.

[arjo129]

Yeah just revert and ignore windows for now. I'm not sure about it.

…

On Thu, Jul 18, 2024, 9:03 PM Pedro Roque ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In test/integration/spacecraft.cc <#2431 (comment)>: > + serverConfig.SetSdfFile(sdfFile); + + auto server = std::make_unique<Server>(serverConfig); + EXPECT_FALSE(server->Running()); + EXPECT_FALSE(*server->Running(0)); + + using namespace std::chrono_literals; + server->SetUpdatePeriod(1ns); + return server; + } +}; + +///////////////////////////////////////////////// +// Test that commanded motor speed is applied +// See #1175 +TEST_F(SpacecraftTest, GZ_UTILS_TEST_DISABLED_ON_WIN32(InputTest)) @arjo129 <https://github.com/arjo129> test fails on Windows - seems like the vehicle did not move at all there. Thoughts? — Reply to this email directly, view it on GitHub <#2431 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAEEMQC3JXL4SF55RMVDFGLZM64LDAVCNFSM6AAAAABIXKU6BOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDCOBVG42TOMZQGQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[Pedro-Roque]

Done. Will wait for @ahcorde and @azeey for inputs

[Crola1702]

I was trying to bring up and agent and I think gz_sim-ci-pr_any-focal-amd64 failure was my fault, sorry

@osrf-jenkins retest this please

[arjo129]

Can you merge with gz-sim7 to make the ABI checker happy

[Pedro-Roque]

@arjo129 just merged it in, thanks for noting that!

Issues addressed, dismissing stale change request.

[bperseghetti]

@Pedro-Roque Thanks for your contribution, note this will not be in the binaries till a release is cut and run (including all other needed backports/forwardports that the team thinks is needed to allow for a release). I will see about the timing of that this coming Monday.

[Pedro-Roque]

Thanks @bperseghetti - how can I get that info on Monday? Reach out via Discord?

[bperseghetti]

Thanks @bperseghetti - how can I get that info on Monday? Reach out via Discord?

I'll ping you on here.

[Pedro-Roque]

@bperseghetti just pinging for further info. A release with these additions would strongly simplify our setup

[bperseghetti]

@bperseghetti just pinging for further info. A release with these additions would strongly simplify our setup

You can follow it here: #2491