Fix network provider issues

app/position/providers/networkpositionprovider.cpp

@@ -34,27 +34,24 @@ NetworkPositionProvider::NetworkPositionProvider( const QString &addr, const QSt
   mReconnectTimer.setSingleShot( false );
   mReconnectTimer.setInterval( ONE_SECOND_MS );
   connect( &mReconnectTimer, &QTimer::timeout, this, &NetworkPositionProvider::reconnectTimeout );
-  mUdpReconnectTimer.setSingleShot( true );
-  connect( &mUdpReconnectTimer, &QTimer::timeout, this, [this]
+  mHeartBeatTimer.setSingleShot( true );
+  connect( &mHeartBeatTimer, &QTimer::timeout, this, [this]
   {
-    if ( mTcpSocket->state() != QAbstractSocket::ConnectedState )
-    {
-      setState( tr( "No connection" ), State::NoConnection );
-      startReconnectTimer();
-      // let's also invalidate current position since we no longer have connection
-      emit positionChanged( GeoPosition() );
-    }
+    setState( tr( "No connection" ), State::NoConnection );
+    startReconnectTimer();
+    // let's also invalidate current position since we no longer have connection
+    emit positionChanged( GeoPosition() );
   } );
 
   NetworkPositionProvider::startUpdates();
 }
 
 void NetworkPositionProvider::startUpdates()
 {
-  // TODO: QHostAddress doesn't support hostname lookup (QHostInfo does)
+  // NOTE: QHostAddress doesn't support hostname lookup (QHostInfo does)
   mTcpSocket->connectToHost( mTargetAddress, mTargetPort );
   mUdpSocket->bind( QHostAddress::LocalHost, mTargetPort );
-  mUdpReconnectTimer.start( ReconnectDelay::ExtraLongDelay );
+  mHeartBeatTimer.start( ReconnectDelay::ExtraLongDelay );
 }
 
 void NetworkPositionProvider::stopUpdates()
@@ -76,7 +73,7 @@ NetworkPositionProvider::~NetworkPositionProvider()
 
 void NetworkPositionProvider::closeProvider()
 {
-  mUdpReconnectTimer.stop();
+  mHeartBeatTimer.stop();
   mReconnectTimer.stop();
 
   if ( mTcpSocket )
@@ -99,7 +96,7 @@ void NetworkPositionProvider::positionUpdateReceived()
   // this approach will let us use QIODevice functions for both sockets
   if ( socket->socketType() == QAbstractSocket::UdpSocket && mUdpSocket->state() != QAbstractSocket::ConnectedState )
   {
-    mUdpReconnectTimer.stop();
+    mHeartBeatTimer.stop();
 
     // if by any chance we showed wrong message in the status like "no connection", fix it here
     // we know the connection is working because we just received data from the device
@@ -126,15 +123,14 @@ void NetworkPositionProvider::positionUpdateReceived()
     {
       mUdpSocket->connectToHost( peerAddress.toString(), peerPort );
     }
+
+    // restart silence timer
+    mHeartBeatTimer.start();
     return;
   }
 
-  // stop the UDP silence timer, we just received data
-  // kills the timer when the app was minimized, and we were able to reconnect in the meantime
-  if ( socket->socketType() == QAbstractSocket::UdpSocket )
-  {
-    mUdpReconnectTimer.stop();
-  }
+  // restart the silence timer, we just received data
+  mHeartBeatTimer.start();
 
   const QByteArray rawNmeaData = socket->readAll();
 
@@ -169,15 +165,10 @@ void NetworkPositionProvider::socketStateChanged( const QAbstractSocket::SocketS
   }
   else if ( state == QAbstractSocket::UnconnectedState )
   {
-    const bool isUdpSocketListening = mUdpSocket->state() == QAbstractSocket::ConnectedState || mUdpSocket->state() == QAbstractSocket::BoundState || mUdpReconnectTimer.isActive();
-    if ( socket->socketType() == QAbstractSocket::TcpSocket && !isUdpSocketListening && QApplication::applicationState() == Qt::ApplicationActive )
-    {
-      setState( tr( "No connection" ), State::NoConnection );
-      startReconnectTimer();
-      // let's also invalidate current position since we no longer have connection
-      emit positionChanged( GeoPosition() );
-    }
-    else if ( socket->socketType() == QAbstractSocket::UdpSocket && QApplication::applicationState() == Qt::ApplicationActive )
+    const bool isUdpSocketListening = mUdpSocket->state() == QAbstractSocket::ConnectedState || mUdpSocket->state() == QAbstractSocket::BoundState || mHeartBeatTimer.isActive();
+    const bool isTcpSocketAndUdpNotListening = socket->socketType() == QAbstractSocket::TcpSocket && !isUdpSocketListening && QApplication::applicationState() == Qt::ApplicationActive;
+    const bool isUdpSocket = socket->socketType() == QAbstractSocket::UdpSocket && QApplication::applicationState() == Qt::ApplicationActive;
+    if ( isTcpSocketAndUdpNotListening || isUdpSocket )
     {
       setState( tr( "No connection" ), State::NoConnection );
       startReconnectTimer();

app/position/providers/networkpositionprovider.h

@@ -59,7 +59,7 @@ class NetworkPositionProvider : public AbstractPositionProvider
     int mReconnectDelay = ReconnectDelay::ShortDelay; // in how many [ms] we will try to reconnect again
     int mSecondsLeftToReconnect; // how many seconds are left to reconnect. Reconnects if less than or equal to one
     QTimer mReconnectTimer; // timer that times out each second and lowers the mSecondsLeftToReconnect by one
-    QTimer mUdpReconnectTimer; // timer that times out after ExtraLongDelay and triggers reconnect
+    QTimer mHeartBeatTimer; // timer that times out after ExtraLongDelay and triggers reconnect
 
     QString mTargetAddress; // IP address or hostname of the receiver
     int mTargetPort; // active port of the receiver

app/position/providers/positionprovidersmodel.cpp

@@ -19,7 +19,7 @@ PositionProvidersModel::PositionProvidersModel( QObject *parent ) : QAbstractLis
 {
   if ( !InputUtils::isMobilePlatform() )
   {
-    const PositionProvider simulated( "Simulated provider", "Simulated position around point", "internal", "simulated" );
+    const PositionProvider simulated( tr( "Simulated provider" ), tr( "Simulated position around point" ), QStringLiteral( "internal" ), QStringLiteral( "simulated" ) );
 
     mProviders.push_front( simulated );
   }
@@ -30,8 +30,8 @@ PositionProvidersModel::PositionProvidersModel( QObject *parent ) : QAbstractLis
   PositionProvider internal;
   internal.name = tr( "Internal" );
   internal.description = tr( "GPS receiver of this device" );
-  internal.providerType = "internal";
-  internal.providerId = "devicegps";
+  internal.providerType = QStringLiteral( "internal" );
+  internal.providerId = QStringLiteral( "devicegps" );
 
   mProviders.push_front( internal );
 
@@ -68,6 +68,7 @@ QHash<int, QByteArray> PositionProvidersModel::roleNames() const
   roles.insert( DataRoles::ProviderName, QByteArray( "providerName" ) );
   roles.insert( DataRoles::ProviderDescription, QByteArray( "providerDescription" ) );
   roles.insert( DataRoles::ProviderType, QByteArray( "providerType" ) );
+  roles.insert( DataRoles::ProviderGroup, QByteArray( "providerGroup" ) );
   roles.insert( DataRoles::ProviderId, QByteArray( "providerId" ) );
   return roles;
 }
@@ -103,6 +104,15 @@ QVariant PositionProvidersModel::data( const QModelIndex &index, const int role
     case DataRoles::ProviderType:
       return provider.providerType;
 
+    case DataRoles::ProviderGroup:
+    {
+      if ( provider.providerType == QStringLiteral( "internal" ) )
+      {
+        return QStringLiteral( "internal" );
+      }
+      return QStringLiteral( "external" );
+    }
+
     default:
       return {};
   }
@@ -164,6 +174,14 @@ void PositionProvidersModel::addProvider( const QString &name, const QString &pr
   }
 }
 
+bool PositionProvidersModel::providerExists( const QString &providerId )
+{
+  return std::any_of( mProviders.begin(), mProviders.end(), [providerId]( const PositionProvider & provider )
+  {
+    return provider.providerId == providerId;
+  } );
+}
+
 AppSettings *PositionProvidersModel::appSettings() const
 {
   return mAppSettings;

app/position/providers/positionprovidersmodel.h

@@ -58,7 +58,8 @@ class PositionProvidersModel : public QAbstractListModel
       ProviderName = Qt::UserRole + 1, // name of bluetooth device or custom name for network device
       ProviderDescription, // device address (IP/BT) + device type
       ProviderId, // device address (IP/BT)
-      ProviderType // external_ip (connected) / external_bt (connected) / internal (device) / simulated (device)
+      ProviderType, // external_ip (connected) / external_bt (connected) / internal (device) / simulated (device)
+      ProviderGroup // internal / external
     };
     Q_ENUM( DataRoles )
 
@@ -69,6 +70,7 @@ class PositionProvidersModel : public QAbstractListModel
 
     Q_INVOKABLE void removeProvider( const QString &providerId );
     Q_INVOKABLE void addProvider( const QString &providerName, const QString &providerId, const QString &providerType );
+    Q_INVOKABLE bool providerExists( const QString &providerId );
 
     AppSettings *appSettings() const;
     void setAppSettings( AppSettings * );

app/qml/gps/MMExternalProviderConnectionDrawer.qml

@@ -67,18 +67,18 @@ MMComponents.MMDrawer {
             : "" )
           message.description: root.providerType === "bluetooth"
             ? qsTr( "We were not able to connect to the specified device. Please make sure your device is powered on and can be connected to." )
-            : qsTr( "We were not able to connect to the specified IP address. Please try again later." )
+            : qsTr( "We were not able to connect to the specified IP address." )
           message.linkText: qsTr( "Learn more" )
         }
       },
       State {
         name: "waitingToReconnect"
         when: root.positionProvider && root.positionProvider.state === PositionProvider.WaitingToReconnect
         PropertyChanges {
-          message.image: root.providerType === "bluetooth" ? __style.externalBluetoothGreenImage : __style.externalNetworkGreenImage
+          message.image: __style.externalGpsRedImage
           message.title: root.providerType === "bluetooth"
             ? qsTr( "We were not able to connect to the specified device. Please make sure your device is powered on and can be connected to." )
-            : qsTr( "We were not able to connect to the specified IP address. Please try again later." )
+            : qsTr( "We were not able to connect to the specified IP address." )
           message.description: root.positionProvider.stateMessage + "<br><br>" + qsTr( "You can close this message, we will try to repeatedly connect to your device." )
           message.linkText: qsTr( "Learn more" )
         }

app/qml/gps/MMNetworkProviderDrawer.qml

@@ -110,14 +110,15 @@ MMComponents.MMDrawer {
         const deviceAddress = ip + ":" + port
 
         root.confirmed( aliasInput.text.trim(), deviceAddress )
-        root.close()
-        ipAddressInput.textField.clear()
-        portInput.textField.clear()
-        aliasInput.textField.clear()
       }
     }
   }
 
+  function showDuplicateProviderError() {
+    ipAddressInput.errorMsg = qsTr( "Network position provider with this IP address & port already exists" )
+    portInput.errorMsg = qsTr( "Network position provider with this IP address & port already exists" )
+  }
+
   onClosed: {
     ipAddressInput.textField.clear()
     portInput.textField.clear()

app/qml/gps/MMPositionProviderPage.qml

@@ -69,6 +69,7 @@ MMComponents.MMPage {
           if ( listdelegate.providerName ) return listdelegate.providerName
           return qsTr( "Unknown device" )
         }
+
         secondaryText: {
           if ( listdelegate.isActive ) {
             if ( listdelegate.providerType === "external_ip" )
@@ -99,12 +100,12 @@ MMComponents.MMPage {
       }
 
       section {
-        property: "providerType"
+        property: "providerGroup"
         delegate: MMComponents.MMText {
           required property string section
           width: ListView.view.width
 
-          text: section === "internal" ? qsTr( "Internal receivers" ) : qsTr( "External receivers" )
+          text: qsTr( "%1 receivers" ).arg( section === "internal" ? qsTr( "Internal" ) : qsTr( "External" ) )
 
           font: __style.p6
           color: __style.nightColor
@@ -154,7 +155,12 @@ MMComponents.MMPage {
       id: networkProviderDrawer
 
       onConfirmed: function( alias, deviceAddress ) {
-        root.activateProvider( "external_ip", deviceAddress, alias )
+        if ( providersModel.providerExists( deviceAddress ) ) {
+          showDuplicateProviderError()
+        } else {
+          close()
+          root.activateProvider( "external_ip", deviceAddress, alias )
+        }
       }
     }
 
@@ -268,8 +274,8 @@ MMComponents.MMPage {
       return // do not construct the same provider again
     }
 
-    providersModel.addProvider( name, id, type )
     PositionKit.positionProvider = PositionKit.constructProvider( type, id, name )
+    providersModel.addProvider( PositionKit.positionProvider.name(), id, type )
 
     if ( type === "external_bt" ) {
       connectingDialogLoader.open( "bluetooth" )