Added range check for parameters ACRO_BAL_PITCH, ACRO_BAL_ROLL and PILOT_SPEED_UP

[Lqs66]

Fixed #30152.

Fixed https://github.com/ArduPilot/ardupilot/issues/30177

[rmackay9]

Hi @Lqs66,

Thanks for the PR. One small request, could you update the commit title to be prefixed with "Copter: "? E.g. "Copter: range check for ACRO_BAL_PITCH and ACRO_BAL_ROLL". If you peek at our commit history you'll see that we always prefix our commits with the subsystem or folder affected because this makes backporting easier. to be clear, I mean the commit title, not the PR title.

Txs!

[Lqs66]

Hi @Lqs66,

Thanks for the PR. One small request, could you update the commit title to be prefixed with "Copter: "? E.g. "Copter: range check for ACRO_BAL_PITCH and ACRO_BAL_ROLL". If you peek at our commit history you'll see that we always prefix our commits with the subsystem or folder affected because this makes backporting easier. to be clear, I mean the commit title, not the PR title.

Txs!

Hi, I have made the requested changes.

[peterbarker]

Closes https://github.com/ArduPilot/ardupilot/issues/30152

[rmackay9]

One of the concerns of adding these checks is the flash cost. So this particular change costs 96 bytes it seems

Binary Name      Text [B]       Data [B]     BSS (B)      Total Flash Change [B] (%)      Flash Free After PR (B)
---------------  -------------  -----------  -----------  ----------------------------  -------------------------
blimp            0 (0.0000%)    0 (0.0000%)  0 (0.0000%)  0 (0.0000%)                                      741612
antennatracker   0 (0.0000%)    0 (0.0000%)  0 (0.0000%)  0 (0.0000%)                                      761904
ardusub          0 (0.0000%)    0 (0.0000%)  0 (0.0000%)  0 (0.0000%)                                      517292
arduplane        0 (0.0000%)    0 (0.0000%)  0 (0.0000%)  0 (0.0000%)                                      342768
ardurover        0 (0.0000%)    0 (0.0000%)  0 (0.0000%)  0 (0.0000%)                                      473236
arducopter       96 (+0.0059%)  0 (0.0000%)  0 (0.0000%)  96 (+0.0059%)                                    328120
arducopter-heli  96 (+0.0059%)  0 (0.0000%)  0 (0.0000%)  96 (+0.0059%)                                    329552```

[Hwurzburg]

Changing pilot set params, even if out of range, without at least a GCS warning is a big problem as far as I am concerned...any param should be able to be set "out of range", as long as it doesn't crash code (and THAT should be protected in the code) for any number of reasons...testing. unique vehicle,etc. if this is going to be done it needs a GCS warning that its been done....more code bloat, however

[lthall]

There is a problem highlighted here but I don't like this fix.

Best I can tell we only need to add a preflight check that acro_balance and climb rate are positive.

[peterbarker]

@Lqs66 is this something you want to pursue?

If so, please remove changes related to pilot speed up/down and change to simply constrain to be >= 0.

[Lqs66]

@Lqs66 is this something you want to pursue?

If so, please remove changes related to pilot speed up/down and change to simply constrain to be >= 0.

I'm sorry for replying to you just now. I determined the parameter range according to the regulations in the ArduPilot documentation. For the ACRO_BAL_PITCH parameter, the documentation specifies an acceptable input range of 0-3 (https://ardupilot.org/copter/docs/parameters.html#acro-bal-pitch-acro-balance-pitch%EF%BC%89). For the ACRO_BAL_ROLL parameter, the documentation specifies an acceptable input range of 0-3 (https://ardupilot.org/copter/docs/parameters.html#acro-bal-roll-acro-balance-roll%EF%BC%89). Is the "change to simply constrain to be >= 0" you mentioned referring to these two parameters?

[peterbarker]

I'm sorry for replying to you just now. I determined the parameter range according to the regulations in the ArduPilot documentation. For the ACRO_BAL_PITCH parameter, the documentation specifies an acceptable input range of 0-3

Yeah, those limits are absolutely advisory. People frequently need to go outside of the recommended ranges.

Is the "change to simply constrain to be >= 0" you mentioned referring to these two parameters?

Yes - just limit so that you can't specify a negative value, which should have been sufficient to avoid the crash (assuming you're capable of uprighting the vehicle yourself :-)

[Lqs66]

I'm sorry for replying to you just now. I determined the parameter range according to the regulations in the ArduPilot documentation. For the ACRO_BAL_PITCH parameter, the documentation specifies an acceptable input range of 0-3

Yeah, those limits are absolutely advisory. People frequently need to go outside of the recommended ranges.

Is the "change to simply constrain to be >= 0" you mentioned referring to these two parameters?

Yes - just limit so that you can't specify a negative value, which should have been sufficient to avoid the crash (assuming you're capable of uprighting the vehicle yourself :-)

I have addressed the comments and modified the PR :-)

[peterbarker]

@Lqs66 thankyou for the patches here.

@lthall has noted the problems and we've merged patches from him addressing the issue.

Apologies we didn't base the fix on your work here.