Skip to content

feat: add U-BLOX GPS support#4689

Merged
pfeerick merged 5 commits into
EdgeTX:mainfrom
DavBfr:gps_ubx
Apr 7, 2024
Merged

feat: add U-BLOX GPS support#4689
pfeerick merged 5 commits into
EdgeTX:mainfrom
DavBfr:gps_ubx

Conversation

@DavBfr

@DavBfr DavBfr commented Mar 3, 2024

Copy link
Copy Markdown
Contributor

Add U-BLOX GPS support.

Summary of changes:

  • Move GPS NMEA support to a dedicated file
  • Add RS232 support to the simulator
  • Add support for UBX GPS protocol
  • Fix Altitude type to show negative values

Demonstration Video: https://youtu.be/i5LhTFyInUY

To debug RS232 with the simulator, pass -DSIMU_COM_PORT=2 where 2 is the third COM port on the machine as listed in the port selection of betaflight for example.
Only one PORT is available to the simulator AUX1 or AUX2.

The protocol and baud rate are automatically detected with 200 ms hops. The first protocol that generates a valid frame is selected. If the GPS stops sending data for any reasons, the auto-detection is restarted.

@pfeerick pfeerick added enhancement ✨ New feature or request contest labels Mar 3, 2024
@DavBfr DavBfr force-pushed the gps_ubx branch 2 times, most recently from 32dd77e to 20ef41e Compare March 4, 2024 21:25
@rotorman

rotorman commented Mar 5, 2024

Copy link
Copy Markdown
Member

Tested this PR with TX16S and Holybro Pixhawk 4 GPS set to NMEA 9600 baud to check compatibility with the previous working state of EdgeTX radio side GPS support. Using TxGPStest widget for testing.
The altitude does not work correctly with this PR. It decrements downwards. By flashing back v2.9.4 - altitude works fine. Re-flashing this PR - altitude does not work correctly. @DavBfr Please double check.

@rotorman

rotorman commented Mar 5, 2024

Copy link
Copy Markdown
Member

Next tested with Matek M10Q-5883 GPS, also with TX16S and TxGPStest widget. Also here, other values than altitude are fine. Although, with UBX GPS, I do not see incremental downwards counting of altitude, just fully wrong (negative) value.

All tests were carried out outside with 10+ satellites in view.

@DavBfr

DavBfr commented Mar 5, 2024

Copy link
Copy Markdown
Contributor Author

Tested this PR with TX16S and Holybro Pixhawk 4 GPS set to NMEA 9600 baud to check compatibility with the previous working state of EdgeTX radio side GPS support. Using TxGPStest widget for testing. The altitude does not work correctly with this PR. It decrements downwards. By flashing back v2.9.4 - altitude works fine. Re-flashing this PR - altitude does not work correctly. @DavBfr Please double check.

Right, a remaining debug data. It's removed.

@rotorman

rotorman commented Mar 5, 2024

Copy link
Copy Markdown
Member

Re-tested with M8Q and M10Q GPS, both work. Well done!

@DavBfr

DavBfr commented Mar 5, 2024

Copy link
Copy Markdown
Contributor Author

Fixed the UBX altitude that was 10 x to high.

@rotorman

rotorman commented Mar 5, 2024

Copy link
Copy Markdown
Member

Hmm... TxGPStest widget showed correct altitude for M8Q and M10Q GPSs for me before 4c76f16 commit though?

@DavBfr

DavBfr commented Mar 5, 2024

Copy link
Copy Markdown
Contributor Author

I tested with the same GPS with NMEA and UBX I had different altitudes. And looking at where I am on google earth it shows something more like the NMEA altitude. I'm a few km form the ocean, so 500m altitude is not right.

@pfeerick pfeerick added this to the 2.11 milestone Mar 6, 2024
@pfeerick pfeerick changed the title Add U-BLOX GPS support feat: add U-BLOX GPS support Mar 14, 2024
Comment thread radio/src/gps.cpp
Comment on lines +195 to +198
if (get_tmr10ms() - time > 20) {
changeBaudrate();
time = get_tmr10ms();
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it might make sense to couple this timeout to the official ACK/NAK timeout of 1s, page 179 in this receiver spec. My experience says that this lower timeout is not an issue, but it is not according to the spec

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it's a bit aggressive 1s seems a lot though.

@bastian2001

Copy link
Copy Markdown
Contributor

The autodetection detects my UBX GPS as NMEA. My assumption is that this is because some GPS modules are preconfigured to output certain NMEA messages by default. In my case, the GPS outputs GGA, GSA, GSV, RMC, VTG and GLL messages in the NMEA format, thus autodetectProtocol recognizes it as a NMEA device. These NMEA messages can be disabled, and the UBX counterparts can be enabled. I'll see tomorrow what I can do about that.

@pfeerick

pfeerick commented Mar 15, 2024

Copy link
Copy Markdown
Member

That of itself isn't that big of a problem IMO... the primary goal of the autodetection is to allow you to plug in a GPS, and it just work... so if it is configures as both NMEA/UBX, it still "just works". This is where there is potential scope for UI options - i.e. where you can set it to NMEA, UBX, Auto perhaps?

@bastian2001

Copy link
Copy Markdown
Contributor

Yeah, it'll just work. But on the other hand, afaik all UBlox GPS modules can speak NMEA. I think UBX should be the encouraged protocol, regardless of the default of that module. UBX uses less computation (binary instead of decimal), the satellite count is more accurate and it is configurable. A toggle would resolve this problem, but it is yet another option the user has to change. I am currently making myself familiar with the codebase, cause I have not done any EdgeTX development yet and I am not familiar with it.

Is there any interest for Open Location Codes / Plus Codes? I have a pretty easy to implement function to generate one in my FC, but I can't think of a use case for it in EdgeTX.

@pfeerick

Copy link
Copy Markdown
Member

Yes, AFAIK, that it is the case (uBlox GPSs able to talk NMEA). However, there are GPSs now that are configured with UBX out of the box, hence why the contest for UBX support to be added to ETX, and the autodetection makes it so the user wouldn't even know.

Plus codes are already possible with https://github.com/kristjanbjarni/opentx-widgets?tab=readme-ov-file#gps-widget-widgetsgpsmainlua

@bastian2001

bastian2001 commented Mar 15, 2024

Copy link
Copy Markdown
Contributor

I wrote some code to prefer UBX over NMEA. For details check the PR. I also increased the timeout to 500ms. You can test it by unplugging the GPS RX/handset TX wire when your GPS is preconfigured with NMEA. My GPS is recognized as a NMEA in that case. When I plug the wire back in, I get UBX 👍🏻

@DavBfr

DavBfr commented Mar 15, 2024

Copy link
Copy Markdown
Contributor Author

I think it's the way to go: prefer UBX if we can and fallback to NMEA. No need for a user input, no need to save the configuration to the GPS unit.

@pfeerick pfeerick merged commit 4b8f80a into EdgeTX:main Apr 7, 2024
@pfeerick pfeerick mentioned this pull request Apr 8, 2024
1 task
@rotorman rotorman mentioned this pull request Apr 21, 2024
1 task
@pfeerick pfeerick added the rn: feature Feature to be highlighted in release notes label Apr 28, 2024
@villamany

Copy link
Copy Markdown

GPS broken in v2.11. Do several test and getting issues with Beitian BN200 in v2.11. EdgeTx still sending commands to GPS all the time
V2.10 works fine. #6518

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contest enhancement ✨ New feature or request rn: feature Feature to be highlighted in release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants