diff --git a/python/PyQt6/core/auto_generated/gps/qgsgpsdetector.sip.in b/python/PyQt6/core/auto_generated/gps/qgsgpsdetector.sip.in index 50011ec27f8..3f4627084ba 100644 --- a/python/PyQt6/core/auto_generated/gps/qgsgpsdetector.sip.in +++ b/python/PyQt6/core/auto_generated/gps/qgsgpsdetector.sip.in @@ -23,11 +23,41 @@ Class to detect the GPS port #include "qgsgpsdetector.h" %End public: - QgsGpsDetector( const QString &portName ); + + + QgsGpsDetector( const QString &portName = QString(), bool useUnsafeSignals = true ); +%Docstring +Constructor for QgsGpsDetector. + +If ``portName`` is specified, then only devices from the given port will be scanned. Otherwise +all connection types will be attempted (including internal GPS devices). + +Since QGIS 3.38, the ``useUnsafeSignals`` parameter can be set to ``False`` to avoid emitting the +dangerous and fragile :py:func:`~QgsGpsDetector.detected` signal. This is highly recommended, but is opt-in to avoid +breaking stable QGIS 3.x API. If ``useUnsafeSignals`` is set to ``False``, only the safe :py:func:`~QgsGpsDetector.connectionDetected` signal +will be emitted and clients must manually take ownership of the detected connection via a call +to :py:func:`~QgsGpsDetector.takeConnection`. +%End ~QgsGpsDetector(); + QgsGpsConnection *takeConnection() /TransferBack/; +%Docstring +Returns the detected GPS connection, and removes it from the detector. + +The caller takes ownership of the connection. Only the first call to this +method following a :py:func:`~QgsGpsDetector.connectionDetected` signal will be able to retrieve the +detected connection -- subsequent calls will return ``None``. + +.. warning:: + + Do not call this method if the useUnsafeSignals option in the + QgsGpsDetector constructor was set to ``True``. + +.. versionadded:: 3.38 +%End + static QList< QPair > availablePorts(); public slots: @@ -37,14 +67,28 @@ Class to detect the GPS port signals: + void connectionDetected(); +%Docstring +Emitted when a GPS connection is successfully detected. - void detected( QgsGpsConnection *connection ); +Call :py:func:`~QgsGpsDetector.takeConnection` to take ownership of the detected connection. + +.. versionadded:: 3.38 +%End + + void detected( QgsGpsConnection *connection ) /Deprecated/; %Docstring Emitted when the GPS connection has been detected. A single connection must listen for this signal and immediately take ownership of the ``connection`` object. + +.. deprecated:: + This signal is dangerous and extremely unsafe! It is recommended to instead set the ``useUnsafeSignals`` parameter to ``False`` in the QgsGpsDetector constructor and use the safe :py:func:`~QgsGpsDetector.connectionDetected` signal instead. %End void detectionFailed(); +%Docstring +Emitted when the detector could not find a valid GPS connection. +%End }; diff --git a/python/core/auto_generated/gps/qgsgpsdetector.sip.in b/python/core/auto_generated/gps/qgsgpsdetector.sip.in index 50011ec27f8..3f4627084ba 100644 --- a/python/core/auto_generated/gps/qgsgpsdetector.sip.in +++ b/python/core/auto_generated/gps/qgsgpsdetector.sip.in @@ -23,11 +23,41 @@ Class to detect the GPS port #include "qgsgpsdetector.h" %End public: - QgsGpsDetector( const QString &portName ); + + + QgsGpsDetector( const QString &portName = QString(), bool useUnsafeSignals = true ); +%Docstring +Constructor for QgsGpsDetector. + +If ``portName`` is specified, then only devices from the given port will be scanned. Otherwise +all connection types will be attempted (including internal GPS devices). + +Since QGIS 3.38, the ``useUnsafeSignals`` parameter can be set to ``False`` to avoid emitting the +dangerous and fragile :py:func:`~QgsGpsDetector.detected` signal. This is highly recommended, but is opt-in to avoid +breaking stable QGIS 3.x API. If ``useUnsafeSignals`` is set to ``False``, only the safe :py:func:`~QgsGpsDetector.connectionDetected` signal +will be emitted and clients must manually take ownership of the detected connection via a call +to :py:func:`~QgsGpsDetector.takeConnection`. +%End ~QgsGpsDetector(); + QgsGpsConnection *takeConnection() /TransferBack/; +%Docstring +Returns the detected GPS connection, and removes it from the detector. + +The caller takes ownership of the connection. Only the first call to this +method following a :py:func:`~QgsGpsDetector.connectionDetected` signal will be able to retrieve the +detected connection -- subsequent calls will return ``None``. + +.. warning:: + + Do not call this method if the useUnsafeSignals option in the + QgsGpsDetector constructor was set to ``True``. + +.. versionadded:: 3.38 +%End + static QList< QPair > availablePorts(); public slots: @@ -37,14 +67,28 @@ Class to detect the GPS port signals: + void connectionDetected(); +%Docstring +Emitted when a GPS connection is successfully detected. - void detected( QgsGpsConnection *connection ); +Call :py:func:`~QgsGpsDetector.takeConnection` to take ownership of the detected connection. + +.. versionadded:: 3.38 +%End + + void detected( QgsGpsConnection *connection ) /Deprecated/; %Docstring Emitted when the GPS connection has been detected. A single connection must listen for this signal and immediately take ownership of the ``connection`` object. + +.. deprecated:: + This signal is dangerous and extremely unsafe! It is recommended to instead set the ``useUnsafeSignals`` parameter to ``False`` in the QgsGpsDetector constructor and use the safe :py:func:`~QgsGpsDetector.connectionDetected` signal instead. %End void detectionFailed(); +%Docstring +Emitted when the detector could not find a valid GPS connection. +%End }; diff --git a/src/app/gps/qgsappgpsconnection.cpp b/src/app/gps/qgsappgpsconnection.cpp index f88b5b83fef..ae8986dcd7a 100644 --- a/src/app/gps/qgsappgpsconnection.cpp +++ b/src/app/gps/qgsappgpsconnection.cpp @@ -57,7 +57,7 @@ void QgsAppGpsConnection::setConnection( QgsGpsConnection *connection ) disconnectGps(); } - onConnected( connection ); + setConnectionPrivate( connection ); } QgsPoint QgsAppGpsConnection::lastValidLocation() const @@ -156,10 +156,11 @@ void QgsAppGpsConnection::connectGps() QgsMessageLog::logMessage( QObject::tr( "Firing up GPS detector" ), QObject::tr( "GPS" ), Qgis::Info ); - QgsGpsDetector *detector = new QgsGpsDetector( port ); - connect( detector, static_cast < void ( QgsGpsDetector::* )( QgsGpsConnection * ) > ( &QgsGpsDetector::detected ), this, &QgsAppGpsConnection::onConnected ); - connect( detector, &QgsGpsDetector::detectionFailed, this, &QgsAppGpsConnection::onTimeOut ); - detector->advance(); // start the detection process + // note -- QgsGpsDetector internally uses deleteLater to clean itself up! + mDetector = new QgsGpsDetector( port, false ); + connect( mDetector, &QgsGpsDetector::connectionDetected, this, &QgsAppGpsConnection::onConnectionDetected ); + connect( mDetector, &QgsGpsDetector::detectionFailed, this, &QgsAppGpsConnection::onTimeOut ); + mDetector->advance(); // start the detection process } void QgsAppGpsConnection::disconnectGps() @@ -181,6 +182,9 @@ void QgsAppGpsConnection::disconnectGps() void QgsAppGpsConnection::onTimeOut() { + if ( sender() != mDetector ) + return; + QgsMessageLog::logMessage( QObject::tr( "GPS detector reported timeout" ), QObject::tr( "GPS" ), Qgis::Critical ); disconnectGps(); emit connectionTimedOut(); @@ -189,11 +193,18 @@ void QgsAppGpsConnection::onTimeOut() showGpsConnectFailureWarning( tr( "TIMEOUT - Failed to connect to GPS device." ) ); } -void QgsAppGpsConnection::onConnected( QgsGpsConnection *conn ) +void QgsAppGpsConnection::onConnectionDetected() { - QgsMessageLog::logMessage( QObject::tr( "GPS detector GOT a connection" ), QObject::tr( "GPS" ), Qgis::Info ); + if ( sender() != mDetector ) + return; - mConnection = conn; + QgsMessageLog::logMessage( QObject::tr( "GPS detector GOT a connection" ), QObject::tr( "GPS" ), Qgis::Info ); + setConnectionPrivate( mDetector->takeConnection() ); +} + +void QgsAppGpsConnection::setConnectionPrivate( QgsGpsConnection *connection ) +{ + mConnection = connection; connect( mConnection, &QgsGpsConnection::stateChanged, this, &QgsAppGpsConnection::stateChanged ); connect( mConnection, &QgsGpsConnection::nmeaSentenceReceived, this, &QgsAppGpsConnection::nmeaSentenceReceived ); connect( mConnection, &QgsGpsConnection::fixStatusChanged, this, &QgsAppGpsConnection::fixStatusChanged ); diff --git a/src/app/gps/qgsappgpsconnection.h b/src/app/gps/qgsappgpsconnection.h index 9c82af42853..7b992c8cfeb 100644 --- a/src/app/gps/qgsappgpsconnection.h +++ b/src/app/gps/qgsappgpsconnection.h @@ -25,6 +25,7 @@ class QgsGpsConnection; class QgsGpsInformation; class QgsPoint; class QgsMessageBarItem; +class QgsGpsDetector; /** * Manages a single "canonical" GPS connection for use in the QGIS app, eg for displaying GPS @@ -140,8 +141,9 @@ class APP_EXPORT QgsAppGpsConnection : public QObject private slots: void onTimeOut(); + void onConnectionDetected(); - void onConnected( QgsGpsConnection *conn ); + void setConnectionPrivate( QgsGpsConnection *connection ); private: @@ -150,6 +152,7 @@ class APP_EXPORT QgsAppGpsConnection : public QObject void showGpsConnectFailureWarning( const QString &message ); void showMessage( Qgis::MessageLevel level, const QString &message ); + QPointer< QgsGpsDetector > mDetector; QgsGpsConnection *mConnection = nullptr; QPointer< QgsMessageBarItem > mConnectionMessageItem; }; diff --git a/src/core/gps/qgsgpsdetector.cpp b/src/core/gps/qgsgpsdetector.cpp index 5f9b594675b..2691905d4ad 100644 --- a/src/core/gps/qgsgpsdetector.cpp +++ b/src/core/gps/qgsgpsdetector.cpp @@ -64,7 +64,8 @@ QList< QPair > QgsGpsDetector::availablePorts() return devs; } -QgsGpsDetector::QgsGpsDetector( const QString &portName ) +QgsGpsDetector::QgsGpsDetector( const QString &portName, bool useUnsafeSignals ) + : mUseUnsafeSignals( useUnsafeSignals ) { #if defined( HAVE_QTSERIALPORT ) mBaudList << QSerialPort::Baud4800 << QSerialPort::Baud9600 << QSerialPort::Baud38400 << QSerialPort::Baud57600 << QSerialPort::Baud115200; //add 57600 for SXBlueII GPS unit @@ -87,11 +88,21 @@ QgsGpsDetector::~QgsGpsDetector() QgsMessageLog::logMessage( QObject::tr( "Destroying GPS detector" ), QObject::tr( "GPS" ), Qgis::Info ); } +QgsGpsConnection *QgsGpsDetector::takeConnection() +{ + if ( mUseUnsafeSignals ) + { + QgsDebugError( QStringLiteral( "QgsGpsDetector::takeConnection() incorrectly called when useUnsafeSignals option is in effect" ) ); + return nullptr; + } + + return mConn.release(); +} + void QgsGpsDetector::advance() { if ( mConn ) { - disconnect( mConn.get(), &QObject::destroyed, this, &QgsGpsDetector::connDestroyed ); mConn.reset(); } @@ -170,22 +181,22 @@ void QgsGpsDetector::advance() QgsMessageLog::logMessage( QObject::tr( "Have a connection, now listening for messages" ), QObject::tr( "GPS" ), Qgis::Info ); connect( mConn.get(), &QgsGpsConnection::stateChanged, this, static_cast < void ( QgsGpsDetector::* )( const QgsGpsInformation & ) >( &QgsGpsDetector::detected ) ); - connect( mConn.get(), &QObject::destroyed, this, &QgsGpsDetector::connDestroyed ); + if ( mUseUnsafeSignals ) + { + connect( mConn.get(), &QObject::destroyed, this, &QgsGpsDetector::connDestroyed ); + } // leave 2s to pickup a valid string QTimer::singleShot( 2000, this, &QgsGpsDetector::connectionTimeout ); } -void QgsGpsDetector::detected( const QgsGpsInformation &info ) +void QgsGpsDetector::detected( const QgsGpsInformation & ) { - Q_UNUSED( info ) - QgsMessageLog::logMessage( QObject::tr( "Detected information" ), QObject::tr( "GPS" ), Qgis::Info ); if ( !mConn ) { // advance if connection was destroyed - QgsMessageLog::logMessage( QObject::tr( "Got information, but CONNECTION WAS DESTROYED EXTERNALLY!" ), QObject::tr( "GPS" ), Qgis::Critical ); advance(); } @@ -194,8 +205,17 @@ void QgsGpsDetector::detected( const QgsGpsInformation &info ) // signal detected QgsMessageLog::logMessage( QObject::tr( "Connection status IS GPSDataReceived" ), QObject::tr( "GPS" ), Qgis::Info ); - // let's hope there's a single, unique connection to this signal... otherwise... boom - emit detected( mConn.release() ); + if ( mUseUnsafeSignals ) + { + // let's hope there's a single, unique connection to this signal... otherwise... boom! + Q_NOWARN_DEPRECATED_PUSH + emit detected( mConn.release() ); + Q_NOWARN_DEPRECATED_POP + } + else + { + emit connectionDetected(); + } deleteLater(); } diff --git a/src/core/gps/qgsgpsdetector.h b/src/core/gps/qgsgpsdetector.h index 266fb0fdf54..e287ccf658c 100644 --- a/src/core/gps/qgsgpsdetector.h +++ b/src/core/gps/qgsgpsdetector.h @@ -47,7 +47,22 @@ class CORE_EXPORT QgsGpsDetector : public QObject { Q_OBJECT public: - QgsGpsDetector( const QString &portName ); + + // TODO QGIS 4.0 -- remove useUnsafeSignals option + + /** + * Constructor for QgsGpsDetector. + * + * If \a portName is specified, then only devices from the given port will be scanned. Otherwise + * all connection types will be attempted (including internal GPS devices). + * + * Since QGIS 3.38, the \a useUnsafeSignals parameter can be set to FALSE to avoid emitting the + * dangerous and fragile detected() signal. This is highly recommended, but is opt-in to avoid + * breaking stable QGIS 3.x API. If \a useUnsafeSignals is set to FALSE, only the safe connectionDetected() signal + * will be emitted and clients must manually take ownership of the detected connection via a call + * to takeConnection(). + */ + QgsGpsDetector( const QString &portName = QString(), bool useUnsafeSignals = true ); #if defined( HAVE_QTSERIALPORT ) static const QgsSettingsEntryEnumFlag *settingsGpsStopBits SIP_SKIP; @@ -58,6 +73,20 @@ class CORE_EXPORT QgsGpsDetector : public QObject ~QgsGpsDetector() override; + /** + * Returns the detected GPS connection, and removes it from the detector. + * + * The caller takes ownership of the connection. Only the first call to this + * method following a connectionDetected() signal will be able to retrieve the + * detected connection -- subsequent calls will return NULLPTR. + * + * \warning Do not call this method if the useUnsafeSignals option in the + * QgsGpsDetector constructor was set to TRUE. + * + * \since QGIS 3.38 + */ + QgsGpsConnection *takeConnection() SIP_TRANSFERBACK; + static QList< QPair > availablePorts(); public slots: @@ -67,15 +96,26 @@ class CORE_EXPORT QgsGpsDetector : public QObject signals: - // TODO QGIS 4.0 - this is horrible, fragile, leaky and crash prone API. - // don't transfer ownership with this signal, and add an explicit takeConnection member! + /** + * Emitted when a GPS connection is successfully detected. + * + * Call takeConnection() to take ownership of the detected connection. + * + * \since QGIS 3.38 + */ + void connectionDetected(); /** * Emitted when the GPS connection has been detected. A single connection must listen for this signal and * immediately take ownership of the \a connection object. + * + * \deprecated This signal is dangerous and extremely unsafe! It is recommended to instead set the \a useUnsafeSignals parameter to FALSE in the QgsGpsDetector constructor and use the safe connectionDetected() signal instead. */ - void detected( QgsGpsConnection *connection ); + Q_DECL_DEPRECATED void detected( QgsGpsConnection *connection ) SIP_DEPRECATED; + /** + * Emitted when the detector could not find a valid GPS connection. + */ void detectionFailed(); private slots: @@ -83,6 +123,7 @@ class CORE_EXPORT QgsGpsDetector : public QObject void connectionTimeout(); private: + bool mUseUnsafeSignals = true; int mPortIndex = 0; int mBaudIndex = -1; QList< QPair< QString, QString > > mPortList;