Allow arming without GPS #7320

TonyBlit · 2018-12-31T12:17:37Z

A bit of context before explaining this PR:

Before GPS Rescue, the quad could not be armed unless a valid GPS Fix was received
GPS Rescue introduced the parameter gps_rescue_min_sats that could be set to 0 to allow arming even if there were no sats
GPS Rescue initially forced a home point reset when the quad was disarmed. Quickly rearming the quad in a different location could miss updating the home point, so this home point reset was moved again to be done just after arming.

This PR allows arming the quad without a GPS Fix, then:

if a valid GPS Fix is received for the next 3 seconds, the home point will be updated
otherwise, the home point will not be updated, so:

home distance and direction will not show up in OSD
if GPS Rescue is enabled, a warning (NO GPS RESC) will show up

Notes:

flying without waiting GPS to produce valid fixes is now possible without messing with gps_rescue_min_sats (which was a bad practice as it required the pilot to be aware of the risk) and it now shows a clear warning (GPS not available) so the pilot is totally aware at all times of what is happening.
arming with a valid GPS_FIX and 5 or more sats will still produce a special arming beep, that's a good indication that home point will very likely be initialized properly afterwards
gps_rescue_min_sats range has been modified to a minimum of 5 which makes more sense from the point of view of gps_rescue operation itself
lat/lon coordinates will show up on the OSD as soon as they're available, regardless of home point having been updated

I've not been able to test this change on the field (yet), so I'm submitting the PR just to check if there is any concern preventing the PR to be accepted. Testing is so time consuming for me that I prefer to defer it until the PR is greenlighted.

etracer65 · 2018-12-31T15:18:23Z

I don't see the point in allowing arming and then imposing a 3 second "grace period" to allow the fix to be established. The pilot could just wait the extra 3 seconds before arming. The current state requires the pilot to explicitly allow arming without a fix by consciously changing a setting. This change allows arming without the pilot deciding that's what they want - not in favor of this.

Also remember that no everybody has or uses an OSD. So relying on that for critical decision making is a bad idea. Also many users don't have beepers and those that do may mot be audible in flight (might be using DSHOT beacon).

I'm concerned we're trying to address to many edge cases with GPS Rescue and deviating from it's actual mission - Rescue. For rescue to work the user needs to have a valid GPS fix. Having it be optional and relying on the user understanding (somehow) that they may or may not have a valid state is starting defeat the purpose.

TonyBlit · 2018-12-31T19:06:34Z

I don't see the point in allowing arming and then imposing a 3 second "grace period" to allow the fix to be established. The pilot could just wait the extra 3 seconds before arming. The current state requires the pilot to explicitly allow arming without a fix by consciously changing a setting. This change allows arming without the pilot deciding that's what they want - not in favor of this.

This is actually a bug fix, I should have explained it better. The existing code allows for arming with 0 sats, and then as soon as a fix arrives, home point is set (not necessarily where you started flying). A grace period is needed to avoid this.

Also remember that no everybody has or uses an OSD. So relying on that for critical decision making is a bad idea. Also many users don't have beepers and those that do may mot be audible in flight (might be using DSHOT beacon).

You're right, it might not be a good idea to have this feature always enabled. Adding a parameter to explicitly enable this behaviour would cover those cases. Something (not tied with min_sats please) like gps_rescue_arming = gps_fix_required / gps_rescue_auto_disable (default = gps_fix_required).

I'm concerned we're trying to address to many edge cases with GPS Rescue and deviating from it's actual mission - Rescue. For rescue to work the user needs to have a valid GPS fix. Having it be optional and relying on the user understanding (somehow) that they may or may not have a valid state is starting defeat the purpose.

In the current code the parameter gps_rescue_min_sats can be set to 0. This exists as some people want to be able to fly with rescue disabled in case there is no GPS. But the resulting behaviour of setting min_sats to 0 is so random as you don't know wether the home point will be updated, or if sanity check will stop doing its job. My proposal pursues to maintain the same functionality in a much saner/understandable way (imho).

McGiverGim · 2019-01-01T08:43:01Z

I'm not too sure about the three seconds too, but I'm for this PR. I think is a very needed feature that fixes some weaknesses of the GPS Rescue.

TonyBlit · 2019-01-01T09:09:44Z

Actually, the discussion on the three seconds thing is making me think that what is really broken is the current code. The home point should use the last valid fix if it was received within the last 3 seconds before arming, this way I think all the corner cases are covered. And for people not wanting to wait for a gps fix, GPS Rescue won't be available from the beginning.

McGiverGim · 2019-01-01T09:13:10Z

Exactly. I think this is the correct way. And +1 for the parameter to let take off without fix (and without rescue).

TonyBlit · 2019-01-01T19:36:44Z

Refactored the PR according to the above comments, much clearer now. Thanks for the feedback!

src/main/flight/gps_rescue.c

src/main/fc/core.c

McGiverGim

To me is good, but I have not tested it ;)

TonyBlit · 2019-01-06T06:47:24Z

I tested this change yesterday in a low sat environment and seems to work fine.

mikeller · 2019-01-07T10:08:06Z

src/main/io/gps.c

 //
-static void GPS_calculateDistanceFlown(bool initialize)
+void GPS_calculateDistanceFlownVerticalSpeed(bool initialize)


This should be static. And reordering the functions will get you out of having to put a forward declaration into the header file.

TBH the fact that this function takes bool initialize as an argument is messy.

@mikeller I was effectively avoiding to reorder the code, I will change it.
@stawiski I personally don't like the bool initialize as well, but it's preferred to prevent polluting the global namespace as I was told in other PRs.

mikeller · 2019-01-07T10:10:45Z

src/main/fc/core.c

@@ -279,7 +279,7 @@ void updateArmingStatus(void)

 #ifdef USE_GPS_RESCUE
        if (gpsRescueIsConfigured()) {
-            if (!gpsRescueConfig()->minSats || STATE(GPS_FIX) || ARMING_FLAG(WAS_EVER_ARMED)) {
+            if (gpsRescueConfig()->allowArmingWithoutFix || STATE(GPS_FIX)) {


This should still check ARMING_FLAG(WAS_EVER_ARMED) - otherwise it will make it possible for the pilot to land / disarm in an inaccessible location and then not being able to get out of there again if there is no GPS reception.

Well spotted! This condition was unnecessary in my former PR, but as I changed the proposal I forgot bringing this condition back. Worth noting that as a consequence of this condition, the parameter "gps_allow_arming_without_fix" will only be useful for the first arming.

McGiverGim · 2019-01-13T12:28:50Z

@TonyBlit I've been testing this today. The GPS Rescue not available message is very annoying. I understand the we must show this warning, but all the time makes this change not too useful. Maybe show it only 3 or 4 secund after take off? With this, and some type of beep, I think it will be enough for the pilot to decide if continue flying out land until GPS available...

TonyBlit · 2019-01-13T18:29:52Z

You're right, it's very annoying. I was afraid of hiding the message after a few seconds to avoid confusion with the normal "not available" message, which is hidden when gps coverage is recovered. Maybe a different message can be shown for a few seconds, for instance "RESC DISABLED". To meet the 11 char limit, it could just be "RESC OFF!!!"

McGiverGim · 2019-01-13T18:33:00Z

True, to me is ok and very descriptive...

TonyBlit · 2019-01-13T18:37:23Z

As this is already merged I'll open a new PR, hopefully later today. Thanks for the feedback!

TonyBlit force-pushed the gps_rescue_arm branch from 0dd9ef9 to c816ba3 Compare January 1, 2019 19:34

TonyBlit force-pushed the gps_rescue_arm branch 2 times, most recently from c5bc24b to 6902300 Compare January 2, 2019 08:58

McGiverGim requested changes Jan 3, 2019

View reviewed changes

src/main/flight/gps_rescue.c Outdated Show resolved Hide resolved

TonyBlit force-pushed the gps_rescue_arm branch from 6902300 to d3e41c0 Compare January 4, 2019 08:00

McGiverGim reviewed Jan 4, 2019

View reviewed changes

src/main/fc/core.c Outdated Show resolved Hide resolved

McGiverGim previously approved these changes Jan 4, 2019

View reviewed changes

IllusionFpv mentioned this pull request Jan 5, 2019

GPS off switch #7335

Closed

mikeller requested changes Jan 7, 2019

View reviewed changes

mikeller added this to the 4.0 milestone Jan 7, 2019

mikeller added the RN: IMPROVEMENT label Jan 7, 2019

GPS Rescue: allow arming without GPS fix (refactor)

007e14f

TonyBlit dismissed McGiverGim’s stale review via 007e14f January 7, 2019 13:27

TonyBlit force-pushed the gps_rescue_arm branch from d3e41c0 to 007e14f Compare January 7, 2019 13:27

mikeller approved these changes Jan 10, 2019

View reviewed changes

mikeller merged commit 6e42e3f into betaflight:master Jan 10, 2019

matoy mentioned this pull request Mar 24, 2019

Betaflight 3.4 RC2 no GPS no arming problem #6198

Closed

nerdCopter mentioned this pull request Feb 14, 2020

Feature Request: GPS Allow Arming (without lock) emuflight/EmuFlight#82

Closed

McGiverGim mentioned this pull request Jan 2, 2023

Disarm on fail-safe when allow arming without fix is enabled and failsafe procedure is set to GPS-RESCUE #12120

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow arming without GPS #7320

Allow arming without GPS #7320

TonyBlit commented Dec 31, 2018

etracer65 commented Dec 31, 2018

TonyBlit commented Dec 31, 2018

McGiverGim commented Jan 1, 2019

TonyBlit commented Jan 1, 2019

McGiverGim commented Jan 1, 2019

TonyBlit commented Jan 1, 2019

McGiverGim left a comment

TonyBlit commented Jan 6, 2019

mikeller Jan 7, 2019

stawiski Jan 7, 2019

TonyBlit Jan 7, 2019

mikeller Jan 7, 2019

TonyBlit Jan 7, 2019

McGiverGim commented Jan 13, 2019

TonyBlit commented Jan 13, 2019

McGiverGim commented Jan 13, 2019

TonyBlit commented Jan 13, 2019

Allow arming without GPS #7320

Allow arming without GPS #7320

Conversation

TonyBlit commented Dec 31, 2018

etracer65 commented Dec 31, 2018

TonyBlit commented Dec 31, 2018

McGiverGim commented Jan 1, 2019

TonyBlit commented Jan 1, 2019

McGiverGim commented Jan 1, 2019

TonyBlit commented Jan 1, 2019

McGiverGim left a comment

Choose a reason for hiding this comment

TonyBlit commented Jan 6, 2019

mikeller Jan 7, 2019

Choose a reason for hiding this comment

stawiski Jan 7, 2019

Choose a reason for hiding this comment

TonyBlit Jan 7, 2019

Choose a reason for hiding this comment

mikeller Jan 7, 2019

Choose a reason for hiding this comment

TonyBlit Jan 7, 2019

Choose a reason for hiding this comment

McGiverGim commented Jan 13, 2019

TonyBlit commented Jan 13, 2019

McGiverGim commented Jan 13, 2019

TonyBlit commented Jan 13, 2019