Skip to content

Conversation

@KarateBrot
Copy link
Member

@KarateBrot KarateBrot commented Nov 25, 2022

This PR fixes some bugs/problems on both NMEA and U-blox protocols to make DOP values work properly. In addition I refactored the code to save some flash and added the U-blox accuracy estimate for position to the GPS solution for use in BF.

The commit names describe in detail what I changed.

Thanks a lot to @ctzsnooze for laying the groundwork for this PR and identifying the problems.

Also thanks a lot to the testers: @benjamind2 and Sek101

@KarateBrot KarateBrot added this to the 4.4 milestone Nov 25, 2022
@KarateBrot KarateBrot self-assigned this Nov 25, 2022
@KarateBrot KarateBrot marked this pull request as draft November 25, 2022 13:50
@github-actions

This comment has been minimized.

@benjamind2
Copy link
Contributor

benjamind2 commented Dec 1, 2022

Tested, works, provided Debug=GPS_DOP log on discord, from commit e521d748b2060b7fd0db9ee204a4fb69fb990d2c.

Used "Flywoo GM8 Mini V2.0 GPS Module" at 115200 baud on a F7X2 SkyStars FC.

Observed very fast lock, but that could be coincidental. Coordinate map, speed, satellites, altitude, distance from home all look good, meet or exceed previous behavior.

@KarateBrot
Copy link
Member Author

KarateBrot commented Dec 1, 2022

@benjamind2 Thanks for testing! You tested this with the UBlox protocol, so now we need to make sure this also works on the NMEA protocol 👍

Edit:
Benjamin tested this on NMEA as well and we are getting valid DOP values (Discord conversation). DOP values are now confirmed to work on UBlox and NMEA.

But there is a bug where velocity for GPS Rescue gets now updated by several GPS messages with different values. There is a fix ready for testing. Tomorrow we will know more.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions
Copy link

github-actions bot commented Dec 6, 2022

Do you want to test this code? Here you have an automated build:
Assets
WARNING: It may be unstable. Use only for testing! See: https://www.youtube.com/watch?v=I1uN9CN30gw for instructions for unified targets!

@KarateBrot KarateBrot marked this pull request as ready for review December 6, 2022 01:48
@blckmn
Copy link
Member

blckmn commented Dec 6, 2022

AUTOMERGE: (FAIL)

  • github identifies PR as mergeable -> PASS
  • assigned to a milestone -> PASS
  • cooling off period lapsed -> PASS
  • commit count less or equal to three -> FAIL
  • Don't merge label NOT found -> PASS
  • at least one RN: label found -> PASS
  • Tested label found -> PASS
  • assigned to an approver -> PASS
  • approver count at least three -> PASS

@KarateBrot
Copy link
Member Author

But there is a bug where velocity for GPS Rescue gets now updated by several GPS messages with different values. There is a fix ready for testing. Tomorrow we will know more.

Sek101 confirmed with logs that it is now fixed

@KarateBrot KarateBrot merged commit 82c23ed into betaflight:master Dec 8, 2022
@KarateBrot KarateBrot deleted the gpsQueryCtrl branch December 8, 2022 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants