Move vehicles to normalized thruster interface in gz-maritime: SITL Phase 2. - #53
Open
bsb808 wants to merge 3 commits into
Open
Move vehicles to normalized thruster interface in gz-maritime: SITL Phase 2.#53bsb808 wants to merge 3 commits into
bsb808 wants to merge 3 commits into
Conversation
bsb808
changed the title
bsb808
marked this pull request as ready for review
September 1, 2026 17:56
bsb808
force-pushed
the
bsb/sitl_boat_phase1
branch
from
September 1, 2026 20:17
62588ad to
c396587
Compare
bsb808
force-pushed
the
bsb/sitl_boat_phase2
branch
2 times, most recently
from
September 1, 2026 20:18
5c00cb2 to
ff5d268
Compare
Phase 1 of the SITL roadmap: the boat driven by the same autopilot it runs on the water, using the thruster interface exactly as it exists today. No plugin work; this proves the plumbing and the channel mapping. The ArduPilot variant is generated from model.sdf.xacro behind a flag that defaults to false, so the model generated today is byte-identical and the ROS 2 path is untouched. The roadmap proposed a hand-written wrapper merge-including model://blueboat instead, which cannot work: merge-include does not nest and the composed model already merge-includes its URDF, so the frame name collides. SITL_DESIGN.md is amended rather than quietly rewritten, so intent and implementation can be reviewed together. The IMU is a frame on the chassis, not a part and not a Gazebo-only link. Parts already declare frames for where they sense, so one line gives a base_link_imu link in the URDF and therefore in TF, with the composition hanging the sensor on it as it already does for the Ping's echosounder. Parameters derive from Blue Robotics' hardware dump, not from ArduPilot's rover-skid defaults and not from the community SITL_Models config, both of which assign the throttles the other way round. Four deltas are marked in the file: both trims to 1500, and SERVO3_REVERSED to 0, which compensates for wiring that does not exist in simulation and would otherwise drive the port thruster backwards. The world loads gz-sim-imu-system. Without it the IMU sensor produces nothing, ArduPilotPlugin has no attitude to report and never replies, and SITL stalls resending servos. The existing worlds never needed it because the boat had no IMU until now. Verified headless: zero duplicate frames, MAVProxy detects the vehicle and downloads 1275 parameters. The drive test itself needs an interactive ground station and has not been run.
Completes phase 2 on the vehicle side. The thruster now takes a command in [-1, 1] through gz_thruster's normalized mode, so the newtons live in one place - the model, where max_thrust_cmd and min_thrust_cmd already declared them - rather than being duplicated into everything that commands a thruster. The ArduPilot binding is the point. Its <control> block reduces to servo_min 1100, servo_max 1900, offset -0.5, multiplier 2: the servo band converted to a normalized command and nothing else. No thrust limits, no vehicle physics, and in particular no second copy of the T200's envelope to drift from the model's. The topic is renamed from <topic_base>/thrust to <topic_base>/cmd, because the units changed and the old name would be a lie. That follows through the generated bridge config, which comes from the same resolution. The teleop mixer moves with it. Its max_thrust_forward / max_thrust_reverse become 1.0 and -1.0 - not a behavior change but a statement that the envelope is no longer the mixer's business. The node is untouched, and the BlueROV2 keeps the stock plugin and its newtons interface, so the config test now accepts both namings and says why. blueboat_gazebo gains an exec_depend on gz_thruster: not a build dependency, since gz-sim loads the plugin from GZ_SIM_SYSTEM_PLUGIN_PATH by filename, but it must be built and on that path before the boat will run. Verified: the world loads on /cmd in normalized mode, the teleop config tests pass, and the ArduPilot loop still closes - zero duplicate frames, IMU found, 1275 parameters downloaded.
Same change as the BlueBoat, for the same reason. Its thruster macro was already structurally identical - explicit topic, empty namespace, limits from the part's drive table - so this is the same six edits. Its mixer gains were already unit vectors, so max_thrust_forward and max_thrust_reverse becoming 1.0 and -1.0 is not a behavior change either. The T200 envelope now lives only in the model, for both vehicles. With both moved, the config test goes back to a single naming rather than accepting either: every mixer output is a normalized command on /cmd. Verified: the ROV loads on the forked plugin, and normalized 0.5 on thruster_1 produces 194.03697775299023 rad/s - the same number the BlueBoat gives, which it should, since both fit T200s with the same 51.5 N limit.
bsb808
force-pushed
the
bsb/sitl_boat_phase1
branch
from
September 1, 2026 20:34
c396587 to
d2e4809
Compare
bsb808
force-pushed
the
bsb/sitl_boat_phase2
branch
from
September 1, 2026 20:34
ff5d268 to
60384f0
Compare
Contributor
|
There are some conflicts and CI isn't happy. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary: Human authored
Order:
lyricalafter Drive the BlueBoat with ArduRover in SITL (phase 1) #44 is merged)This is the vehicle half of the SITL work. See SITL_DESIGN.md for the entire plan. Modifies both ROV and Boat.
gz-maritime-thruster-systemand asks for a normalized command, so they are driven with a fraction of full thrust rather than a force in Newtons.<topic_base>/thrustto<topic_base>/cmd<control>block is now just the servo band converted to a normalized command.blueboat_gazeboandbluerov2_gazebogain a runtime dependency ongz_thruster, the first dependency from this repo on gz-maritime. This is temporarly until we upstream the plugin modifications.How to test
All the instructions are in gz-maritime#18
The short version is that behavior should be indistinguishable from #44: run the teleop and SITL examples and nothing should have changed.
The rest of this is the AI remnants from my local build. I'm leaving this here because (1) it is a useful, if overly verbose, explanation of what was done and (2) may be useful context for reviewer's agents.
What this buys
The ArduPilot
<control>block, which is the point of the whole exercise:The servo band converted to a normalized command, and nothing else. Before this it carried
multiplier 80.4— a thrust limit, copied out of the model into the autopilot binding, in a different file with different provenance and nothing to keep the two in step. That is the class of duplication that produces a vehicle which still runs and quietly does the wrong thing.Now the newtons live in exactly one place:
max_thrust_cmdandmin_thrust_cmdon the thruster, where they were already declared for clamping.What changed
The model.
blueboat_gazebo/model.sdf.xacronamesgz-maritime-thruster-systemwith<use_normalized_cmd>true</use_normalized_cmd>.The topic.
${topic_base}/thrustbecomes${topic_base}/cmd, because the units changed and the old name would be a lie. That follows through the generatedros_gz_bridge.yaml, which derives from the same resolution rather than being maintained separately.The teleop mixer.
max_thrust_forwardandmax_thrust_reversebecome 1.0 and -1.0. Not a behavior change — the gains were already unit vectors — but a statement that the envelope is no longer the mixer's business. The node is untouched.Both vehicles move together. The BlueROV2's thruster macro was already structurally identical to the BlueBoat's, so it is the same change; with both on
/cmd,test_mixer_topics_exist_in_the_bridgegoes back to a single naming rather than accepting either. The T200 envelope now lives only in the models.A cross-repo dependency.
blueboat_gazebogains<exec_depend>gz_thruster</exec_depend>. Not a build dependency — gz-sim loads the plugin by filename fromGZ_SIM_SYSTEM_PLUGIN_PATH— but the package has to exist and be on that path before the boat will run. This is the first dependency frombluerobotics_modelsongz-maritime, which release tooling will need to know about.Verified
The world loads on
/cmdin normalized mode, the teleop config tests pass, and the ArduPilot loop closes with zero duplicate frames, the IMU found, and 1275 parameters downloaded.Not verified: the drive test, same as #44. It needs an interactive ground station, and the exit criterion above is a prediction from the parameters rather than an observation.