SITL: Fix the --param (aka -P) cli option

[hunt0r]

This addresses issue #6114 .

Solution summary: split cli handling into a "parse" phase which verifies and saves the info, and a "set" phase which is called after the params exist with default values to set them to the -P-provided values.

Test summary: Reviewers please request other specific tests in case the original coverage is not sufficient.

Also, do we have any suitable automatic testing infra for cli-parsing like this?
Per reviewer: no.

Design review

While the solution is straightforward, and the testing examples are helpful, reviewers should start with these design-level questions:

1. Is a compile-constant max on the number of cli params a desired design? If yes, what value is desired? (Currently at 4, which made testing simpler, trivial to increase it to any desired value.) Reviewer: Use 20.
2. Should this option be supported for examples when AP::sitl() is nullptr? (If yes, I think that's an easy fix.)
3. Consider alternative interfaces: -P NAME1=VAL1,NAME2=VAL2 vs -P NAME1=VAL1 -P NAME2=VAL2. Do we want to permit only one, or both? If both, do we want the usage to describe both? (Right now both are permitted, usage only describes one.)
4. How do we want -P to interact with other possibilities for setting parameter values? (e.g. --wipe, --defaults --model) The same question holds for the values which get auto-saved to a file and auto-loaded next time. (I propose we postpone this discussion, land the PR, and just let the interaction(s) emerge. With clean/clear code, adjusting them in the future is not hard.)
5. Due to basics of CLI parsing, forgetting to provide a NAME=VALUE means the next option gets consumed as that. (e.g. -P --model=foo thinks the param name is "--model", which fortunately fails to be found.) Do we need specific detection/prevention of this? (I propose that we don't, because it will fail naturally.)
6. sim_vehicle.py has a -P, but it does not use this (now fixed) capability. Should it? (I propose we postpone this discussion to unblock the PR. We can make that a 'next step' in the issue, if desired.)
7. Do we want to continue avoiding nice std::string in SITL? (OFC this PR does, if for no stronger reason than preserving stylistic consistency.) Reviewer: yes.

Testing

All testing performed under valgrind, no leaks.

Happy-path test summary:

• For single param -P MNT1_TYPE=8, confirmed expected value via param show MNT1_TYPE in MAVLink.
• For multiple params -P MNT1_TYPE=8,MNT1_ROLL_MIN=-88.8,MNT1_ROLL_MAX=99, confirmed expected values via param show MNT1* in MAVLink.
• Empty string -P "" continues with no parameters changed. (Same as omitting -P ... altogether.)

Error-catching test summary:

• Single-space string -P " " errors with Specify param+val via: NAME=VALUE (No '=' found in )
• Forgotten name/val -P --model + errors with Specify param+val via: NAME=VALUE (No '=' found in --model)
• Invalid format -P MNT1_TYPE:8 errors with Specify param+val via: NAME=VALUE (No '=' found in MNT1_TYPE:8)
• Invalid format -P MNT1_TYPE==8 errors with Unable to convert to float: =8
• Misspelling -P MXT1_TYPE=8 errors with Unknown parameter: MXT1_TYPE
• Invalid value -P MNT1_TYPE=abcd errors with Unable to convert to float: abcd
• Invalid value -P MNT1_TYPE=8abcd errors with Unable to convert to float: 8abcd (Relevant because behavior for this case changed during review process.)

For testing purposes, I lowered the MAX from 20 to 4, recompiled, and did this additional test:

• Too many values -P MNT1_TYPE=8,MNT1_ROLL_MIN=-88.8,MNT1_ROLL_MAX=99,MNT1_PITCH_MIN=-44,MNT1_PITCH_MAX=44 errors with -P only supports 4 values. To proceed, recompile with increased maximum.

Corner case summary:

• Extra commas don't cause problem: -P MNT1_TYPE=8,,,,,,,,MNT1_ROLL_MIN=-88.8, works to set those 2 params.
• Max-length param works: -P FLTMODE_GCSBLOCK=19 works to set that 16-char param.

Testing details: I used --wipe infront of every -P to ensure its behavior was being observed, not previously saved values. From the ardupilot/ArduCopter folder, my commands were like

/Users/hunt0r/ardupilot/build/sitl/bin/arducopter --wipe -P [...] --model + --slave 0 --sim-address=127.0.0.1 -I0

and the MAVProxy command I used to verify was

~/MAVProxy/MAVProxy/mavproxy.py --console --out 127.0.0.1:14550 --master tcp:127.0.0.1:5760 --sitl 127.0.0.1:5501.

The valgrind specifics were:
valgrind --leak-check=full --show-leak-kinds=all --track-origins=yes --num-callers=20 [...].
I saw 0 direct, indirect, and possibly. (Nonzero 'still-reachable', but that seems typical.)

[peterbarker]

There's no testing infrastructure or this.

Test with a maximum-name parameter

Fail if there are any characters in the value we don't understand (which is checking the other bit out of strtof, I think?)

Not a problem to have 20 parameters settable with this mechanism.

Continue to avoid std::string

Do your testing while also running with --valgrind, just in case.

Thanks for working on this!

[peterbarker]

This is a wildly different call than exit.

Why are you not being consistent with other errors in here?

[hunt0r]

I was following (or attempting to follow) what I see in SITL_State::_parse_command_line().

• panic() used when out of memory
• exit() used for the cases where an option is invalid

But, as is probably obvious, I don't understand when/why to use panic() in SITL.

[hunt0r]

I changed it to exit: 94c2b9e

But of course if consistency with _parse_command_line() is better, I can easily drop my recent commit, just say the word.

[hunt0r]

Test with a maximum-name parameter

Added to description: I used FLTMODE_GCSBLOCK which is AP_MAX_NAME_SIZE==16 chars.

Fail if there are any characters in the value we don't understand (which is checking the other bit out of strtof, I think?)

Changed: efde652.

Not a problem to have 20 parameters settable with this mechanism.

Changed: 96ff06f.

Do your testing while also running with --valgrind, just in case.

Done, and added to PR description.

[peterbarker]

This isn't working for a lot of parameters; the current spot where things are being set is before the simulation has set the paramete rvalue from the default parameters.

e.g. I was using WP_RADIUS on Plane and it didn't work.

Please create an autotest which uses customise_SITL_cmdline to test this.

Need to work out whether this should set the parameter value - or the default parameter value.

[hunt0r]

This isn't working for a lot of parameters; the current spot where things are being set is before the simulation has set the paramete rvalue from the default parameters.

e.g. I was using WP_RADIUS on Plane and it didn't work.

I agree (have repro'ed). Thanks for catching!

Please create an autotest which uses customise_SITL_cmdline to test this.

Will do.

Need to work out whether this should set the parameter value - or the default parameter value.

Should we generalize that to include the desired interactions of -P with other possibilities for setting parameter values (e.g. --wipe, --defaults, --model)? With clarity on that, I can author (and test to verify) an implementation which meets our needs.

[hunt0r]

Per dev call 2026-02-23, we are considering instead simply deleting this --param feature.

[peterbarker]

Please remove the support as discussed at DevCall

[hunt0r]

Change of direction: will delete this feature instead.