From 031b9a6b7c894d3aaeebe38915bdbc32debd7937 Mon Sep 17 00:00:00 2001 From: Murat Seker Date: Sun, 2 Aug 2020 22:32:00 +0200 Subject: [PATCH] refactor: Modernize Qt connections (#914) --- include/hyperion/Hyperion.h | 4 +- include/ssdp/SSDPDiscover.h | 5 +- .../BoblightClientConnection.cpp | 4 +- libsrc/boblightserver/BoblightServer.cpp | 6 +- libsrc/bonjour/bonjourservicebrowser.cpp | 2 +- libsrc/bonjour/bonjourserviceregister.cpp | 2 +- libsrc/bonjour/bonjourserviceresolver.cpp | 2 +- libsrc/effectengine/EffectEngine.cpp | 4 +- libsrc/grabber/v4l2/V4L2Grabber.cpp | 2 +- libsrc/grabber/v4l2/V4L2Wrapper.cpp | 4 +- libsrc/hyperion/Hyperion.cpp | 10 +-- libsrc/hyperion/SettingsManager.cpp | 1 - libsrc/jsonserver/JsonServer.cpp | 2 +- libsrc/leddevice/LedDevice.cpp | 2 +- libsrc/leddevice/dev_net/ProviderRestApi.cpp | 4 +- .../dev_other/LedDevicePiBlaster.cpp | 2 - .../dev_serial/LedDeviceAdalight.cpp | 6 +- .../leddevice/dev_serial/LedDeviceAdalight.h | 2 +- .../leddevice/dev_serial/LedDeviceKarate.cpp | 1 - libsrc/leddevice/dev_serial/ProviderRs232.cpp | 9 +- libsrc/ssdp/SSDPDiscover.cpp | 6 +- src/hyperiond/hyperiond.cpp | 83 +++++++++++-------- 22 files changed, 86 insertions(+), 77 deletions(-) diff --git a/include/hyperion/Hyperion.h b/include/hyperion/Hyperion.h index 7f5b2612..ae321ed1 100644 --- a/include/hyperion/Hyperion.h +++ b/include/hyperion/Hyperion.h @@ -68,7 +68,7 @@ public: /// /// free all alocated objects, should be called only from constructor or before restarting hyperion /// - void freeObjects(bool emitCloseSignal=false); + void freeObjects(); ImageProcessor* getImageProcessor() const { return _imageProcessor; } @@ -398,8 +398,6 @@ signals: /// void currentImage(const Image & image); - void closing(); - /// Signal which is emitted, when a new json message should be forwarded void forwardJsonMessage(QJsonObject); diff --git a/include/ssdp/SSDPDiscover.h b/include/ssdp/SSDPDiscover.h index ba3adc4b..290a7690 100644 --- a/include/ssdp/SSDPDiscover.h +++ b/include/ssdp/SSDPDiscover.h @@ -1,13 +1,14 @@ #ifndef SSDPDISCOVER_H #define SSDPDISCOVER_H -#include #include +#include #include #include #include +class Logger; class QUdpSocket; enum class searchType{ @@ -202,7 +203,7 @@ private: int _ssdpMaxWaitResponseTime; int _ssdpTimeout; - QMap _services; + QMultiMap _services; QStringList _usnList; QString _searchTarget; diff --git a/libsrc/boblightserver/BoblightClientConnection.cpp b/libsrc/boblightserver/BoblightClientConnection.cpp index cb1d7040..d051d579 100644 --- a/libsrc/boblightserver/BoblightClientConnection.cpp +++ b/libsrc/boblightserver/BoblightClientConnection.cpp @@ -39,8 +39,8 @@ BoblightClientConnection::BoblightClientConnection(Hyperion* hyperion, QTcpSocke _locale.setNumberOptions(QLocale::OmitGroupSeparator | QLocale::RejectGroupSeparator); // connect internal signals and slots - connect(_socket, SIGNAL(disconnected()), this, SLOT(socketClosed())); - connect(_socket, SIGNAL(readyRead()), this, SLOT(readData())); + connect(_socket, &QTcpSocket::disconnected, this, &BoblightClientConnection::socketClosed); + connect(_socket, &QTcpSocket::readyRead, this, &BoblightClientConnection::readData); } BoblightClientConnection::~BoblightClientConnection() diff --git a/libsrc/boblightserver/BoblightServer.cpp b/libsrc/boblightserver/BoblightServer.cpp index 1db0036c..59b4fa57 100644 --- a/libsrc/boblightserver/BoblightServer.cpp +++ b/libsrc/boblightserver/BoblightServer.cpp @@ -28,9 +28,9 @@ BoblightServer::BoblightServer(Hyperion* hyperion,const QJsonDocument& config) Debug(_log, "Instance created"); // listen for component change - connect(_hyperion, SIGNAL(compStateChangeRequest(hyperion::Components,bool)), this, SLOT(compStateChangeRequest(hyperion::Components,bool))); + connect(_hyperion, &Hyperion::compStateChangeRequest, this, &BoblightServer::compStateChangeRequest); // listen new connection signal from server - connect(_server, SIGNAL(newConnection()), this, SLOT(newConnection())); + connect(_server, &QTcpServer::newConnection, this, &BoblightServer::newConnection); // init handleSettingsUpdate(settings::BOBLSERVER, config); @@ -101,7 +101,7 @@ void BoblightServer::newConnection() _openConnections.insert(connection); // register slot for cleaning up after the connection closed - connect(connection, SIGNAL(connectionClosed(BoblightClientConnection*)), this, SLOT(closedConnection(BoblightClientConnection*))); + connect(connection, &BoblightClientConnection::connectionClosed, this, &BoblightServer::closedConnection); } } diff --git a/libsrc/bonjour/bonjourservicebrowser.cpp b/libsrc/bonjour/bonjourservicebrowser.cpp index 5b7664dd..a43346b0 100644 --- a/libsrc/bonjour/bonjourservicebrowser.cpp +++ b/libsrc/bonjour/bonjourservicebrowser.cpp @@ -63,7 +63,7 @@ void BonjourServiceBrowser::browseForServiceType(const QString &serviceType) else { bonjourSocket = new QSocketNotifier(sockfd, QSocketNotifier::Read, this); - connect(bonjourSocket, SIGNAL(activated(int)), this, SLOT(bonjourSocketReadyRead())); + connect(bonjourSocket, &QSocketNotifier::activated, this, &BonjourServiceBrowser::bonjourSocketReadyRead); } } } diff --git a/libsrc/bonjour/bonjourserviceregister.cpp b/libsrc/bonjour/bonjourserviceregister.cpp index 9755ed1e..4e473432 100644 --- a/libsrc/bonjour/bonjourserviceregister.cpp +++ b/libsrc/bonjour/bonjourserviceregister.cpp @@ -115,7 +115,7 @@ void BonjourServiceRegister::registerService(const BonjourRecord &record, quint1 else { bonjourSocket = new QSocketNotifier(sockfd, QSocketNotifier::Read, this); - connect(bonjourSocket, SIGNAL(activated(int)), this, SLOT(bonjourSocketReadyRead())); + connect(bonjourSocket, &QSocketNotifier::activated, this, &BonjourServiceRegister::bonjourSocketReadyRead); } } } diff --git a/libsrc/bonjour/bonjourserviceresolver.cpp b/libsrc/bonjour/bonjourserviceresolver.cpp index aa1a932f..14cebcb1 100644 --- a/libsrc/bonjour/bonjourserviceresolver.cpp +++ b/libsrc/bonjour/bonjourserviceresolver.cpp @@ -82,7 +82,7 @@ bool BonjourServiceResolver::resolveBonjourRecord(const BonjourRecord &record) else { bonjourSocket = new QSocketNotifier(sockfd, QSocketNotifier::Read, this); - connect(bonjourSocket, SIGNAL(activated(int)), this, SLOT(bonjourSocketReadyRead())); + connect(bonjourSocket, &QSocketNotifier::activated, this, &BonjourServiceResolver::bonjourSocketReadyRead); } } return true; diff --git a/libsrc/effectengine/EffectEngine.cpp b/libsrc/effectengine/EffectEngine.cpp index 0ce5c839..f8f839bb 100644 --- a/libsrc/effectengine/EffectEngine.cpp +++ b/libsrc/effectengine/EffectEngine.cpp @@ -32,8 +32,8 @@ EffectEngine::EffectEngine(Hyperion * hyperion) qRegisterMetaType("hyperion::Components"); // connect the Hyperion channel clear feedback - connect(_hyperion, SIGNAL(channelCleared(int)), this, SLOT(channelCleared(int))); - connect(_hyperion, SIGNAL(allChannelsCleared()), this, SLOT(allChannelsCleared())); + connect(_hyperion, &Hyperion::channelCleared, this, &EffectEngine::channelCleared); + connect(_hyperion, &Hyperion::allChannelsCleared, this, &EffectEngine::allChannelsCleared); // get notifications about refreshed effect list connect(_effectFileHandler, &EffectFileHandler::effectListChanged, this, &EffectEngine::handleUpdatedEffectList); diff --git a/libsrc/grabber/v4l2/V4L2Grabber.cpp b/libsrc/grabber/v4l2/V4L2Grabber.cpp index 0f71e2e3..e1536fff 100644 --- a/libsrc/grabber/v4l2/V4L2Grabber.cpp +++ b/libsrc/grabber/v4l2/V4L2Grabber.cpp @@ -357,7 +357,7 @@ bool V4L2Grabber::open_device() // create the notifier for when a new frame is available _streamNotifier = new QSocketNotifier(_fileDescriptor, QSocketNotifier::Read); _streamNotifier->setEnabled(false); - connect(_streamNotifier, SIGNAL(activated(int)), this, SLOT(read_frame())); + connect(_streamNotifier, &QSocketNotifier::activated, this, &V4L2Grabber::read_frame); return true; } diff --git a/libsrc/grabber/v4l2/V4L2Wrapper.cpp b/libsrc/grabber/v4l2/V4L2Wrapper.cpp index 730e44c3..6824ce0e 100644 --- a/libsrc/grabber/v4l2/V4L2Wrapper.cpp +++ b/libsrc/grabber/v4l2/V4L2Wrapper.cpp @@ -29,8 +29,8 @@ V4L2Wrapper::V4L2Wrapper(const QString &device, qRegisterMetaType>("Image"); // Handle the image in the captured thread using a direct connection - connect(&_grabber, SIGNAL(newFrame(Image)), this, SLOT(newFrame(Image)), Qt::DirectConnection); - connect(&_grabber, SIGNAL(readError(const char*)), this, SLOT(readError(const char*)), Qt::DirectConnection); + connect(&_grabber, &V4L2Grabber::newFrame, this, &V4L2Wrapper::newFrame, Qt::DirectConnection); + connect(&_grabber, &V4L2Grabber::readError, this, &V4L2Wrapper::readError, Qt::DirectConnection); } V4L2Wrapper::~V4L2Wrapper() diff --git a/libsrc/hyperion/Hyperion.cpp b/libsrc/hyperion/Hyperion.cpp index 0a5365ed..71008115 100644 --- a/libsrc/hyperion/Hyperion.cpp +++ b/libsrc/hyperion/Hyperion.cpp @@ -39,7 +39,6 @@ // Boblight #include - Hyperion::Hyperion(const quint8& instance) : QObject() , _instIndex(instance) @@ -61,7 +60,7 @@ Hyperion::Hyperion(const quint8& instance) Hyperion::~Hyperion() { - freeObjects(false); + freeObjects(); } void Hyperion::start() @@ -152,16 +151,11 @@ void Hyperion::stop() thread()->wait(); } -void Hyperion::freeObjects(bool emitCloseSignal) +void Hyperion::freeObjects() { // switch off all leds clear(-1,true); - if (emitCloseSignal) - { - emit closing(); - } - // delete components on exit of hyperion core delete _boblightServer; delete _captureCont; diff --git a/libsrc/hyperion/SettingsManager.cpp b/libsrc/hyperion/SettingsManager.cpp index 51a3291d..44de6357 100644 --- a/libsrc/hyperion/SettingsManager.cpp +++ b/libsrc/hyperion/SettingsManager.cpp @@ -80,7 +80,6 @@ SettingsManager::SettingsManager(const quint8& instance, QObject* parent) saveSettings(dbConfig, true); } - // validate full dbconfig against schema, on error we need to rewrite entire table QJsonSchemaChecker schemaChecker; schemaChecker.setSchema(schemaJson); diff --git a/libsrc/jsonserver/JsonServer.cpp b/libsrc/jsonserver/JsonServer.cpp index 4618b9ae..b97bfe9d 100644 --- a/libsrc/jsonserver/JsonServer.cpp +++ b/libsrc/jsonserver/JsonServer.cpp @@ -27,7 +27,7 @@ JsonServer::JsonServer(const QJsonDocument& config) Debug(_log, "Created instance"); // Set trigger for incoming connections - connect(_server, SIGNAL(newConnection()), this, SLOT(newConnection())); + connect(_server, &QTcpServer::newConnection, this, &JsonServer::newConnection); // init handleSettingsUpdate(settings::JSONSERVER, config); diff --git a/libsrc/leddevice/LedDevice.cpp b/libsrc/leddevice/LedDevice.cpp index bb446e39..2019c287 100644 --- a/libsrc/leddevice/LedDevice.cpp +++ b/libsrc/leddevice/LedDevice.cpp @@ -245,7 +245,7 @@ int LedDevice::writeBlack(int numberOfBlack) { // Wait latch time before writing black QEventLoop loop; - QTimer::singleShot( _latchTime_ms, &loop, SLOT( quit() ) ); + QTimer::singleShot(_latchTime_ms, &loop, &QEventLoop::quit); loop.exec(); } rc = write(std::vector(static_cast(_ledCount), ColorRgb::BLACK )); diff --git a/libsrc/leddevice/dev_net/ProviderRestApi.cpp b/libsrc/leddevice/dev_net/ProviderRestApi.cpp index 783667aa..d40a2d7d 100644 --- a/libsrc/leddevice/dev_net/ProviderRestApi.cpp +++ b/libsrc/leddevice/dev_net/ProviderRestApi.cpp @@ -128,7 +128,7 @@ httpResponse ProviderRestApi::get(const QUrl &url) QNetworkReply* reply = _networkManager->get(request); // Connect requestFinished signal to quit slot of the loop. QEventLoop loop; - loop.connect(reply, SIGNAL(finished()), SLOT(quit())); + loop.connect(reply, &QNetworkReply::finished, &loop, &QEventLoop::quit); // Go into the loop until the request is finished. loop.exec(); @@ -156,7 +156,7 @@ httpResponse ProviderRestApi::put(const QUrl &url, const QString &body) QNetworkReply* reply = _networkManager->put(request, body.toUtf8()); // Connect requestFinished signal to quit slot of the loop. QEventLoop loop; - loop.connect(reply, SIGNAL(finished()), SLOT(quit())); + loop.connect(reply, &QNetworkReply::finished, &loop, &QEventLoop::quit); // Go into the loop until the request is finished. loop.exec(); diff --git a/libsrc/leddevice/dev_other/LedDevicePiBlaster.cpp b/libsrc/leddevice/dev_other/LedDevicePiBlaster.cpp index 89ef50e5..816d7fee 100644 --- a/libsrc/leddevice/dev_other/LedDevicePiBlaster.cpp +++ b/libsrc/leddevice/dev_other/LedDevicePiBlaster.cpp @@ -17,8 +17,6 @@ LedDevicePiBlaster::LedDevicePiBlaster(const QJsonObject &deviceConfig) _activeDeviceType = deviceConfig["type"].toString("UNSPECIFIED").toLower(); - signal(SIGPIPE, SIG_IGN); - // initialise the mapping tables // -1 is invalid // z is also meaningless diff --git a/libsrc/leddevice/dev_serial/LedDeviceAdalight.cpp b/libsrc/leddevice/dev_serial/LedDeviceAdalight.cpp index f8509fa6..66f106e9 100644 --- a/libsrc/leddevice/dev_serial/LedDeviceAdalight.cpp +++ b/libsrc/leddevice/dev_serial/LedDeviceAdalight.cpp @@ -1,5 +1,7 @@ #include "LedDeviceAdalight.h" +#include + LedDeviceAdalight::LedDeviceAdalight(const QJsonObject &deviceConfig) : ProviderRs232() , _headerSize(6) @@ -78,7 +80,9 @@ int LedDeviceAdalight::write(const std::vector & ledValues) } else { - memcpy(_headerSize + _ledBuffer.data(), ledValues.data(), ledValues.size() * 3); + assert(_headerSize + ledValues.size() * sizeof(ColorRgb) <= _ledBuffer.size()); + + memcpy(_headerSize + _ledBuffer.data(), ledValues.data(), ledValues.size() * sizeof(ColorRgb)); } int rc = writeBytes(_ledBuffer.size(), _ledBuffer.data()); diff --git a/libsrc/leddevice/dev_serial/LedDeviceAdalight.h b/libsrc/leddevice/dev_serial/LedDeviceAdalight.h index 3812a457..38a522b1 100644 --- a/libsrc/leddevice/dev_serial/LedDeviceAdalight.h +++ b/libsrc/leddevice/dev_serial/LedDeviceAdalight.h @@ -44,7 +44,7 @@ private: /// @return Zero on success, else negative /// virtual int write(const std::vector & ledValues) override; - + const short _headerSize; bool _ligthBerryAPA102Mode; }; diff --git a/libsrc/leddevice/dev_serial/LedDeviceKarate.cpp b/libsrc/leddevice/dev_serial/LedDeviceKarate.cpp index e37d8507..f7467a03 100644 --- a/libsrc/leddevice/dev_serial/LedDeviceKarate.cpp +++ b/libsrc/leddevice/dev_serial/LedDeviceKarate.cpp @@ -50,7 +50,6 @@ int LedDeviceKarate::write(const std::vector &ledValues) { for (signed iLed=0; iLed< static_cast(_ledCount); iLed++) { - const ColorRgb& rgb = ledValues[iLed]; _ledBuffer[iLed*3+4] = rgb.green; _ledBuffer[iLed*3+5] = rgb.blue; diff --git a/libsrc/leddevice/dev_serial/ProviderRs232.cpp b/libsrc/leddevice/dev_serial/ProviderRs232.cpp index d7309247..662f06d2 100644 --- a/libsrc/leddevice/dev_serial/ProviderRs232.cpp +++ b/libsrc/leddevice/dev_serial/ProviderRs232.cpp @@ -12,8 +12,7 @@ // Constants constexpr std::chrono::milliseconds WRITE_TIMEOUT{1000}; // device write timeout in ms constexpr std::chrono::milliseconds OPEN_TIMEOUT{5000}; // device open timeout in ms -const int MAX_WRITE_TIMEOUTS = 5; // maximum number of allowed timeouts - +const int MAX_WRITE_TIMEOUTS = 5; // Maximum number of allowed timeouts const int NUM_POWEROFF_WRITE_BLACK = 2; // Number of write "BLACK" during powering off ProviderRs232::ProviderRs232() @@ -43,9 +42,9 @@ bool ProviderRs232::init(const QJsonObject &deviceConfig) // If device name was given as unix /dev/ system-location, get port name if ( _deviceName.startsWith(QLatin1String("/dev/")) ) - _deviceName = _deviceName.mid(5); + _deviceName = _deviceName.mid(5); - _isAutoDeviceName = _deviceName.toLower() == "auto"; + _isAutoDeviceName = _deviceName.toLower() == "auto"; _baudRate_Hz = deviceConfig["rate"].toInt(); _delayAfterConnect_ms = deviceConfig["delayAfterConnect"].toInt(1500); @@ -174,7 +173,7 @@ bool ProviderRs232::tryOpen(const int delayAfterConnect_ms) // Wait delayAfterConnect_ms before allowing write QEventLoop loop; - QTimer::singleShot( delayAfterConnect_ms, &loop, SLOT( quit() ) ); + QTimer::singleShot(delayAfterConnect_ms, &loop, &QEventLoop::quit); loop.exec(); Debug(_log, "delayAfterConnect for %d ms - finished", delayAfterConnect_ms); diff --git a/libsrc/ssdp/SSDPDiscover.cpp b/libsrc/ssdp/SSDPDiscover.cpp index af952f94..d1e1ee50 100644 --- a/libsrc/ssdp/SSDPDiscover.cpp +++ b/libsrc/ssdp/SSDPDiscover.cpp @@ -1,15 +1,15 @@ #include +#include #include // Qt includes #include #include #include +#include #include -#include - -#include +#include // Constants namespace { diff --git a/src/hyperiond/hyperiond.cpp b/src/hyperiond/hyperiond.cpp index dd4e4e1f..1d8962c7 100644 --- a/src/hyperiond/hyperiond.cpp +++ b/src/hyperiond/hyperiond.cpp @@ -117,8 +117,6 @@ HyperionDaemon::HyperionDaemon(const QString rootPath, QObject *parent, const bo // spawn all Hyperion instances (non blocking) _instanceManager->startAll(); - //connect(_hyperion,SIGNAL(closing()),this,SLOT(freeObjects())); // TODO for app restart, refactor required - //Cleaning up Hyperion before quit connect(parent, SIGNAL(aboutToQuit()), this, SLOT(freeObjects())); @@ -175,38 +173,64 @@ void HyperionDaemon::freeObjects() // destroy network first as a client might want to access hyperion delete _jsonServer; + _jsonServer = nullptr; - auto flatBufferServerThread = _flatBufferServer->thread(); - flatBufferServerThread->quit(); - flatBufferServerThread->wait(); - delete flatBufferServerThread; + if (_flatBufferServer) + { + auto flatBufferServerThread = _flatBufferServer->thread(); + flatBufferServerThread->quit(); + flatBufferServerThread->wait(); + delete flatBufferServerThread; + _flatBufferServer = nullptr; + } - auto protoServerThread = _protoServer->thread(); - protoServerThread->quit(); - protoServerThread->wait(); - delete protoServerThread; + if (_protoServer) + { + auto protoServerThread = _protoServer->thread(); + protoServerThread->quit(); + protoServerThread->wait(); + delete protoServerThread; + _protoServer = nullptr; + } //ssdp before webserver - auto ssdpThread = _ssdp->thread(); - ssdpThread->quit(); - ssdpThread->wait(); - delete ssdpThread; + if (_ssdp) + { + auto ssdpThread = _ssdp->thread(); + ssdpThread->quit(); + ssdpThread->wait(); + delete ssdpThread; + _ssdp = nullptr; + } - auto webserverThread =_webserver->thread(); - webserverThread->quit(); - webserverThread->wait(); - delete webserverThread; + if(_webserver) + { + auto webserverThread =_webserver->thread(); + webserverThread->quit(); + webserverThread->wait(); + delete webserverThread; + _webserver = nullptr; + } - auto sslWebserverThread =_sslWebserver->thread(); - sslWebserverThread->quit(); - sslWebserverThread->wait(); - delete sslWebserverThread; + if (_sslWebserver) + { + auto sslWebserverThread =_sslWebserver->thread(); + sslWebserverThread->quit(); + sslWebserverThread->wait(); + delete sslWebserverThread; + _sslWebserver = nullptr; + } #ifdef ENABLE_CEC - _cecHandler->thread()->quit(); - _cecHandler->thread()->wait(1000); - delete _cecHandler->thread(); - delete _cecHandler; + if (_cecHandler) + { + auto cecHandlerThread = _cecHandler->thread(); + cecHandlerThread->quit(); + cecHandlerThread->wait(); + delete cecHandlerThread; + delete _cecHandler; + _cecHandler = nullptr; + } #endif // stop Hyperions (non blocking) @@ -221,19 +245,12 @@ void HyperionDaemon::freeObjects() delete _v4l2Grabber; _v4l2Grabber = nullptr; - _cecHandler = nullptr; _bonjourBrowserWrapper = nullptr; _amlGrabber = nullptr; _dispmanx = nullptr; _fbGrabber = nullptr; _osxGrabber = nullptr; _qtGrabber = nullptr; - _flatBufferServer = nullptr; - _protoServer = nullptr; - _ssdp = nullptr; - _webserver = nullptr; - _sslWebserver = nullptr; - _jsonServer = nullptr; } void HyperionDaemon::startNetworkServices()