From 0e2125710249e3713b5fae537838fa8ae24f4737 Mon Sep 17 00:00:00 2001 From: Michael Zanetti Date: Sat, 21 Mar 2020 00:45:10 +0100 Subject: [PATCH] Fix race conditions at startup. Rework the setup sequence to not rely on timers anymore but instead always react to wifi state changes only. Fixes #29 --- debian/control | 2 +- nymea-networkmanager/core.cpp | 85 +++++++++-------------------------- nymea-networkmanager/core.h | 7 +-- 3 files changed, 25 insertions(+), 69 deletions(-) diff --git a/debian/control b/debian/control index ad81c87..3946f7e 100644 --- a/debian/control +++ b/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 diff --git a/nymea-networkmanager/core.cpp b/nymea-networkmanager/core.cpp index af128ed..2f5d76c 100644 --- a/nymea-networkmanager/core.cpp +++ b/nymea-networkmanager/core.cpp @@ -119,13 +119,7 @@ void Core::setButtonGpio(int buttonGpio) void Core::run() { // Start the networkmanager - if (!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); + m_networkManager->start(); } 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."; - // Give some grace periode for networkmanager - QTimer::singleShot(4000, this, &Core::startService); + qCDebug(dcApplication()) << "Starting the Bluetooth service because of \"always\" mode."; + 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; } } diff --git a/nymea-networkmanager/core.h b/nymea-networkmanager/core.h index f06fb35..47fc34d 100644 --- a/nymea-networkmanager/core.h +++ b/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 "nymea-networkmanager/bluetooth/bluetoothserver.h" +#include "networkmanager.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);