Adding initial support for NTRIP / TCP source of RTCM data. #9493

TobinHall · 2021-02-27T23:57:33Z

Currently RTK corrections are supported using a GPS attached to the ground station.
NTRIP is often a lot more convenient and can be used with mobile devices.

This implementation is based off the ADSBVehicleManager module which shares a lot of the requirements with NTRIP.

This implementation is fairly crude without a lot of error-checking. Because I borrowed heavily from the ADSB implementation, the split into two classes (NTRIP and NTRIPTCPLink) might not be appropriate until more than one stream is supported.

This implementation does not touch querying the server for the caster table. This functionality could reasonably be added.

Currently there are no reconnection attempts and the settings require restart. This could be changed fairly easily.

I have not done anything to show the NTRIP / RTCM status, I am just looking at changes to the fix type of the GPS.

I am fairly new to the codebase, so some of the decisions I made here may not be ideal.

Should fix #7133 and is a possible fix / alternative to #3946.

TobinHall · 2021-02-28T00:00:28Z

@DonLakeFlyer

LorenzMeier · 2021-02-28T10:17:57Z

Thanks! What is your testing level - have you done full missions with this in the loop yet? Which provider are you using?

TobinHall · 2021-02-28T19:18:34Z

Testing is still fairly minimal at this stage. I would appreciate some additional testers, it's a bit hard for me to fly missions in my city.

I am using the PositioNZ service provided by Land Information New Zealand.
I have also tested this with the base station we use for our UGVs which has a plain TCP connection.

I have flown a mission with this, the telemetry kept dropping out.
I suspect that the RTCM bandwidth was too high for my cheap radios at 57600.
If anyone has any radio suggestions that will run from USB on the ground station side, that would be great.

Part of the problem is that because this immediately connects, the corrections are being forwarded over mavlink when the parameters are being pulled through. This makes the parameter transfer take a long long time.
It might be a good idea to reduce or stop the RTCM when parameters are being transferred. I haven't really looked at the mavlink code, so I am not sure how easy it would be to de-prioritise RTCM injection.

I am currently looking at adding the ability to throttle the corrections based of message type. I have already got the message id out of the RTCM messages. Throttling messages by type should be fairly easy to implement in RTCMMavlink.

I am also looking at adding RTCM message white and blacklists. The ublox M8P I have on the drone can only use less than half of the messages that the reference station produces. Whitelisting these messages would drastically reduce bandwidth requirements.

patrickelectric · 2021-03-11T11:55:36Z

src/NTRIP/NTRIP.cc

+    start();
+    for(const auto& msg: whitelist.split(',')){
+        int msg_int = msg.toInt();
+        if(msg_int)


In some parts of the code there is single line ifs with brackets and others without, be consistent in the some code base.

patrickelectric · 2021-03-11T12:19:17Z

src/NTRIP/NTRIP.h

+    void RTCMDataUpdate(QByteArray message);
+
+protected:
+    void run(void) final;


just a tip, (void) is not necessary in C++ code.

patrickelectric · 2021-03-11T12:20:31Z

src/NTRIP/NTRIP.cc

+
+void NTRIPTCPLink::_readBytes(void)
+{
+    if (_socket) {


To avoid the extra indentation level

if (!socket) { return; }

patrickelectric · 2021-03-11T12:21:51Z

src/NTRIP/NTRIP.cc

+    , _hostAddress  (hostAddress)
+    , _port         (port)
+    , _username     (username)
+    , _password     (password)


Why it's not using the QUrl object, it should provide all these fields for you.

patrickelectric · 2021-03-11T12:22:09Z

src/NTRIP/NTRIP.cc

+    // If mountpoint is specified, send an http get request for data
+    if ( !_mountpoint.isEmpty()){
+        qCDebug(NTRIPLog) << "Sending HTTP request";
+        QString auth = QString(_username + ":"  + _password).toUtf8().toBase64();


QUrl can also be used for it

patrickelectric · 2021-03-11T12:23:19Z

src/NTRIP/NTRIP.h

+
+#include <QThread>
+#include <QTcpSocket>
+#include <QTimer>


QTimer appears to be not used

patrickelectric · 2021-03-11T12:23:37Z

src/NTRIP/NTRIP.h

+#include <QThread>
+#include <QTcpSocket>
+#include <QTimer>
+#include <QGeoCoordinate>


The same things appears to happen with QGeoCoordinate

patrickelectric · 2021-03-11T12:25:57Z

src/ui/preferences/GeneralSettings.qml

+                        visible:    QGroundControl.settingsManager.ntripSettings.visible
+                    }
+                    Rectangle {
+                        Layout.preferredHeight: ntripGrid.y + ntripGrid.height + _margins


Layout.margins ?
and Layout.topMargin

TobinHall · 2021-03-11T20:31:43Z

@patrickelectric Thanks for your feedback.
Many of the things you pointed out come from the implementation I copied as a skeleton.
I didn't spend much time trying to figure out how the qml works, just threw something together.
I will have a look at cleaning it up this weekend.

DonLakeFlyer · 2021-03-16T18:38:37Z

@bkueng Can you take a look? You kind of the RTK person.

bkueng

Not much to add here, the structure is fine. And definitely a nice feature to add.

As you mention another cleanup round with error handling is required
Having UI feedback would be nice
Not sure if it's relevant in practice, but we might want to prevent the simultaneous use of a base gps + ntrip

bkueng · 2021-03-17T10:38:23Z

src/Settings/NTRIP.SettingsGroup.json

+    "shortDesc":     "RTCM Message Whitelist",
+    "longDesc":      "RTCM Messages to send over mavlink. Leave blank for all messages",
+    "type":                 "string",
+    "default":         "",


Can we add a list of defaults here (for u-blox)?

I thought about it. I was not sure whether to add them in the long description or as defaults.
I am also thinking about adding per-message rate limiting. I was considering the following syntax:
1074@0.5, 1084@0.5, 1124@0.5
-1@0.5, 1074, 1084, 1124
Where @x would specify the maximum frequency for each message. or all (-1) messages.

Right, that would provide maximum flexibility, but it seems to me almost too complex already.

bkueng · 2021-03-17T12:00:32Z

src/NTRIP/NTRIP.cc

+    , _mountpoint   (mountpoint)
+{
+    moveToThread(this);
+    start();


You need to start at the end of initialization, otherwise there are race conditions

bkueng · 2021-03-17T12:03:49Z

src/NTRIP/NTRIP.cc

+    quit();
+    wait();


Same here: quit & wait first, then cleanup.
Also there's a memory leak for _rtcm_parsing

bkueng · 2021-03-17T12:22:49Z

src/NTRIP/NTRIP.h

+    QString         _username;
+    QString         _password;
+    QString         _mountpoint;
+    QVector<int>    _whitelist;


Suggested change

QVector<int> _whitelist;

QSet<int> _whitelist;

DonLakeFlyer · 2021-04-19T16:10:26Z

@TobinHall This is waiting on review updates

DonLakeFlyer · 2021-05-04T19:57:14Z

@TobinHall Updates?

TobinHall · 2021-05-08T23:58:03Z

@TobinHall Updates?

Hi @DonLakeFlyer.
Unfortunately, my circumstances have changed and II have not been able to make the suggested changes yet. I may not be able to for a few weeks.

mrpollo · 2021-06-11T15:19:42Z

@bkueng hey do you have a few free cycles to get this PR to the finish line? the main author is MIA, and it seems like a nice feature

bkueng · 2021-06-15T06:49:37Z

I see what I can do, but I can't promise anything.

mkameke · 2021-09-12T12:34:07Z

Any news? Would be a very great improvement for QGC.

TobinHall · 2021-09-12T19:55:04Z

Any news? Would be a very great improvement for QGC.

Unfortunately, I have not had the time to polish this up and test it.

It does still need some work. Particularly around the UI.

Feedback, statistics, and being able to start and stop corrections would be good features to add.

I cant recall if the corrections were paused during parameter sync. It really needs to be.

Tobin.

syt011019 · 2022-01-26T07:31:25Z

Does the ntrip work on this custom code?

AndKe · 2022-02-01T09:49:52Z

I get this when building the PR branch:

compiling /home/andre/Downloads/qgroundcontrol/src/QmlControls/ScreenToolsController.cc
/home/andre/Downloads/qgroundcontrol/src/NTRIP/NTRIP.cc: In member function ‘void NTRIPTCPLink::_parse(const QByteArray&)’:
/home/andre/Downloads/qgroundcontrol/src/NTRIP/NTRIP.cc:130:24: error: loop variable ‘byte’ of type ‘const uint8_t&’ {aka ‘const unsigned char&’} binds to a temporary constructed from type ‘const char’ [-Werror=range-loop-construct]
  130 |     for(const uint8_t& byte : buffer){
      |                        ^~~~
/home/andre/Downloads/qgroundcontrol/src/NTRIP/NTRIP.cc:130:24: note: use non-reference type ‘const uint8_t’ {aka ‘const unsigned char’} to make the copy explicit or ‘const char&’ to prevent copying
cc1plus: all warnings being treated as errors
make: *** [Makefile:167102: NTRIP.o] Error 1
make: *** Waiting for unfinished jobs....

Keep up the good work, I would very much like to see this feature in QGC.

TobinHall · 2022-02-01T19:25:22Z

Hi guys. There seems to be a bit of renewed interest in this functionality.
Unfortunately, I have been unable to revisit this (or fly my quad) since I hacked together the original implementation.
There are obviously things that need to be done to tidy up the code and fix a few issues. I am also not sure how much the rest of the code base has changed since I wrote this PR, so it may now be wildly inconsistent.

I am still not in a position to get this over the line.

I get this when building the PR branch:

compiling /home/andre/Downloads/qgroundcontrol/src/QmlControls/ScreenToolsController.cc
/home/andre/Downloads/qgroundcontrol/src/NTRIP/NTRIP.cc: In member function ‘void NTRIPTCPLink::_parse(const QByteArray&)’:
/home/andre/Downloads/qgroundcontrol/src/NTRIP/NTRIP.cc:130:24: error: loop variable ‘byte’ of type ‘const uint8_t&’ {aka ‘const unsigned char&’} binds to a temporary constructed from type ‘const char’ [-Werror=range-loop-construct]
  130 |     for(const uint8_t& byte : buffer){
      |                        ^~~~
/home/andre/Downloads/qgroundcontrol/src/NTRIP/NTRIP.cc:130:24: note: use non-reference type ‘const uint8_t’ {aka ‘const unsigned char’} to make the copy explicit or ‘const char&’ to prevent copying
cc1plus: all warnings being treated as errors
make: *** [Makefile:167102: NTRIP.o] Error 1
make: *** Waiting for unfinished jobs....

Keep up the good work, I would very much like to see this feature in QGC.

I am not sure how my build setup is different from yours, I don't see this warning. But I think it should be fine to remove the & as the compiler output suggests:
for(const uint8_t& byte : buffer){
Becomes:
for(const uint8_t byte : buffer){

The primary issue I had with this functionality was integrating the GUI elements. I did not like my implementation of this.
Someone with more experience in the project should be able to do a better job more quickly than me fumbling through it.

andresiko · 2022-05-19T08:28:24Z

wanting this implementation, but coding is definitely out of my abilities.
Own a CUAV X7, and a H16 radio controller, and also access to local NTRIP data provider, definitely willing to help testing code if needed.

highfreq · 2022-12-21T20:55:29Z

Still polishing the code after 2 years? I bet is shining like an Harley by now :)

swimmingseeds · 2023-02-16T00:43:56Z

This pull request has been mentioned on Discussion Forum, for PX4, Pixhawk, QGroundControl, MAVSDK, MAVLink. There might be relevant details there:

https://discuss.px4.io/t/px4-community-q-a-february-15-2023/30744/1

jjmmss00 · 2024-03-07T15:03:10Z

I would like this feature. As a workaround I am able to fly with mission planner, and configure and plan with QGC. Seems that this would be very valuable feature. Perhaps one could look at the MP code for guidance on the implementation.

TobinHall added 2 commits February 28, 2021 12:09

Minimal (non-functional) implementation based off ADSBVehicleManager

d46b0fe

Adding NTRIP settings. Crude working implementation.

c489409

RTCM is now included in mobile builds

c4d014a

LorenzMeier added this to In progress in High Priority Issues via automation Feb 28, 2021

Basic RTCM whitelisting

5b836d8

patrickelectric reviewed Mar 11, 2021

View reviewed changes

src/NTRIP/NTRIP.h

#include <QThread>

#include <QTcpSocket>

#include <QTimer>

Copy link

Member

patrickelectric Mar 11, 2021

Choose a reason for hiding this comment

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

QTimer appears to be not used

patrickelectric reviewed Mar 11, 2021

View reviewed changes

DonLakeFlyer requested a review from bkueng March 16, 2021 18:38

bkueng reviewed Mar 17, 2021

View reviewed changes

WTPENGUIN mentioned this pull request Jan 25, 2022

qgroundcontrol ntrip #10139

Closed

WTPENGUIN mentioned this pull request Dec 22, 2022

Add NTRIP with VRS support #10524

Closed

3 tasks

HTRamsey marked this pull request as draft April 12, 2024 12:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding initial support for NTRIP / TCP source of RTCM data. #9493

Adding initial support for NTRIP / TCP source of RTCM data. #9493

TobinHall commented Feb 27, 2021

TobinHall commented Feb 28, 2021

LorenzMeier commented Feb 28, 2021

TobinHall commented Feb 28, 2021

patrickelectric Mar 11, 2021

patrickelectric Mar 11, 2021

patrickelectric Mar 11, 2021

patrickelectric Mar 11, 2021

patrickelectric Mar 11, 2021

patrickelectric Mar 11, 2021

patrickelectric Mar 11, 2021

patrickelectric Mar 11, 2021

TobinHall commented Mar 11, 2021

DonLakeFlyer commented Mar 16, 2021

bkueng left a comment

bkueng Mar 17, 2021

TobinHall Mar 17, 2021

bkueng Mar 18, 2021

bkueng Mar 17, 2021

bkueng Mar 17, 2021

bkueng Mar 17, 2021

DonLakeFlyer commented Apr 19, 2021

DonLakeFlyer commented May 4, 2021

TobinHall commented May 8, 2021

mrpollo commented Jun 11, 2021

bkueng commented Jun 15, 2021

mkameke commented Sep 12, 2021

TobinHall commented Sep 12, 2021

syt011019 commented Jan 26, 2022

AndKe commented Feb 1, 2022 •

edited

TobinHall commented Feb 1, 2022

andresiko commented May 19, 2022

highfreq commented Dec 21, 2022

swimmingseeds commented Feb 16, 2023

jjmmss00 commented Mar 7, 2024

Adding initial support for NTRIP / TCP source of RTCM data. #9493

Are you sure you want to change the base?

Adding initial support for NTRIP / TCP source of RTCM data. #9493

Conversation

TobinHall commented Feb 27, 2021

TobinHall commented Feb 28, 2021

LorenzMeier commented Feb 28, 2021

TobinHall commented Feb 28, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TobinHall commented Mar 11, 2021

DonLakeFlyer commented Mar 16, 2021

bkueng left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DonLakeFlyer commented Apr 19, 2021

DonLakeFlyer commented May 4, 2021

TobinHall commented May 8, 2021

mrpollo commented Jun 11, 2021

bkueng commented Jun 15, 2021

mkameke commented Sep 12, 2021

TobinHall commented Sep 12, 2021

syt011019 commented Jan 26, 2022

AndKe commented Feb 1, 2022 • edited

TobinHall commented Feb 1, 2022

andresiko commented May 19, 2022

highfreq commented Dec 21, 2022

swimmingseeds commented Feb 16, 2023

jjmmss00 commented Mar 7, 2024

AndKe commented Feb 1, 2022 •

edited