GetPotentialTorque can receive negative values, breaks cooked steering

[Majromax]

As previously mentioned on Reddit, with FAR integration (v0.15.6.5) aerodynamic control surfaces can apparently return negative values to GetPotentialTorque. This causes the cooked steering manager to calculate negative raw torque values, which threshold to the don't-divide-by-zero epsilon value.

Example of this problem on a sounding rocket, from CSV steering output ("lock steering to up."):

Versus "correct" behaviour with FAR disabled:

(Although bear in mind that the stock control surfaces apparently return extremely erratic values. This is bad for cooked steering stability, but in a more benign and definitely-not-kOS-bug way.)

This may be a bug with FAR's specification of those values, but MechJeb uses the absolute value of returned torques for its own calculations.

[Dunbaratu]

Regardless of whether or not it's wrong for GetPotentialTorque() to return a negative number, shouldn't our epsilon check be checking for epsilons near zero rather than just hardcoding to less than some X. (meaning it should be an absolute value magnitude check, or a check that says it has to be < X AND > -X to count as being "too small").

[Majromax]

That was discussed in the Reddit thread. Negative raw torque would ordinarily tell the steering manager (or user) that control response is inverted -- call for pitch down to actually pitch up -- but hvacengi pointed out that properly supporting that odd behaviour would require a deeper rewrite of cooked steering.

The EPSILON value in the codebase exists not to preserve meaningful steering behaviour, but to prevent divide-by-zero exceptions. It would probably be better (and more obvious to the user) if thresholded steeing values just disabled steering control along that axis, but that might be too complicated to be worth it.

Aside from this case, which is probably a FAR bug because it returns the opposite sign compared to stock, few if any ship designs with functional steering should ever see negative or zero raw torques.

[hvacengi]

Based on comments by @Ferram4 and @NathanKell it appears GetPotentialTorque is indeed intended to return positive values. The question at this point is whether or not we should internally force that check as well. If the steering manager performs it's own absolute value, then the epsilon check as written will be fine. Really, the epsilon check is intended to prevent issues with division by zero and to give a fall back for control in case we don't find any parts implementing ITorqueProvider.

I presume that FAR will just be returning the absolute value of what it currently returns, but I haven't looked at it's implementation enough to know if it is returning the static torque plus the additional actuated torque, or just the additional actuated torque of the control surfaces. It's been a while since I look and discussed with Nathan, but I think that the stock control surfaces do not subtract their static un-actuated torque from the value returned.

[hvacengi]

It would probably be better (and more obvious to the user) if thresholded steeing values just disabled steering control along that axis, but that might be too complicated to be worth it.

The bigger issue I see is that this could lead to ships that don't even try to turn, making it look like the steering code isn't even trying, while oscillating control are already identified as tuning issues for either the control loop or the amount of torque on the vessel. If we at least try to turn, the user may try to add a reaction wheel or RCS port before reporting it as a bug.

[Majromax]

I presume that FAR will just be returning the absolute value of what it currently returns, but I haven't looked at it's implementation enough to know if it is returning the static torque plus the additional actuated torque, or just the additional actuated torque of the control surfaces

See FAR's calculation for PotentialTorque. I don't think it's even necessarily a problem with needing an absolute value, it feels to me like the calculation has the wrong orientation. I don't see any static torque being applied or subtracted, just the maximum amount for the control surface apportioned between pitch/yaw/roll.

The bigger issue I see is that this could lead to ships that don't even try to turn, making it look like the steering code isn't even trying, while oscillating control are already identified as tuning issues for either the control loop or the amount of torque on the vessel.

My aesthetic preferences aside, this is probably a moot point. Once this bug is fixed (workaround in kOS or properly in FAR), users should never ordinarily see zero torque calculated if the craft in fact can be controlled along that axis.

[hvacengi]

@Majromax I don't think FAR is updated officially for KSP 1.2.0 yet, but I actually ended up having to add the absolute value fix to the new implementation because stock was spitting out negative values, and I had to incorporate some averaging (because a part may cause asymmetric torque, and that's an easy way to account for it). Once you're able to, could you test with the latest FAR to make sure we seem to be working better?