Merge PR #30: Fix race conditions at startup.

debian/control

@@ -10,7 +10,7 @@ Build-Depends: debhelper (>= 9.0.0),
   qtbase5-dev-tools,
   libqt5bluetooth5,
   qtconnectivity5-dev,
-  libnymea-networkmanager-dev,
+  libnymea-networkmanager-dev (>= 0.3.0),
   libnymea-gpio-dev
 Standards-Version: 3.9.7
 

nymea-networkmanager/core.cpp

@@ -119,13 +119,7 @@ void Core::setButtonGpio(int buttonGpio)
 void Core::run()
 {
     // Start the networkmanager
-    if (!m_networkManager->start()) {
+    m_networkManager->start();
-        qCWarning(dcApplication()) << "Could not start network manager. Please make sure the networkmanager is available.";
-        return;
-    }
-
-    // Note: give network-manager more time to start and get online status
-    QTimer::singleShot(3000, this, &Core::postRun);
 }
 
 Core::Core(QObject *parent) :
@@ -146,6 +140,10 @@ Core::Core(QObject *parent) :
     m_advertisingTimer = new QTimer(this);
     m_advertisingTimer->setSingleShot(true);
     connect(m_advertisingTimer, &QTimer::timeout, this, &Core::onAdvertisingTimeout);
+
+    m_button = new GpioButton(m_buttonGpio, this);
+    m_button->setLongPressedTimeout(2000);
+    connect(m_button, &GpioButton::longPressed, this, &Core::onButtonLongPressed);
 }
 
 Core::~Core()
@@ -168,10 +166,6 @@ void Core::evaluateNetworkManagerState(NetworkManager::NetworkManagerState state
     if (m_mode != ModeOffline)
         return;
 
-    // If we are still initializing, we don't need to react on the state changed
-    if (m_initRunning)
-        return;
-
     // Note: if the wireless device is in the access point mode, the bluetooth server should stop
     if (m_wirelessDevice && m_wirelessDevice->mode() == WirelessNetworkDevice::ModeAccessPoint) {
         stopService();
@@ -210,7 +204,6 @@ void Core::evaluateNetworkManagerState(NetworkManager::NetworkManagerState state
 
 void Core::startService()
 {
-    qCDebug(dcApplication()) << "Start the service...";
     if (!m_networkManager->available()) {
         qCWarning(dcApplication()) << "Could not start services. There is no network manager available.";
         return;
@@ -235,51 +228,11 @@ void Core::startService()
 void Core::stopService()
 {
     if (m_bluetoothServer && m_bluetoothServer->running()) {
-        qCDebug(dcApplication()) << "Stop bluetooth service";
+        qCDebug(dcApplication()) << "Stopping bluetooth service";
         m_bluetoothServer->stop();
     }
 }
 
-void Core::postRun()
-{
-    qCDebug(dcApplication()) << "Post run service";
-    m_initRunning = false;
-
-    switch (m_mode) {
-    case ModeAlways:
-        qCDebug(dcApplication()) << "Start the bluetooth service because of \"always\" mode.";
-        startService();
-        break;
-    case ModeStart:
-        qCDebug(dcApplication()) << "Start the bluetooth service because of \"start\" mode.";
-        startService();
-        m_advertisingTimer->start(m_advertisingTimeout * 1000);
-        break;
-    case ModeOffline:
-        evaluateNetworkManagerState(m_networkManager->state());
-        break;
-    case ModeOnce:
-        if (m_networkManager->networkSettings()->connections().isEmpty()) {
-            qCDebug(dcApplication()) << "Start the bluetooth service because of \"once\" mode and there is currenlty no network configured yet.";
-            startService();
-        } else {
-            qCDebug(dcApplication()) << "Not starting the bluetooth service because of \"once\" mode. There are" << m_networkManager->networkSettings()->connections().count() << "network configurations.";
-        }
-        break;
-    case ModeButton:
-        // Enable button
-        m_button = new GpioButton(m_buttonGpio, this);
-        m_button->setLongPressedTimeout(2000);
-        connect(m_button, &GpioButton::longPressed, this, &Core::onButtonLongPressed);
-        if (!m_button->enable()) {
-            qCCritical(dcApplication()) << "Could not not enable GPIO button for" << m_buttonGpio;
-            m_button->deleteLater();
-            m_button = nullptr;
-        }
-        break;
-    }
-}
-
 void Core::onAdvertisingTimeout()
 {
     if (m_mode != ModeStart)
@@ -348,31 +301,37 @@ void Core::onNetworkManagerAvailableChanged(bool available)
 
     qCDebug(dcApplication()) << "Networkmanager is now available.";
 
-    if (m_initRunning) {
-        qCDebug(dcApplication()) << "Init is still running...";
-        return;
-    }
-
     switch (m_mode) {
     case ModeAlways:
-        qCDebug(dcApplication()) << "Start the bluetooth service because of \"always\" mode.";
+        qCDebug(dcApplication()) << "Starting the Bluetooth service because of \"always\" mode.";
-        // Give some grace periode for networkmanager
+        startService();
-        QTimer::singleShot(4000, this, &Core::startService);
         break;
     case ModeStart:
+        // Only start it once in "start" mode...
+        static bool alreadyRan = false;
+        if (alreadyRan) {
+            return;
+        }
+        qCDebug(dcApplication()) << "Starting the Bluetooth service because of \"start\" mode.";
+        alreadyRan = true;
+        startService();
+        m_advertisingTimer->start(m_advertisingTimeout * 1000);
         break;
     case ModeOffline:
         evaluateNetworkManagerState(m_networkManager->state());
         break;
     case ModeOnce:
         if (m_networkManager->networkSettings()->connections().isEmpty()) {
-            qCDebug(dcApplication()) << "Start the bluetooth service because of \"once\" mode and there is currenlty no network configured yet.";
+            qCDebug(dcApplication()) << "Starting the Bluetooth service because of \"once\" mode and there is currenlty no network configured yet.";
             startService();
         } else {
-            qCDebug(dcApplication()) << "Not starting the bluetooth service because of \"once\" mode. There are" << m_networkManager->networkSettings()->connections().count() << "network configurations.";
+            qCDebug(dcApplication()) << "Not starting the Bluetooth service because of \"once\" mode. There are" << m_networkManager->networkSettings()->connections().count() << "network configurations.";
         }
         break;
     case ModeButton:
+        if (!m_button->enable()) {
+            qCCritical(dcApplication()) << "Failed to enable the GPIO button for" << m_buttonGpio;
+        }
         break;
     }
 }

nymea-networkmanager/core.h

@@ -36,8 +36,8 @@
 #include "nymeadservice.h"
 #include "nymea-gpio/gpiobutton.h"
 #include "bluetooth/bluetoothserver.h"
-#include "nymea-networkmanager/networkmanager.h"
+#include "networkmanager.h"
-#include "nymea-networkmanager/bluetooth/bluetoothserver.h"
+#include "bluetooth/bluetoothserver.h"
 
 class Core : public QObject
 {
@@ -95,7 +95,6 @@ private:
     QString m_advertiseName;
     QString m_platformName;
     int m_advertisingTimeout = 60;
-    bool m_initRunning = true;
     int m_buttonGpio = -1;
 
     void evaluateNetworkManagerState(NetworkManager::NetworkManagerState state);
@@ -104,8 +103,6 @@ private slots:
     void startService();
     void stopService();
 
-    void postRun();
-
     void onAdvertisingTimeout();
 
     void onBluetoothServerRunningChanged(bool running);