Address review (part1)

src/core/CMakeLists.txt

@@ -26,6 +26,7 @@ set(QFIELD_CORE_SRCS
     locator/gotolocatorfilter.cpp
     locator/helplocatorfilter.cpp
     locator/locatormodelsuperbridge.cpp
+    positioning/abstractgnssreceiver.cpp
     positioning/gnsspositioninformation.cpp
     positioning/internalgnssreceiver.cpp
     positioning/nmeagnssreceiver.cpp

src/core/positioning/abstractgnssreceiver.cpp

@@ -0,0 +1,32 @@
+#include "abstractgnssreceiver.h"
+
+AbstractGnssReceiver::AbstractGnssReceiver( QObject *parent )
+  : QObject( parent )
+{
+}
+
+QString AbstractGnssReceiver::socketStateString()
+{
+  switch ( mSocketState )
+  {
+    case QAbstractSocket::ConnectingState:
+    case QAbstractSocket::HostLookupState:
+      return tr( "Connecting…" );
+    case QAbstractSocket::ConnectedState:
+    case QAbstractSocket::BoundState:
+      return tr( "Successfully connected" );
+    case QAbstractSocket::UnconnectedState:
+      return tr( "Disconnected" );
+    default:
+      return tr( "Socket state %1" ).arg( static_cast<int>( socketState() ) );
+  }
+}
+
+void AbstractGnssReceiver::setSocketState( const QAbstractSocket::SocketState &state )
+{
+  if ( mSocketState == state )
+    return;
+  mSocketState = state;
+  emit socketStateChanged( mSocketState );
+  emit socketStateStringChanged( socketStateString() );
+}

src/core/positioning/abstractgnssreceiver.h

@@ -26,6 +26,8 @@ class AbstractGnssReceiver : public QObject
     Q_OBJECT
 
     Q_PROPERTY( GnssPositionInformation lastGnssPositionInformation READ lastGnssPositionInformation NOTIFY lastGnssPositionInformationChanged )
+    Q_PROPERTY( QAbstractSocket::SocketState socketState READ socketState WRITE setSocketState NOTIFY socketStateChanged )
+    Q_PROPERTY( QString socketStateString READ socketStateString NOTIFY socketStateStringChanged )
     Q_PROPERTY( QString lastError READ lastError NOTIFY lastErrorChanged )
 
   public:
@@ -38,8 +40,7 @@ class AbstractGnssReceiver : public QObject
     Q_DECLARE_FLAGS( Capabilities, Capability )
     Q_FLAGS( Capabilities )
 
-    explicit AbstractGnssReceiver( QObject *parent = nullptr )
-      : QObject( parent ) {}
+    explicit AbstractGnssReceiver( QObject *parent = nullptr );
     virtual ~AbstractGnssReceiver() = default;
 
     bool valid() const { return mValid; }
@@ -57,33 +58,20 @@ class AbstractGnssReceiver : public QObject
 
     Q_INVOKABLE virtual AbstractGnssReceiver::Capabilities capabilities() const { return NoCapabilities; }
 
-    virtual QList<QPair<QString, QVariant>> details() { return {}; }
+    virtual QList<QPair<QString, QVariant>> details() const { return {}; }
+    virtual QAbstractSocket::SocketState socketState() const { return mSocketState; }
+
 
   public slots:
-    virtual QAbstractSocket::SocketState socketState() { return QAbstractSocket::SocketState::UnconnectedState; }
-    virtual QString socketStateString()
-    {
-      switch ( socketState() )
-      {
-        case QAbstractSocket::ConnectingState:
-        case QAbstractSocket::HostLookupState:
-          return tr( "Connecting…" );
-        case QAbstractSocket::ConnectedState:
-        case QAbstractSocket::BoundState:
-          return tr( "Successfully connected" );
-        case QAbstractSocket::UnconnectedState:
-          return tr( "Disconnected" );
-        default:
-          return tr( "Socket state %1" ).arg( static_cast<int>( socketState() ) );
-      }
-    }
+    virtual QString socketStateString();
+    void setSocketState( const QAbstractSocket::SocketState &newSocketState );
 
   signals:
     void validChanged();
-    void lastGnssPositionInformationChanged( GnssPositionInformation &lastGnssPositionInformation );
+    void lastGnssPositionInformationChanged( const GnssPositionInformation &lastGnssPositionInformation );
     void socketStateChanged( const QAbstractSocket::SocketState socketState );
     void socketStateStringChanged( const QString &socketStateString );
-    void lastErrorChanged( QString &lastError );
+    void lastErrorChanged( const QString &lastError );
 
   private:
     friend class InternalGnssReceiver;
@@ -102,6 +90,7 @@ class AbstractGnssReceiver : public QObject
 
     bool mValid = false;
     GnssPositionInformation mLastGnssPositionInformation;
+    QAbstractSocket::SocketState mSocketState = QAbstractSocket::UnconnectedState;
     QString mLastError;
 };
 

src/core/positioning/bluetoothreceiver.cpp

@@ -27,8 +27,10 @@ BluetoothReceiver::BluetoothReceiver( const QString &address, QObject *parent )
   , mLocalDevice( std::make_unique<QBluetoothLocalDevice>() )
   , mSocket( new QBluetoothSocket( QBluetoothServiceInfo::RfcommProtocol ) )
 {
-  connect( mSocket, &QBluetoothSocket::stateChanged, this, &BluetoothReceiver::setSocketState );
   connect( mSocket, qOverload<QBluetoothSocket::SocketError>( &QBluetoothSocket::errorOccurred ), this, &BluetoothReceiver::handleError );
+  connect( mSocket, &QBluetoothSocket::stateChanged, this, [=]( QBluetoothSocket::SocketState state ) {
+    setSocketState( static_cast<QAbstractSocket::SocketState>( state ) );
+  } );
 
   connect( mLocalDevice.get(), &QBluetoothLocalDevice::pairingFinished, this, &BluetoothReceiver::pairingFinished );
   connect( mLocalDevice.get(), &QBluetoothLocalDevice::errorOccurred, [=]( QBluetoothLocalDevice::Error error ) {
@@ -57,8 +59,6 @@ BluetoothReceiver::BluetoothReceiver( const QString &address, QObject *parent )
 
 BluetoothReceiver::~BluetoothReceiver()
 {
-  disconnect( mSocket, &QBluetoothSocket::stateChanged, this, &BluetoothReceiver::setSocketState );
-
   mSocket->deleteLater();
   mSocket = nullptr;
 }
@@ -161,15 +161,13 @@ void BluetoothReceiver::doConnectDevice()
   repairDevice( QBluetoothAddress( mAddress ) );
 }
 
-void BluetoothReceiver::setSocketState( const QBluetoothSocket::SocketState socketState )
+QAbstractSocket::SocketState BluetoothReceiver::socketState() const
 {
-  emit socketStateChanged( static_cast<QAbstractSocket::SocketState>( socketState ) );
-  emit socketStateStringChanged( socketStateString() );
-}
-
-QAbstractSocket::SocketState BluetoothReceiver::socketState()
-{
-  return mSocket ? static_cast<QAbstractSocket::SocketState>( mSocket->state() ) : QAbstractSocket::UnconnectedState;
+  if ( !mSocket )
+  {
+    return QAbstractSocket::UnconnectedState;
+  }
+  return static_cast<QAbstractSocket::SocketState>( mSocket->state() );
 }
 
 QString BluetoothReceiver::socketStateString()
@@ -184,12 +182,12 @@ QString BluetoothReceiver::socketStateString()
       return tr( "Successfully connected" );
     case QAbstractSocket::UnconnectedState:
     {
-      QString mSocketStateString = tr( "Disconnected" );
+      QString socketStateString = tr( "Disconnected" );
       if ( !mDisconnecting && mSocket->error() != QBluetoothSocket::SocketError::NoSocketError )
-        mSocketStateString.append( QStringLiteral( ": %1" ).arg( mSocket->errorString() ) );
+        socketStateString.append( QStringLiteral( ": %1" ).arg( mSocket->errorString() ) );
       if ( mConnectOnDisconnect )
         doConnectDevice();
-      return mSocketStateString;
+      return socketStateString;
     }
     default:
       return tr( "Socket state %1" ).arg( static_cast<int>( currentState ) );

src/core/positioning/bluetoothreceiver.h

@@ -33,9 +33,9 @@ class BluetoothReceiver : public NmeaGnssReceiver
   public:
     explicit BluetoothReceiver( const QString &address = QString(), QObject *parent = nullptr );
     ~BluetoothReceiver() override;
+    QAbstractSocket::SocketState socketState() const override;
 
   public slots:
-    QAbstractSocket::SocketState socketState() override;
     QString socketStateString() override;
 
   private slots:
@@ -46,8 +46,6 @@ class BluetoothReceiver : public NmeaGnssReceiver
      */
     void pairingFinished( const QBluetoothAddress &address, QBluetoothLocalDevice::Pairing status );
 
-    void setSocketState( const QBluetoothSocket::SocketState socketState );
-
   private:
     void handleConnectDevice() override;
     void handleDisconnectDevice() override;

src/core/positioning/egenioussreceiver.cpp

@@ -9,13 +9,17 @@ EgenioussReceiver::EgenioussReceiver( QObject *parent )
 {
   connect( mTcpSocket, &QTcpSocket::readyRead, this, &EgenioussReceiver::onReadyRead );
   connect( mTcpSocket, &QTcpSocket::errorOccurred, this, &EgenioussReceiver::handleError );
-  connect( mTcpSocket, &QTcpSocket::stateChanged, this, &EgenioussReceiver::setSocketState );
+  connect( mTcpSocket, &QTcpSocket::stateChanged, this, [=]( QAbstractSocket::SocketState state ) {
+    setSocketState( state );
+  } );
+
   setValid( true );
 }
 
 EgenioussReceiver::~EgenioussReceiver()
 {
-  disconnect( mTcpSocket, &QTcpSocket::stateChanged, this, &EgenioussReceiver::setSocketState );
+  mTcpSocket->deleteLater();
+  mTcpSocket = nullptr;
 }
 
 void EgenioussReceiver::handleConnectDevice()
@@ -28,18 +32,16 @@ void EgenioussReceiver::handleDisconnectDevice()
   mTcpSocket->disconnectFromHost();
 }
 
-QAbstractSocket::SocketState EgenioussReceiver::socketState()
-{
-  return mTcpSocket ? mTcpSocket->state() : QAbstractSocket::UnconnectedState;
-}
-
-void EgenioussReceiver::setSocketState( const QAbstractSocket::SocketState socketState )
+QAbstractSocket::SocketState EgenioussReceiver::socketState() const
 {
-  emit socketStateChanged( socketState );
-  emit socketStateStringChanged( socketStateString() );
+  if ( mTcpSocket == nullptr )
+  {
+    return QAbstractSocket::UnconnectedState;
+  }
+  return mTcpSocket->state();
 }
 
-QList<QPair<QString, QVariant>> EgenioussReceiver::details()
+QList<QPair<QString, QVariant>> EgenioussReceiver::details() const
 {
   QList<QPair<QString, QVariant>> detailsList;
 
@@ -130,8 +132,6 @@ void EgenioussReceiver::handleError( QAbstractSocket::SocketError error )
       mLastError = tr( "TCP receiver error (%1)" ).arg( QMetaEnum::fromType<QAbstractSocket::SocketError>().valueToKey( error ) );
       break;
   }
-  setSocketState( QAbstractSocket::UnconnectedState );
-
   qInfo() << QStringLiteral( "EgenioussReceiver: Error: %1" ).arg( mLastError );
 
   emit lastErrorChanged( mLastError );

src/core/positioning/egenioussreceiver.h

@@ -14,18 +14,16 @@ class EgenioussReceiver : public AbstractGnssReceiver
     explicit EgenioussReceiver( QObject *parent = nullptr );
     ~EgenioussReceiver();
 
-  public slots:
-    QAbstractSocket::SocketState socketState() override;
+    QList<QPair<QString, QVariant>> details() const override;
+    QAbstractSocket::SocketState socketState() const override;
 
   private:
     void handleConnectDevice() override;
     void handleDisconnectDevice() override;
-    QList<QPair<QString, QVariant>> details() override;
 
   private slots:
     void onReadyRead();
     void handleError( QAbstractSocket::SocketError error );
-    void setSocketState( const QAbstractSocket::SocketState socketState );
 
   private:
     void processReceivedData();

src/core/positioning/internalgnssreceiver.cpp

@@ -35,7 +35,7 @@ InternalGnssReceiver::InternalGnssReceiver( QObject *parent )
     connect( mGeoPositionSource.get(), &QGeoPositionInfoSource::positionUpdated, this, &InternalGnssReceiver::handlePositionUpdated );
     connect( mGeoPositionSource.get(), qOverload<QGeoPositionInfoSource::Error>( &QGeoPositionInfoSource::errorOccurred ), this, &InternalGnssReceiver::handleError );
 
-    mSocketState = QAbstractSocket::ConnectedState;
+    setSocketState( QAbstractSocket::ConnectedState );
 
     setValid( true );
   }
@@ -52,12 +52,6 @@ InternalGnssReceiver::InternalGnssReceiver( QObject *parent )
   connect( QgsApplication::instance(), &QGuiApplication::applicationStateChanged, this, &InternalGnssReceiver::onApplicationStateChanged );
 }
 
-
-QAbstractSocket::SocketState InternalGnssReceiver::socketState()
-{
-  return mSocketState;
-}
-
 void InternalGnssReceiver::onApplicationStateChanged( Qt::ApplicationState state )
 {
 #ifdef Q_OS_ANDROID

src/core/positioning/internalgnssreceiver.h

@@ -31,9 +31,6 @@ class InternalGnssReceiver : public AbstractGnssReceiver
     explicit InternalGnssReceiver( QObject *parent = nullptr );
     ~InternalGnssReceiver() override = default;
 
-  public slots:
-    QAbstractSocket::SocketState socketState() override;
-
   private slots:
 
     void onApplicationStateChanged( Qt::ApplicationState state );
@@ -60,7 +57,6 @@ class InternalGnssReceiver : public AbstractGnssReceiver
     QList<int> mSatellitesID;
     QList<QgsSatelliteInfo> mSatellitesInfo;
     bool mSatelliteInformationValid = true;
-    QAbstractSocket::SocketState mSocketState = QAbstractSocket::UnconnectedState;
 };
 
 #endif // INTERNALGNSSRECEIVER_H

src/core/positioning/nmeagnssreceiver.cpp

@@ -116,7 +116,7 @@ void NmeaGnssReceiver::handleStopLogging()
   mLogFile.close();
 }
 
-QList<QPair<QString, QVariant>> NmeaGnssReceiver::details()
+QList<QPair<QString, QVariant>> NmeaGnssReceiver::details() const
 {
   QList<QPair<QString, QVariant>> dataList;
 

src/core/positioning/nmeagnssreceiver.h

@@ -50,7 +50,7 @@ class NmeaGnssReceiver : public AbstractGnssReceiver
   private:
     void handleStartLogging() override;
     void handleStopLogging() override;
-    QList<QPair<QString, QVariant>> details() override;
+    QList<QPair<QString, QVariant>> details() const override;
 
     void processImuSentence( const QString &sentence );
 

src/core/positioning/serialportreceiver.cpp

@@ -19,34 +19,27 @@
 SerialPortReceiver::SerialPortReceiver( const QString &address, QObject *parent )
   : NmeaGnssReceiver( parent )
   , mAddress( address )
-  , mSocket( new QSerialPort() )
+  , mSerialPort( new QSerialPort() )
 {
-  connect( mSocket, qOverload<QSerialPort::SerialPortError>( &QSerialPort::errorOccurred ), this, &SerialPortReceiver::handleError );
+  connect( mSerialPort, qOverload<QSerialPort::SerialPortError>( &QSerialPort::errorOccurred ), this, &SerialPortReceiver::handleError );
 
-  initNmeaConnection( mSocket );
+  initNmeaConnection( mSerialPort );
 
   setValid( !mAddress.isEmpty() );
 }
 
 SerialPortReceiver::~SerialPortReceiver()
 {
-  mSocket->deleteLater();
-  mSocket = nullptr;
-}
-
-QAbstractSocket::SocketState SerialPortReceiver::socketState()
-{
-  return mSocketState;
+  mSerialPort->deleteLater();
+  mSerialPort = nullptr;
 }
 
 void SerialPortReceiver::handleDisconnectDevice()
 {
-  if ( mSocketState == QAbstractSocket::ConnectedState )
+  if ( socketState() == QAbstractSocket::ConnectedState )
   {
-    mSocket->close();
-    mSocketState = QAbstractSocket::UnconnectedState;
-    emit socketStateChanged( mSocketState );
-    emit socketStateStringChanged( socketStateString() );
+    mSerialPort->close();
+    setSocketState( QAbstractSocket::UnconnectedState );
   }
 }
 
@@ -58,13 +51,11 @@ void SerialPortReceiver::handleConnectDevice()
   }
   qInfo() << "SerialPortReceiver: Initiating connection to port name: " << mAddress;
 
-  mSocket->setPortName( mAddress );
-  mSocket->setBaudRate( QSerialPort::Baud9600 );
-  if ( mSocket->open( QIODevice::ReadOnly ) )
+  mSerialPort->setPortName( mAddress );
+  mSerialPort->setBaudRate( QSerialPort::Baud9600 );
+  if ( mSerialPort->open( QIODevice::ReadOnly ) )
   {
-    mSocketState = QAbstractSocket::ConnectedState;
-    emit socketStateChanged( mSocketState );
-    emit socketStateStringChanged( socketStateString() );
+    setSocketState( QAbstractSocket::ConnectedState );
   }
 }