From 9ae7040a91c3e3e0606d7abe085ef6da47efd6d2 Mon Sep 17 00:00:00 2001 From: Dirk Hohndel Date: Fri, 27 Sep 2019 16:26:54 -0700 Subject: Revert the singleton PR It turns out that this isn't working the way it was intended to. Signed-off-by: Dirk Hohndel --- core/btdiscovery.cpp | 14 ++++++++ core/btdiscovery.h | 5 +-- core/gpslocation.cpp | 21 ++++++++++++ core/gpslocation.h | 4 +++ core/singleton.h | 74 ------------------------------------------ desktop-widgets/mainwindow.cpp | 10 ++++++ desktop-widgets/mainwindow.h | 5 +-- mobile-widgets/qmlmanager.cpp | 10 +++++- mobile-widgets/qmlmanager.h | 5 +-- mobile-widgets/qmlprefs.cpp | 17 ++++++++++ mobile-widgets/qmlprefs.h | 8 +++-- qt-models/divelistmodel.cpp | 8 +++++ qt-models/divelistmodel.h | 6 ++-- qt-models/gpslistmodel.cpp | 12 ++++++- qt-models/gpslistmodel.h | 8 +++-- 15 files changed, 118 insertions(+), 89 deletions(-) delete mode 100644 core/singleton.h diff --git a/core/btdiscovery.cpp b/core/btdiscovery.cpp index 1387be400..8db1e02cc 100644 --- a/core/btdiscovery.cpp +++ b/core/btdiscovery.cpp @@ -15,6 +15,7 @@ extern QMap descriptorLookup; namespace { QHash btDeviceInfo; } +BTDiscovery *BTDiscovery::m_instance = NULL; static dc_descriptor_t *getDeviceType(QString btName) // central function to convert a BT name to a Subsurface known vendor/model pair @@ -138,6 +139,11 @@ BTDiscovery::BTDiscovery(QObject*) : m_btValid(false), m_showNonDiveComputers(false), discoveryAgent(nullptr) { + if (m_instance) { + qDebug() << "trying to create an additional BTDiscovery object"; + return; + } + m_instance = this; #if defined(BT_SUPPORT) QLoggingCategory::setFilterRules(QStringLiteral("qt.bluetooth* = true")); BTDiscoveryReDiscover(); @@ -196,11 +202,19 @@ void BTDiscovery::BTDiscoveryReDiscover() BTDiscovery::~BTDiscovery() { + m_instance = NULL; #if defined(BT_SUPPORT) delete discoveryAgent; #endif } +BTDiscovery *BTDiscovery::instance() +{ + if (!m_instance) + m_instance = new BTDiscovery(); + return m_instance; +} + #if defined(BT_SUPPORT) extern void addBtUuid(QBluetoothUuid uuid); extern QHash productList; diff --git a/core/btdiscovery.h b/core/btdiscovery.h index a7d68decd..d6c25d8b0 100644 --- a/core/btdiscovery.h +++ b/core/btdiscovery.h @@ -10,7 +10,6 @@ #include #include #include "core/libdivecomputer.h" -#include "core/singleton.h" #if defined(Q_OS_ANDROID) #include @@ -24,12 +23,13 @@ QString extractBluetoothAddress(const QString &address); QString extractBluetoothNameAddress(const QString &address, QString &name); QBluetoothDeviceInfo getBtDeviceInfo(const QString &devaddr); -class BTDiscovery : public QObject, public SillySingleton { +class BTDiscovery : public QObject { Q_OBJECT public: BTDiscovery(QObject *parent = NULL); ~BTDiscovery(); + static BTDiscovery *instance(); struct btPairedDevice { QString address; @@ -57,6 +57,7 @@ public: void discoverAddress(QString address); private: + static BTDiscovery *m_instance; bool m_btValid; bool m_showNonDiveComputers; diff --git a/core/gpslocation.cpp b/core/gpslocation.cpp index e18e46321..c5b81e024 100644 --- a/core/gpslocation.cpp +++ b/core/gpslocation.cpp @@ -15,12 +15,16 @@ #include #include +GpsLocation *GpsLocation::m_Instance = NULL; + GpsLocation::GpsLocation(void (*showMsgCB)(const char *), QObject *parent) : QObject(parent), m_GpsSource(0), waitingForPosition(false), haveSource(UNKNOWN) { + Q_ASSERT_X(m_Instance == NULL, "GpsLocation", "GpsLocation recreated"); + m_Instance = this; showMessageCB = showMsgCB; // create a QSettings object that's separate from the main application settings geoSettings = new QSettings(QSettings::NativeFormat, QSettings::UserScope, @@ -33,6 +37,23 @@ GpsLocation::GpsLocation(void (*showMsgCB)(const char *), QObject *parent) : connect(qPrefLocationService::instance(), SIGNAL(time_thresholdChanged(int)), this, SLOT(setGpsTimeThreshold(int))); } +GpsLocation *GpsLocation::instance() +{ + Q_ASSERT(m_Instance != NULL); + + return m_Instance; +} + +bool GpsLocation::hasInstance() +{ + return m_Instance != NULL; +} + +GpsLocation::~GpsLocation() +{ + m_Instance = NULL; +} + void GpsLocation::setGpsTimeThreshold(int seconds) { if (m_GpsSource) { diff --git a/core/gpslocation.h b/core/gpslocation.h index 60e16e186..02377bfca 100644 --- a/core/gpslocation.h +++ b/core/gpslocation.h @@ -24,6 +24,9 @@ class GpsLocation : public QObject { Q_OBJECT public: GpsLocation(void (*showMsgCB)(const char *msg), QObject *parent); + ~GpsLocation(); + static GpsLocation *instance(); + static bool hasInstance(); bool applyLocations(); int getGpsNum() const; bool hasLocationsSource(); @@ -40,6 +43,7 @@ private: QNetworkReply *reply; QString userAgent; void (*showMessageCB)(const char *msg); + static GpsLocation *m_Instance; bool waitingForPosition; QMap m_trackers; QList m_deletedTrackers; diff --git a/core/singleton.h b/core/singleton.h deleted file mode 100644 index 9573b07e8..000000000 --- a/core/singleton.h +++ /dev/null @@ -1,74 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0 -/* - * We use singletons in numerous places. In combination with QML this gives - * us a very fundamental problem, because QML likes to allocate objects by - * itself. There are known workarounds, but currently we use idiosyncratic - * singleton classes, which initialize the global instance pointer in the - * constructor. Things get even more complicated if the same singleton is used - * on mobile and on desktop. The latter might want to simply use the classical - * instance() function without having to initialize the singleton first, as - * that would beat the purpose of the instance() method. - * - * The template defined here, SillySingleton, codifies all this. Simply derive - * a class from this template: - * class X : public SillySingleton { - * ... - * }; - * This will generate an instance() method. This will do the right thing for - * both methods: explicit construction of the singleton via new or implicit - * by calling the instance() method. It will also generate warnings if a - * singleton class is generated more than once (i.e. first instance() is called - * and _then_ new). - * - * In the long run we should get rid of all users of this class. - */ -#ifndef SINGLETON_H -#define SINGLETON_H - -#include -#include - -// 1) Declaration -template -class SillySingleton { - static T *self; -protected: - SillySingleton(); - ~SillySingleton(); -public: - static T *instance(); -}; - -template -T *SillySingleton::self = nullptr; - -// 2) Implementation - -template -SillySingleton::SillySingleton() -{ - if (self) - qWarning("Generating second instance of singleton %s", typeid(T).name()); - self = static_cast(this); - qDebug("Generated singleton %s", typeid(T).name()); -} - -template -SillySingleton::~SillySingleton() -{ - if (self == this) - self = nullptr; - else - qWarning("Destroying unknown instance of singleton %s", typeid(T).name()); - qDebug("Destroyed singleton %s", typeid(T).name()); -} - -template -T *SillySingleton::instance() -{ - if (!self) - new T; - return self; -} - -#endif diff --git a/desktop-widgets/mainwindow.cpp b/desktop-widgets/mainwindow.cpp index ef55b3d06..bed142ecc 100644 --- a/desktop-widgets/mainwindow.cpp +++ b/desktop-widgets/mainwindow.cpp @@ -109,6 +109,8 @@ extern "C" int updateProgress(const char *text) return progressDialogCanceled; } +MainWindow *MainWindow::m_Instance = nullptr; + extern "C" void showErrorFromC(char *buf) { QString error(buf); @@ -126,6 +128,8 @@ MainWindow::MainWindow() : QMainWindow(), survey(nullptr), findMovedImagesDialog(nullptr) { + Q_ASSERT_X(m_Instance == NULL, "MainWindow", "MainWindow recreated!"); + m_Instance = this; ui.setupUi(this); read_hashes(); Command::init(); @@ -353,6 +357,7 @@ MainWindow::MainWindow() : QMainWindow(), MainWindow::~MainWindow() { write_hashes(); + m_Instance = nullptr; } void MainWindow::setupSocialNetworkMenu() @@ -393,6 +398,11 @@ void MainWindow::setDefaultState() ui.bottomLeft->currentWidget()->setEnabled(false); } +MainWindow *MainWindow::instance() +{ + return m_Instance; +} + // This gets called after one or more dives were added, edited or downloaded for a dive computer void MainWindow::refreshDisplay(bool doRecreateDiveList) { diff --git a/desktop-widgets/mainwindow.h b/desktop-widgets/mainwindow.h index 4b91b0dc3..62cc1b6f1 100644 --- a/desktop-widgets/mainwindow.h +++ b/desktop-widgets/mainwindow.h @@ -21,7 +21,6 @@ #include "desktop-widgets/filterwidget2.h" #include "core/applicationstate.h" #include "core/gpslocation.h" -#include "core/singleton.h" #define NUM_RECENT_FILES 4 @@ -43,7 +42,7 @@ class LocationInformationWidget; typedef std::pair WidgetProperty; typedef QVector PropertyList; -class MainWindow : public QMainWindow, public SillySingleton { +class MainWindow : public QMainWindow { Q_OBJECT public: enum { @@ -62,6 +61,7 @@ public: MainWindow(); ~MainWindow(); + static MainWindow *instance(); void loadRecentFiles(); void updateRecentFiles(); void updateRecentFilesMenu(); @@ -193,6 +193,7 @@ private: QString filter_open(); QString filter_import(); QString filter_import_dive_sites(); + static MainWindow *m_Instance; QString displayedFilename(QString fullFilename); bool askSaveChanges(); bool okToClose(QString message); diff --git a/mobile-widgets/qmlmanager.cpp b/mobile-widgets/qmlmanager.cpp index 62c167653..858f2c730 100644 --- a/mobile-widgets/qmlmanager.cpp +++ b/mobile-widgets/qmlmanager.cpp @@ -44,6 +44,7 @@ #include "core/settings/qPrefUnit.h" #include "core/trip.h" +QMLManager *QMLManager::m_instance = NULL; bool noCloudToCloud = false; #define RED_FONT QLatin1Literal("") @@ -149,6 +150,7 @@ QMLManager::QMLManager() : m_locationServiceEnabled(false), m_showNonDiveComputers(false) { LOG_STP("qmlmgr starting"); + m_instance = this; m_lastDevicePixelRatio = qApp->devicePixelRatio(); timer.start(); connect(qobject_cast(QApplication::instance()), &QApplication::applicationStateChanged, this, &QMLManager::applicationStateChanged); @@ -449,6 +451,12 @@ QMLManager::~QMLManager() if (appLogFileOpen) appLogFile.close(); #endif + m_instance = NULL; +} + +QMLManager *QMLManager::instance() +{ + return m_instance; } #define CLOUDURL QString(prefs.cloud_base_url) @@ -1556,7 +1564,7 @@ void QMLManager::applyGpsData() void QMLManager::populateGpsData() { if (GpsListModel::instance()) - GpsListModel::instance()->update(QVector::fromList(locationProvider->currentGPSInfo().values())); + GpsListModel::instance()->update(); } void QMLManager::clearGpsData() diff --git a/mobile-widgets/qmlmanager.h b/mobile-widgets/qmlmanager.h index 963c563d7..e47119a71 100644 --- a/mobile-widgets/qmlmanager.h +++ b/mobile-widgets/qmlmanager.h @@ -13,14 +13,13 @@ #include "core/btdiscovery.h" #include "core/gpslocation.h" #include "core/downloadfromdcthread.h" -#include "core/singleton.h" #include "qt-models/divelistmodel.h" #include "qt-models/completionmodels.h" #include "qt-models/divelocationmodel.h" #define NOCLOUD_LOCALSTORAGE format_string("%s/cloudstorage/localrepo[master]", system_default_directory()) -class QMLManager : public QObject, public SillySingleton { +class QMLManager : public QObject { Q_OBJECT Q_PROPERTY(QString logText READ logText WRITE setLogText NOTIFY logTextChanged) Q_PROPERTY(bool locationServiceEnabled MEMBER m_locationServiceEnabled WRITE setLocationServiceEnabled NOTIFY locationServiceEnabledChanged) @@ -90,6 +89,7 @@ public: Q_INVOKABLE void setGitLocalOnly(const bool &value); Q_INVOKABLE void setFilter(const QString filterText); + static QMLManager *instance(); Q_INVOKABLE void registerError(QString error); QString consumeError(); @@ -219,6 +219,7 @@ private: bool m_verboseEnabled; GpsLocation *locationProvider; bool m_loadFromCloud; + static QMLManager *m_instance; struct dive *deletedDive; struct dive_trip *deletedTrip; QString m_notificationText; diff --git a/mobile-widgets/qmlprefs.cpp b/mobile-widgets/qmlprefs.cpp index e415c0e5d..cf7b5ddbb 100644 --- a/mobile-widgets/qmlprefs.cpp +++ b/mobile-widgets/qmlprefs.cpp @@ -8,13 +8,30 @@ /*** Global and constructors ***/ +QMLPrefs *QMLPrefs::m_instance = NULL; + QMLPrefs::QMLPrefs() : m_credentialStatus(qPrefCloudStorage::CS_UNKNOWN), m_oldStatus(qPrefCloudStorage::CS_UNKNOWN), m_showPin(false) { + // This strange construct is needed because QMLEngine calls new and that + // cannot be overwritten + if (!m_instance) + m_instance = this; +} + +QMLPrefs::~QMLPrefs() +{ + m_instance = NULL; } +QMLPrefs *QMLPrefs::instance() +{ + return m_instance; +} + + /*** public functions ***/ const QString QMLPrefs::cloudPassword() const { diff --git a/mobile-widgets/qmlprefs.h b/mobile-widgets/qmlprefs.h index 70964608b..99819fd7c 100644 --- a/mobile-widgets/qmlprefs.h +++ b/mobile-widgets/qmlprefs.h @@ -5,9 +5,9 @@ #include #include "core/settings/qPrefCloudStorage.h" #include "core/settings/qPrefDisplay.h" -#include "core/singleton.h" -class QMLPrefs : public QObject, public SillySingleton { + +class QMLPrefs : public QObject { Q_OBJECT Q_PROPERTY(QString cloudPassword MEMBER m_cloudPassword @@ -35,6 +35,9 @@ class QMLPrefs : public QObject, public SillySingleton { NOTIFY oldStatusChanged) public: QMLPrefs(); + ~QMLPrefs(); + + static QMLPrefs *instance(); const QString cloudPassword() const; void setCloudPassword(const QString &cloudPassword); @@ -63,6 +66,7 @@ private: QString m_cloudPin; QString m_cloudUserName; qPrefCloudStorage::cloud_status m_credentialStatus; + static QMLPrefs *m_instance; qPrefCloudStorage::cloud_status m_oldStatus; bool m_showPin; diff --git a/qt-models/divelistmodel.cpp b/qt-models/divelistmodel.cpp index e2fddbf83..779e695ab 100644 --- a/qt-models/divelistmodel.cpp +++ b/qt-models/divelistmodel.cpp @@ -134,8 +134,11 @@ QString DiveListSortModel::tripShortDate(const QString §ion) return QStringLiteral("%1\n'%2").arg(firstMonth,firstTime.toString("yy")); } +DiveListModel *DiveListModel::m_instance = NULL; + DiveListModel::DiveListModel(QObject *parent) : QAbstractListModel(parent) { + m_instance = this; } void DiveListModel::insertDive(int i) @@ -271,6 +274,11 @@ QString DiveListModel::startAddDive() return QString::number(d->id); } +DiveListModel *DiveListModel::instance() +{ + return m_instance; +} + struct dive *DiveListModel::getDive(int i) { if (i < 0 || i >= dive_table.nr) { diff --git a/qt-models/divelistmodel.h b/qt-models/divelistmodel.h index cccece51d..d6eb07463 100644 --- a/qt-models/divelistmodel.h +++ b/qt-models/divelistmodel.h @@ -6,7 +6,6 @@ #include #include "core/subsurface-qt/DiveObjectHelper.h" -#include "core/singleton.h" class DiveListSortModel : public QSortFilterProxyModel { @@ -29,7 +28,7 @@ private: void updateFilterState(); }; -class DiveListModel : public QAbstractListModel, public SillySingleton +class DiveListModel : public QAbstractListModel { Q_OBJECT public: @@ -46,6 +45,7 @@ public: DepthDurationRole, }; + static DiveListModel *instance(); DiveListModel(QObject *parent = 0); void addDive(const QList &listOfDives); void addAllDives(); @@ -63,6 +63,8 @@ public: QString startAddDive(); void resetInternalData(); Q_INVOKABLE DiveObjectHelper at(int i); +private: + static DiveListModel *m_instance; }; #endif // DIVELISTMODEL_H diff --git a/qt-models/gpslistmodel.cpp b/qt-models/gpslistmodel.cpp index 7849561d3..8d874d67b 100644 --- a/qt-models/gpslistmodel.cpp +++ b/qt-models/gpslistmodel.cpp @@ -3,12 +3,16 @@ #include "core/qthelper.h" #include +GpsListModel *GpsListModel::m_instance = NULL; + GpsListModel::GpsListModel(QObject *parent) : QAbstractListModel(parent) { + m_instance = this; } -void GpsListModel::update(QVector trackers) +void GpsListModel::update() { + QVector trackers = QVector::fromList(GpsLocation::instance()->currentGPSInfo().values()); beginResetModel(); m_gpsFixes = trackers; endResetModel(); @@ -58,3 +62,9 @@ QHash GpsListModel::roleNames() const roles[GpsLongitudeRole] = "longitude"; return roles; } + +GpsListModel *GpsListModel::instance() +{ + return m_instance; +} + diff --git a/qt-models/gpslistmodel.h b/qt-models/gpslistmodel.h index a49855a4d..a1c82e5d6 100644 --- a/qt-models/gpslistmodel.h +++ b/qt-models/gpslistmodel.h @@ -3,10 +3,10 @@ #define GPSLISTMODEL_H #include "core/gpslocation.h" -#include "core/singleton.h" +#include #include -class GpsListModel : public QAbstractListModel, public SillySingleton +class GpsListModel : public QAbstractListModel { Q_OBJECT public: @@ -19,14 +19,16 @@ public: GpsWhenRole }; + static GpsListModel *instance(); GpsListModel(QObject *parent = 0); void clear(); int rowCount(const QModelIndex &parent = QModelIndex()) const; QHash roleNames() const; QVariant data(const QModelIndex &index, int role = Qt::DisplayRole) const; - void update(QVector trackers); + void update(); private: QVector m_gpsFixes; + static GpsListModel *m_instance; }; #endif // GPSLISTMODEL_H -- cgit v1.2.3-70-g09d2