PX4-Autopilot icon indicating copy to clipboard operation
PX4-Autopilot copied to clipboard

Add secondary geofence at an outer offset distance from the primary geofence.

Open mcsauder opened this issue 3 years ago • 5 comments
trafficstars

Describe problem solved by this pull request

For operations that require a hard terminate on Geofence violation, the predictive behavior helps avoid flight termination that can be avoided, however this alone does not allow an initial behavior such as hold or land to be followed later by a hard flight termination on Geofence breach. In the event of a poor position estimate and/or attitude estimate due to vibrations the drone might still be able to be held within the Geofence while ending the flight in a semi-controlled descent.

Describe your solution

This PR creates a secondary Geofence with associated behavior via an offset distance parameter that allow the drone to enter another mode prior to any flight termination commands if the primary Geofence is actually violated, or vice versa.

Describe possible alternatives

Ultimately it would be nice to migrate geofence vertex lookups into volatile memory, however this implementation mirrors the current architecture.

Test data / coverage

Additional flight testing to be accomplished

Additional context

Replaces #19112, potentially impacts #18821. Predictive Geofence action could be deprecated in lieu of this functionality. @RomanBapst @NAmmann

mcsauder avatar Aug 15 '22 17:08 mcsauder

Hi @xinnad . I finally got back to collapsing the buffer zone checks to remove the duplicate dm reads and have no clue why I didn't see things this way before. I'd recommend re-working #20065 to use what's here, I think you'll find it very straightforward. If you have any chance to re-run your initial tests that would be awesome.

mcsauder avatar Sep 01 '22 01:09 mcsauder

Rebased into a single commit.

mcsauder avatar Sep 01 '22 01:09 mcsauder

@mcsauder

  1. Does the buffer apply to mission geofence, failsafe geofence, or both?
  2. FMI does the failsafe action in https://docs.px4.io/main/en/config/safety.html#geofence-failsafe apply to both mission and failsafe geofences.
  3. Does this setting need to be in QGC too? i.e. is the setup screen as shown in https://docs.px4.io/main/en/config/safety.html#geofence-failsafe properly reflect the failsafe settings that users should readily set?

Have you had a think about possible doc requirements of this update?

Looking at existing docs:

  • Flying > Geofence
    • Pretty out of date, needs an update to talk about preemtive geofence settings and also this buffer when it goes in.
    • Link to http://docs.px4.io/main/en/advanced_config/parameter_reference.html#geofence for "other features".
    • Fixes for more recent QGC UI.
  • Safety > Geofence
    • Update on settings
    • Mention of buffer and preemtive geofence
    • Confirm what settings apply to the plan (mission) vs default geonfence.

Thoughts?

hamishwillee avatar Sep 01 '22 01:09 hamishwillee

PS From a GCS point of view I might like to see the buffer zone. Worth thinking about how this might be exposed via mavlink?

hamishwillee avatar Sep 01 '22 01:09 hamishwillee

Thanks @hamishwillee !

Does the buffer apply to mission geofence, failsafe geofence, or both? FMI does the failsafe action in https://docs.px4.io/main/en/config/safety.html#geofence-failsafe apply to both mission and failsafe geofences.

Yes, the buffer applies to both if a geofence is active. If the buffer distance is set to zero then they are one and the same.The Navigator class checks for fence violations in both cases and the Commander class enforces the behaviors.

Does this setting need to be in QGC too? i.e. is the setup screen as shown in https://docs.px4.io/main/en/config/safety.html#geofence-failsafe properly reflect the failsafe settings that users should readily set?

Yeah, that's a good point, thanks for pointing that out.

Also, in answering your points I just came across a problem with buffer code for exclusion circles where the buffer needs to be reversed. I'm glad you asked about these things, I'll get on fixing that corner case immediately. image

Have you had a think about possible doc requirements of this update? Looking at existing docs: Flying > Geofence Pretty out of date, needs an update to talk about preemtive geofence settings and also this buffer when it goes in. Link to http://docs.px4.io/main/en/advanced_config/parameter_reference.html#geofence for "other features". Fixes for more recent QGC UI. Safety > Geofence Update on settings Mention of buffer and preemtive geofence Confirm what settings apply to the plan (mission) vs default geonfence.

Yes, it would be a great time for us to work on revising those pages. Happy roll up my sleeves if we get this in.

PS From a GCS point of view I might like to see the buffer zone. Worth thinking about how this might be exposed via mavlink?

I think the buffer would be fairly straightforward to illustrate/shade within QGC similar to the exclusion shading. Given that QGC has access to the buffer distance param, hopefully no mavlink messages would be required.

mcsauder avatar Sep 01 '22 02:09 mcsauder

I need to do some additional debugging after the commander re-write before this PR will be ready again.

mcsauder avatar Nov 14 '22 17:11 mcsauder

@mcsauder I am trying to make this work only for 2nd Geofence capability Mark. If I have understood correctly from the code, commander changes are not relevant with Geofence side, right? Correct me if I am wrong, please!

farhangnaderi avatar May 25 '23 12:05 farhangnaderi

@mcsauder I am trying to make this work only for 2nd Geofence capability Mark. If I have understood correctly from the code, commander changes are not relevant with Geofence side, right? Correct me if I am wrong, please!

Hi @farhangnaderi , Everything in this PR worked perfectly until the Commander refactor, and that unfortunately broke a lot of aspects in this PR, and then I dropped the ball. If this PR is useful to you I will get back to work on it. Please feel free to push to this branch if you have any fixes already. I'll rebase against main in just a moment.

Getting PR #21448 in would simplify the diff for me a lot.

Let me know if this is priority to you. If this PR makes it in cutting out the geofence predictor would be simple.

-Mark

mcsauder avatar May 26 '23 00:05 mcsauder

Closing in favor of #22394. Future work might revisit.

mcsauder avatar Nov 21 '23 17:11 mcsauder