Copter Fence Fixes

[andyp1per]

Separated out from #25698

• Implements Min Alt fence on copter
• Adds support for auto-enabling fence floors on copter
• Messages correctly about fence breach / enablement
• Auto-disable floor fence on landing on copter
• Always checks breaches regardless of manual recovery state - means the scripting API is up-to-date
• Prevent manual recovery logic from engaging when no fence action has been set - allows better control from scripting
• Implements FENCE_AUTOENABLE for copter
• Implements FENCE_OPTIONS for copter

Here is an attempt at describing what this is supposed to do:

1. A fence is a 2D plane in 3D space separating "inside" the fence from "outside" the fence
2. A fence can be in one of four incremental states (so e.g. a fence can only be active if it is already configured):
   • Unconfigured - the fence is unavailable for use in any way
   • Configured - the fence is able to be activated by some activation mechanism
   • Active - the fence is active and will be put into the breached state if the vehicle is "outside" the fence
   • Breached - the fence is active, the vehicle is "outside" the fence and any breach action will be taken
3. The state of each fence is fully independent of all of the others
4. A fence is configured by setting the appropriate bit in FENCE_TYPES
5. Fences come in two categories - Normal and Auto. By default Max Alt, Circle and Poly fences are considered Normal fences and Min Alt is considered an Auto fence.
6. A fence is activated by one of the following mechanisms:
   • Setting FENCE_ENABLE to 1 activates all configured normal fences
   • Setting FENCE_ENABLE to 1 activates all configured auto fences when their autoconditions become true
   • Setting an RC switch, configured with option 11 (FENCE), high activates all configured fences
   • Sending the mavlink message MAV_CMD_DO_FENCE_ENABLE actives all fences specified in the message
   • If FENCE_AUTOENABLE is set to some value causes all normal fences to become auto fences that are activated when their auto conditions become true
7. A fence is de-activated by one of the following mechanisms:
   • Setting FENCE_ENABLE to 0 de-activates all configured fences
   • Setting an RC switch, configured with option 11 (FENCE), low de-activates all configured fences
   • Sending the mavlink message MAV_CMD_DO_FENCE_ENABLE de-actives all fences specified in the message
   • An automatic landing (RTL in Copter and in Plane if certain options are set, LAND, land in AUTO) will disable all auto fences and ignore all breach actions
8. The following auto-conditions are available:
   • Ascending above Min Alt by default
   • On arming via FENCE_AUTOENABLE
   • On auto takeoff via FENCE_AUTOENABLE
9. The execution of a breach action prevents the execution of any further breach action until the breach action is completed or pilot changes mode or disarms

Requires ArduPilot/mavlink#349

[tridge]

I'd like @Hwurzburg to have a look at the impact on plane

[Hwurzburg]

I think a few changes are needed: disabling floor on disarm for all autoenable values if the option is set...the impact of long continued breaching flight in plane must be either accepted by @tridge or changed.
This does fix some Plane bugs #1 and #2 and #4 if the change to when disable_floor is done:

1.AutoEnable on arming, or not using AutoEnabled, never disables a floor for landing. So all the Plane situations which think they disable floor, wont.
2.MIN_ALT floor always disabled on boot irrespective FENCE_ENABLE, never gets enabled unless: autoenabled, or overt enable(GCS or switch).
3.MIN_ALT fence records breach even if floor is not enabled. MIN_ALT breach is recorded but not cleared after landing, preventing mode changes if mode change prevention is enabled (Copter allows mode changes always). This prevents mode changes after landing.
4.MIN_ALT floor stays disabled after disable floor for landing, used by NAV_LAND, QLAND and other VTOL landings in Plane, LAND, Copter RTL
5.Plane automatically ignores ANY breach while performing a breach action. Again prevents mode changes in Plane while landed.

[andyp1per]

Just to try and capture what @rmackay9 would like - he would prefer that we auto-disable the floor fence as we go through it on a land/RTL action rather than leaving the fence enabled and ignoring it

[andyp1per]

Trying to write down some rules here, as this stuff is convoluted ...

• Fence presence (FENCE_TYPE) means a fence is eligible to be enabled, but nothing more
• Fence enablement (FENCE_ENABLE) means a fence is active and will take action if breached. We currently have no way of selectively enabling but I am beginning to think we should.
• Fence breach (get_breaches()) means a present and enabled fence has been breached. Breaches should always be correct, if you want to suspend a fence (e.g. to land) then it should be disabled.
• Fence auto-enablement (FENCE_AUTOENABLE) allows present fences to be automatically enabled and disabled under certain conditions - on arming, on takeoff, on landing. It's not clear why auto-enablement of anything other than floor fences makes sense. Auto-enablement also suffers from memory loss - if a fence is auto-enabled and then directly enabled via mavlink (as we do in the tests), what happens when you land - should the (non-floor) fence be disabled or enabled?
• Certain modes (RTL descent, Land, Auto) will automatically disable fences for landing so that they can land with a floor fence. It's not clear why this should be anything other than the floor fence.
• Fences can be enabled and disabled via mavlink. Initially this was all fences, but more recent changes allow the floor fence to be switched off only. I am planning to add an option that does the opposite - allows all fences to be enabled except the floor.

The current behaviour is confusing and inconsistent because sometimes all fences are automatically disabled to take an action when only the floor fence needs to be disabled, similarly current behaviour ignores breached floor fences in certain (landing) circumstances. In order to get consistency I think we need individual control over presence (OK), enablement (MISSING) and breaches (OK). This means turning the enabled flag into a bitmask.

[Hwurzburg]

"Fence enablement (FENCE_ENABLE) means a fence is active and will take action if breached. We currently have no way of selectively enabling but I am beginning to think we should." is untrue
....if _floor_enable is false the ALT_MIN fence is disabled...currently starts in this state after boot even if FENCE_ENABLE is true at boot...takes a direct action to enable it or AUTOENABLE condition get floor enabled...

Personally, I think:

• We should have the fence enable/disable AUX function have a bit mask variable for what it controls. The same for MAVLink fence command.
• We should have auto enable and disable functions of ALT_MIN that can be called from takeoff and landing code. I am not sure why you would ever NOT want to disable the ALT_MIN fence on an auto landing...its a pilot control protection I think
• We should have a fence option to disable horizontal fences during breach actions (RTLs) Plane just ignores breaches during breach recovery actions, but doesn't clear the breach...we should prevent a breach by allowing a temporary disable with auto-re-enable if breach action abort or when changing mode to re-takeoff
• I can see the need for selectively auto-enabling non ALT_MIN fences on arming (to move out of an exclusion zone like pit area without breaching), but why its needed otherwise I am not sure. If we just enable the ALT_MIN once its reached, disable on auto landings, and re-enable once another climb above it occurs. Maybe we can deprecate the FENCE_AUTOENABLE and replace with a FENCE_OPTION for autoenabling non-ALT_MIN fences on arming. Not sure why you need the enable on takoff if you have the enable on arming. And making ALT_MIN autoenable removes the need for the normal enable on takoff.

[andyp1per]

"Fence enablement (FENCE_ENABLE) means a fence is active and will take action if breached. We currently have no way of selectively enabling but I am beginning to think we should." is untrue ....if _floor_enable is false the ALT_MIN fence is disabled...currently starts in this state after boot even if FENCE_ENABLE is true at boot...takes a direct action to enable it or AUTOENABLE condition get floor enabled...

_floor_enabled is not a parameter, so there is some indirect stuff going on, but nothing that allows you to directly enable or disable individual fences

Personally, I think:

* We should have the  fence enable/disable AUX function have a bit mask variable for what it controls. The same for MAVLink fence command.

Yes, 100% agree

* We should have auto enable and disable functions of ALT_MIN that can be called from takeoff and landing code. I am not sure why you would ever NOT want to disable the ALT_MIN fence on an auto landing...its a pilot control protection I think

Yes agree, auto enable of anything else seems a bit suspect to me

* We should have a fence option to disable horizontal fences during breach actions (RTLs) Plane just ignores breaches during breach recovery actions, but doesn't clear the breach...we should prevent a breach by allowing a temporary disable with auto-re-enable if breach action abort  or when changing mode to re-takeoff

Yes, so this is not possible without finer grained control of enablement - bu I agree this is a good way forward

* I can see the need for selectively auto-enabling non ALT_MIN fences on arming (to move out of an exclusion zone like pit area without breaching), but why its needed otherwise I am not sure. If we just enable the ALT_MIN once its reached, disable on auto landings, and re-enable once another climb above it occurs. Maybe we can deprecate the FENCE_AUTOENABLE and replace with a FENCE_OPTION for autoenabling non-ALT_MIN fences on arming.  Not sure why you need the enable on takoff if you have the enable on arming. And making ALT_MIN autoenable removes the need for the normal enable on takoff.

Agree about simplifying, still not quite sure what the correct mix is

[andyp1per]

I have turned FENCE_ENABLE into a bitmask and it makes everything much easier.

[Hwurzburg]

I think this is good, not recording an ALTMIN breach if not enabled....and having a configured ALT_MIN fence always autoenable (if not enabled by FENCE_ENABLE but configured in FENCE_TYPE) upon reaching the alt, and reenabling after a landing module disables it for landing if below the alt and a mode change occurs.....but we should also deprecate AutoEnable and convert "3" to setting a new FENCE_OPTIONS bit to enable all non MIN_ALT configured fences on arming, which would not be the default for the option bit
This will fix many problems in Plane and still function as desired (but better really)

[Hwurzburg]

here is another nasty one....Tridges patch fixes AUTOENABLE =2 operation, but AUTOENABLE =1 is broken still....first flight it will enable the poly on takeoff and breach and take breach action on the poly...but if you refly by restarting the mission, it will say that the poly fence has been enabled after takeoff, but not breach when it hits the poly....log :https://www.dropbox.com/scl/fi/1hdmsykaefv8yrhir92t9/00000003.BIN?rlkey=ajbgixqzzubbrby0wy5omvo9c&dl=0

[tridge]

@andyp1per @Hwurzburg this fixes the lastest issue that Henry found:
andyp1per#33

[tridge]

I think we should mark FENCE_AUTOENABLE=1 and FENCE_AUTOENABLE=2 as deprecated, not actually remove them yet, but issue a deprecation warning, and recommend not using them in the wiki, instead pointing users at either FENCE_ENABLE=1 or FENCE_AUTOENABLE=3

[andyp1per]

I think we should mark FENCE_AUTOENABLE=1 and FENCE_AUTOENABLE=2 as deprecated, not actually remove them yet, but issue a deprecation warning, and recommend not using them in the wiki, instead pointing users at either FENCE_ENABLE=1 or FENCE_AUTOENABLE=3

I think these are all up for grabs, but would like to have that conversation after the PR goes in!

[Hwurzburg]

another issue...if you have setup a fence sw, you cant autoenable any more and that includes min_alt....if you set it high on the ground you breach min_alt...if its low, then autoenable (at least 3) does not enable any fences....I would think at least on boot, the fence switch would allow autoenable if low....ie like fence_enable = 0.
But i guess we could say that autoenable does not work if you have setup a fence sw.....I dont like it myself....I think that maybe we should change the fence sw init to do nothing and need a switch transition, or make the MIN_ALT work the same as in Autoenable...ie have to be above before enabling....anyway its very awkward now....

[timtuxworth]

another issue...if you have setup a fence sw, you cant autoenable any more and that includes min_alt....if you set it high on the ground you breach min_alt...if its low, then autoenable (at least 3) does not enable any fences....I would think at least on boot, the fence switch would allow autoenable if low....ie like fence_enable = 0. But i guess we could say that autoenable does not work if you have setup a fence sw.....I dont like it myself....I think that maybe we should change the fence sw init to do nothing and need a switch transition, or make the MIN_ALT work the same as in Autoenable...ie have to be above before enabling....anyway its very awkward now....

Yes - the switch should be "turn on fence function/turn off fence function" - when on, it should mean that fence functionality is on, (not that all fences turn on right away) - so autoenable will function if the switch is on, when off all fences are off and autoenable will not function.

[andyp1per]

I was trying to be very conservative. It’s very easy for mavlink not to work. Also switch behaviour needs to be idempotent which makes it largely incompatible with auto enable.Andy Sent from my iPhoneOn 21 Jul 2024, at 18:46, Henry Wurzburg ***@***.***> wrote: another issue...if you have setup a fence sw, you cant autoenable any more and that includes min_alt....if you set it high on the ground you breach min_alt...if its low, then autoenable (at least 3) does not enable any fences....I would think at least on boot, the fence switch would allow autoenable if low....ie like fence_enable = 0. But i guess we could say that autoenable does not work if you have setup a fence sw.....I dont like it myself....I think that maybe we should change the fence sw init to do nothing and need a switch transition, or make the MIN_ALT work the same as in Autoenable...ie have to be above before enabling....anyway its very awkward now.... —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>

[timtuxworth]

Also switch behaviour needs to be idempotent which makes it largely incompatible with auto enable.

Does it make sense If you think of the switch as "enable/disable Fence capability" rather than "enable/disable the actual fence"?

[andyp1per]

Also switch behaviour needs to be idempotent which makes it largely incompatible with auto enable.

Does it make sense If you think of the switch as "enable/disable Fence capability" rather than "enable/disable the actual fence"?

I can't change the current meaning of what happens on a switch, the behaviour should be the same

[Hwurzburg]

1. With a couple of exceptions, ALT_MIN fence, if configured, will become enabled once MIN_ALT is obtained. It is disabled on NAV_LAND. Adding a margin seems like a good idea...maybe +2m
   -AUTOENABLE =1 enables it at takeoff alt completion (I think this should change for consistency)
   -AUX switch high for fence instantly enables all fences including MIN_ALT (I think this should change for consistency)
2. FENCE_ENABLE=1 enables all fences except MIN_ALT until its reaches MIN_ALT. Dynamically changing FENCE_ENABLE to 0 disables all fences immediately. FENCE_AUTOENABLEs need FENCE_ENABLE= 0 to work.
3. Fence AUX switch change enable or disables, omnipotently. Sw value is scanned at boot (init'd). This results in MIN_ALT breach on ground preventing boot. I think this should change, as stated above in 1.
4. I agree with deprecating AUTOENABLE to only be off or on ARM only.

Stale

[tridge]

@andyp1per this PR makes the change to your PR as requested by @IamPete1
andyp1per#34
if we merge that then I believe we will then be able to merge this PR

[andyp1per]

@andyp1per this PR makes the change to your PR as requested by @IamPete1 andyp1per#34 if we merge that then I believe we will then be able to merge this PR

Merged thanks @tridge