common-ice: expand and update documentation

[robertlong13]

When discussing ArduPilot/ardupilot#30201 on the dev call, Tridge requested that I overhaul the whole ICE wiki page, and go into more detail on disarmed operation.

I've been through the source in detail and tried to make sure every nuance is captured here.

In the past I really struggled to get the ICE library to play nicely with handheld starters when you also have an RPM sensor. The approach I added in here has worked well for me for such systems.

[Hwurzburg]

havent reviewed fully but all parameters that can be, should be properly linked, ie
instead of ICE_START_PCT it should be :ref:`ICE_START_PCT<ICE_START_PCT>`

[robertlong13]

havent reviewed fully but all parameters that can be, should be properly linked, ie instead of ICE_START_PCT it should be :ref:ICE_START_PCT<ICE_START_PCT>

Fixed. Most of the non-linked cases were existing. Agree that consistency is best.

[robertlong13]

Ugh, I just found a mistake in part of my explanation. By default, the engine cannot run disarmed unless you set ICE_IDLE_PCT to 1 or higher (I was doing that a lot in my testing while drafting), or if you set the disarmed manual throttle option (but that one is more obvious). I went through the line history of that to try to get a sense of the reasoning, but it's a bit baffling.

https://github.com/ArduPilot/ardupilot/blob/86e22fd5705d78e7329de707468fac25feb221ee/libraries/AP_ICEngine/AP_ICEngine.cpp#L454

I'll retool that explanation. But it's SO complicated. The engine will not run while disarmed, unless you set an idle throttle or "disarmed manual throttle option-bit", unless you set the "don't start disarmed option-bit", unless you set param4 of a DO_ENGINE_COMMAND to 1, (but that param4 override only overrides that "don't start disarmed option-bit"; if you don't have an idle percent or the "disarmed manual throttle option-bit" than that override doesn't do anything).

There are 4 stacked layers with alternating "unless"... 😩

[robertlong13]

Some of the history there
ArduPilot/ardupilot#11109
ArduPilot/ardupilot#25053
ArduPilot/ardupilot#25138

I don't think the idle percentage parameter should ever have been tied to the behavior of whether to run while disarmed, but it's been in there long enough that I doubt I can fix that without breaking current users' setups.

[robertlong13]

I've corrected it now. Hopefully in a way that isn't terribly confusing.

[Hwurzburg]

Many ICE params are wrong names. here are the ICE param

[robertlong13]

Many ICE params are wrong names

Can you point to one? I only found a single wrong name, and one pre-existing stale name in the Options table. I've now fixed those two.

If you're referring to the params on lines 99-102 and line 126, those are in the ArduPilot prior to 4.6 tabs, so of course they aren't the right names for 4.6+, but that's expected.

[Hwurzburg]

@robertlong13 but those added invalid param links now cause build errors....any invalid param should just be highlighted via backticks and not linked...ie ICE_PWM_STRT_ON

[robertlong13]

Gotcha. I take it we don't have any way to pin them to the 4.5.5 param wiki page?

[Hwurzburg]

no....that page is not regenerated anyway, its an archive...only master and new releases gets regenerated as I understand it....could be changed I guess

poke Tridge for an approval...he mostly ignores pings via email

[Hwurzburg]

@robertlong13 did you incorporate #7044? and if you did I will merge...no point in waiting on Tridge longer, I guess...

[robertlong13]

@Hwurzburg I have rebased to include Tridge's #7044. I have also prefixed my note about the idle governor's quirks with "Prior to ArduPilot 4.7", which covers ArduPilot/ardupilot#30201 (my outstanding TODO in #6390). This should be good to go.

[Hwurzburg]

made a few tweaks...rather than waiting on the 4.7 minor changes to merge, merging now since this a big improvement on ICE docs