From 9065a4cb88c46e131ef8af77fb9b1f7d24194cc9 Mon Sep 17 00:00:00 2001 From: Elias Steurer Date: Sun, 18 Apr 2021 18:03:08 +0200 Subject: [PATCH] Refactor most ScreenPlayManager functions to retrun bool Fix removal of ScreenPlay Wallpaper iteration --- ScreenPlay/src/screenplaymanager.cpp | 134 +++++++++++++++++-------- ScreenPlay/src/screenplaymanager.h | 10 +- ScreenPlay/src/screenplaywallpaper.cpp | 10 +- ScreenPlay/src/screenplaywallpaper.h | 2 +- ScreenPlay/src/sdkconnection.cpp | 4 +- ScreenPlay/src/sdkconnection.h | 2 +- ScreenPlayWallpaper/main.cpp | 2 +- 7 files changed, 106 insertions(+), 58 deletions(-) diff --git a/ScreenPlay/src/screenplaymanager.cpp b/ScreenPlay/src/screenplaymanager.cpp index afd1fdec..aa810a92 100644 --- a/ScreenPlay/src/screenplaymanager.cpp +++ b/ScreenPlay/src/screenplaymanager.cpp @@ -238,9 +238,15 @@ bool ScreenPlayManager::removeAllWallpapers() return false; } + QStringList appIDs; + // Do not remove items from the vector you iterate on. for (auto& client : m_screenPlayWallpapers) { - if (removeWallpaper(client->appID())) { - decreaseActiveWallpaperCounter(); + appIDs.append(client->appID()); + } + + for (const auto& appID : appIDs) { + if (!removeWallpaper(appID)) { + return false; } } @@ -260,8 +266,8 @@ bool ScreenPlayManager::removeAllWidgets() } for (auto& client : m_screenPlayWallpapers) { - if (removeWidget(client->appID())) { - decreaseActiveWidgetsCounter(); + if (!removeWidget(client->appID())) { + return false; } } @@ -292,16 +298,17 @@ bool ScreenPlayManager::removeWallpaperAt(int index) /*! \brief Request a spesific json profile to display in the active wallpaper popup on the right. */ -void ScreenPlayManager::requestProjectSettingsAtMonitorIndex(const int index) +bool ScreenPlayManager::requestProjectSettingsAtMonitorIndex(const int index) { for (const std::shared_ptr& uPtrWallpaper : qAsConst(m_screenPlayWallpapers)) { if (uPtrWallpaper->screenNumber()[0] == index) { emit projectSettingsListModelResult( uPtrWallpaper->getProjectSettingsListModel()); - return; + return true; } } + return false; } /*! @@ -310,8 +317,7 @@ void ScreenPlayManager::requestProjectSettingsAtMonitorIndex(const int index) bool ScreenPlayManager::setWallpaperValueAtMonitorIndex(const int index, const QString& key, const QString& value) { if (auto appID = m_monitorListModel->getAppIDByMonitorIndex(index)) { - setWallpaperValue(*appID, key, value); - return true; + return setWallpaperValue(*appID, key, value); } qWarning() << "Could net get appID from m_monitorListModel!"; @@ -321,11 +327,12 @@ bool ScreenPlayManager::setWallpaperValueAtMonitorIndex(const int index, const Q /*! \brief Convenient function to set a \a value at a given \a index and \a key for all wallaper. For exmaple used to mute all wallpaper. */ -void ScreenPlayManager::setAllWallpaperValue(const QString& key, const QString& value) +bool ScreenPlayManager::setAllWallpaperValue(const QString& key, const QString& value) { for (auto& wallpaper : m_screenPlayWallpapers) { - setWallpaperValue(wallpaper->appID(), key, value); + return setWallpaperValue(wallpaper->appID(), key, value); } + return true; } /*! @@ -403,54 +410,83 @@ void ScreenPlayManager::newConnection() */ bool ScreenPlayManager::removeWallpaper(const QString& appID) { - for (auto& wallpaper : m_screenPlayWallpapers) { - if (wallpaper->appID() == appID) { + m_screenPlayWallpapers.erase( + std::remove_if( + m_screenPlayWallpapers.begin(), + m_screenPlayWallpapers.end(), + [this, appID](auto& wallpaper) { + if (wallpaper->appID() != appID) { + return false; + } - qInfo() << "Remove wallpaper " << wallpaper->file() << "at monitor " << wallpaper->screenNumber(); + qInfo() << "Remove wallpaper " << wallpaper->file() << "at monitor " << wallpaper->screenNumber(); - // The MonitorListModel contains a shared_ptr of this object that needs to be removed - // for shared_ptr to release the object. - m_monitorListModel->setWallpaperMonitor({}, wallpaper->screenNumber()); + // The MonitorListModel contains a shared_ptr of this object that needs to be removed + // for shared_ptr to release the object. + m_monitorListModel->setWallpaperMonitor({}, wallpaper->screenNumber()); - decreaseActiveWallpaperCounter(); - if (!m_screenPlayWallpapers.removeOne(wallpaper)) { - qWarning() << "Unable to remove Wallpaper from Wallpaper List"; - return false; - } + decreaseActiveWallpaperCounter(); - if (activeWallpaperCounter() != m_screenPlayWallpapers.length()) { - qWarning() << "activeWallpaperCounter value: " << activeWallpaperCounter() - << "does not match m_screenPlayWallpapers length:" << m_screenPlayWallpapers.length(); - return false; - } - return true; - } + return true; + })); + + if (activeWallpaperCounter() != m_screenPlayWallpapers.length()) { + qWarning() << "activeWallpaperCounter value: " << activeWallpaperCounter() + << "does not match m_screenPlayWallpapers length:" << m_screenPlayWallpapers.length(); + return false; } - return false; + + return true; } +/*! + \brief Removes a Widget from the given appID. Returns true on success. +*/ bool ScreenPlayManager::removeWidget(const QString& appID) { + m_screenPlayWidgets.erase( + std::remove_if( + m_screenPlayWidgets.begin(), + m_screenPlayWidgets.end(), + [this, appID](auto& widget) { + if (widget->appID() != appID) { + return false; + } + + qInfo() << "Remove widget "; + + decreaseActiveWidgetsCounter(); + + return true; + })); + + if (activeWidgetsCounter() != m_screenPlayWidgets.length()) { + qWarning() << "activeWallpaperCounter value: " << activeWidgetsCounter() + << "does not match m_screenPlayWallpapers length:" << m_screenPlayWidgets.length(); + return false; + } + return false; } /*! \brief Sets a given \a value to a given \a key. The \a appID is used to identify the receiver socket. */ -void ScreenPlayManager::setWallpaperValue(const QString& appID, const QString& key, const QString& value) +bool ScreenPlayManager::setWallpaperValue(const QString& appID, const QString& key, const QString& value) { for (auto& wallpaper : m_screenPlayWallpapers) { if (wallpaper->appID() == appID) { - wallpaper->setWallpaperValue(key, value, true); + return wallpaper->setWallpaperValue(key, value, true); } } + return false; } /*! \brief Saves a given wallpaper \a newProfileObject to a \a profileName. We ignore the profileName argument because we currently only support one profile. Returns \c true if successfuly saved to profiles.json, otherwise \c false. */ -void ScreenPlayManager::saveProfiles() +bool ScreenPlayManager::saveProfiles() { m_saveLimiter.stop(); @@ -477,34 +513,37 @@ void ScreenPlayManager::saveProfiles() profile.insert("version", "1.0.0"); profile.insert("profiles", activeProfileList); - if (Util::writeJsonObjectToFile({ m_globalVariables->localSettingsPath().toString() + "/profiles.json" }, profile)) + if (Util::writeJsonObjectToFile({ m_globalVariables->localSettingsPath().toString() + "/profiles.json" }, profile)) { emit profilesSaved(); + return true; + } + return false; } /*! \brief Loads all wallpaper from profiles.json when the version number matches and starts the available wallpaper */ -void ScreenPlayManager::loadProfiles() +bool ScreenPlayManager::loadProfiles() { - auto configObj = ScreenPlayUtil::openJsonFileToObject(m_globalVariables->localSettingsPath().toString() + "/profiles.json"); + const auto configObj = ScreenPlayUtil::openJsonFileToObject(m_globalVariables->localSettingsPath().toString() + "/profiles.json"); if (!configObj) { qWarning() << "Could not load active profiles at path: " << m_globalVariables->localSettingsPath().toString() + "/profiles.json"; - return; + return false; } std::optional version = ScreenPlayUtil::getVersionNumberFromString(configObj->value("version").toString()); if (version && *version != m_globalVariables->version()) { qWarning() << "Version missmatch fileVersion: " << version->toString() << "m_version: " << m_globalVariables->version().toString(); - return; + return false; } QJsonArray activeProfilesTmp = configObj->value("profiles").toArray(); if (activeProfilesTmp.size() > 1) { qWarning() << "We currently only support one profile!"; - return; + return false; } qInfo() << "Loading profiles " << activeProfilesTmp.size(); @@ -528,12 +567,12 @@ void ScreenPlayManager::loadProfiles() int value = monitorNumber.toInt(-1); if (value == -1) { qWarning() << "Could not parse monitor number to display content at"; - return; + return false; } if (monitors.contains(value)) { qWarning() << "The monitor: " << value << " is sharing the config multiple times. "; - return; + return false; } monitors.append(value); } @@ -554,8 +593,11 @@ void ScreenPlayManager::loadProfiles() const auto type = QStringToEnum(typeString, InstalledType::InstalledType::VideoWallpaper); const auto fillMode = QStringToEnum(fillModeString, FillMode::FillMode::Cover); - qInfo() << "Start wallpaper from profile:" << type << fillMode << monitors << absolutePath; - createWallpaper(type, fillMode, absolutePath, previewImage, file, monitors, volume, playbackRate, properties, false); + const bool success = createWallpaper(type, fillMode, absolutePath, previewImage, file, monitors, volume, playbackRate, properties, false); + + if (!success) { + qWarning() << "Unable to start Wallpaper! " << type << fillMode << monitors << absolutePath; + } } for (const QJsonValueRef widget : wallpaper.toObject().value("widgets").toArray()) { @@ -573,10 +615,14 @@ void ScreenPlayManager::loadProfiles() const auto type = QStringToEnum(typeString, InstalledType::InstalledType::QMLWidget); const QJsonObject properties = widgetObj.value("properties").toObject(); - qInfo() << "Start widget from profile:" << type << position << absolutePath; - createWidget(type, position, absolutePath, previewImage, properties, false); + const bool success = createWidget(type, position, absolutePath, previewImage, properties, false); + + if (!success) { + qWarning() << "Unable to start Widget! " << type << position << absolutePath; + } } } + return true; } TEST_CASE("Loads profiles.json") diff --git a/ScreenPlay/src/screenplaymanager.h b/ScreenPlay/src/screenplaymanager.h index b5509161..4550d10b 100644 --- a/ScreenPlay/src/screenplaymanager.h +++ b/ScreenPlay/src/screenplaymanager.h @@ -88,7 +88,7 @@ signals: void displayErrorPopup(const QString& msg); private slots: - void saveProfiles(); + bool saveProfiles(); public slots: // moc needs full enum namespace info see QTBUG-58454 @@ -116,10 +116,10 @@ public slots: bool removeAllWidgets(); bool removeWallpaperAt(const int index); - void requestProjectSettingsAtMonitorIndex(const int index); + bool requestProjectSettingsAtMonitorIndex(const int index); bool setWallpaperValueAtMonitorIndex(const int index, const QString& key, const QString& value); - void setAllWallpaperValue(const QString& key, const QString& value); - void setWallpaperValue(const QString& appID, const QString& key, const QString& value); + bool setAllWallpaperValue(const QString& key, const QString& value); + bool setWallpaperValue(const QString& appID, const QString& key, const QString& value); ScreenPlayWallpaper* getWallpaperByAppID(const QString& appID) const; void newConnection(); @@ -173,7 +173,7 @@ public slots: } private: - void loadProfiles(); + bool loadProfiles(); bool checkIsAnotherScreenPlayInstanceRunning(); bool removeWallpaper(const QString& appID); bool removeWidget(const QString& appID); diff --git a/ScreenPlay/src/screenplaywallpaper.cpp b/ScreenPlay/src/screenplaywallpaper.cpp index 2edde2d5..143b013e 100644 --- a/ScreenPlay/src/screenplaywallpaper.cpp +++ b/ScreenPlay/src/screenplaywallpaper.cpp @@ -16,7 +16,7 @@ namespace ScreenPlay { ScreenPlayWallpaper::~ScreenPlayWallpaper() { qInfo() << "Remove wallpaper " << m_appID; - m_connection->close(); + m_connection->close(); } /*! @@ -168,7 +168,7 @@ void ScreenPlayWallpaper::processError(QProcess::ProcessError error) playbackRate or fillMode. Otherwise it is a simple key, value json pair. */ -void ScreenPlayWallpaper::setWallpaperValue(const QString& key, const QString& value, const bool save) +bool ScreenPlayWallpaper::setWallpaperValue(const QString& key, const QString& value, const bool save) { QJsonObject obj; obj.insert(key, value); @@ -183,10 +183,12 @@ void ScreenPlayWallpaper::setWallpaperValue(const QString& key, const QString& v setFillMode(QStringToEnum(value, FillMode::FillMode::Cover)); } - m_connection->sendMessage(QJsonDocument(obj).toJson(QJsonDocument::Compact)); + const bool success = m_connection->sendMessage(QJsonDocument(obj).toJson(QJsonDocument::Compact)); - if (save) + if (save && success) emit requestSave(); + + return success; } /*! diff --git a/ScreenPlay/src/screenplaywallpaper.h b/ScreenPlay/src/screenplaywallpaper.h index 89d57ffe..88cc30f5 100644 --- a/ScreenPlay/src/screenplaywallpaper.h +++ b/ScreenPlay/src/screenplaywallpaper.h @@ -140,7 +140,7 @@ signals: public slots: void processExit(int exitCode, QProcess::ExitStatus exitStatus); void processError(QProcess::ProcessError error); - void setWallpaperValue(const QString& key, const QString& value, const bool save = false); + bool setWallpaperValue(const QString& key, const QString& value, const bool save = false); void setScreenNumber(QVector screenNumber) { diff --git a/ScreenPlay/src/sdkconnection.cpp b/ScreenPlay/src/sdkconnection.cpp index 4e1aebc4..23af48bd 100644 --- a/ScreenPlay/src/sdkconnection.cpp +++ b/ScreenPlay/src/sdkconnection.cpp @@ -88,10 +88,10 @@ void ScreenPlay::SDKConnection::readyRead() /*! \brief Sends a message to the connected socket. */ -void ScreenPlay::SDKConnection::sendMessage(const QByteArray& message) +bool ScreenPlay::SDKConnection::sendMessage(const QByteArray& message) { m_socket->write(message); - m_socket->waitForBytesWritten(); + return m_socket->waitForBytesWritten(); } /*! diff --git a/ScreenPlay/src/sdkconnection.h b/ScreenPlay/src/sdkconnection.h index 39cea72b..15c669bd 100644 --- a/ScreenPlay/src/sdkconnection.h +++ b/ScreenPlay/src/sdkconnection.h @@ -93,7 +93,7 @@ signals: public slots: void readyRead(); - void sendMessage(const QByteArray& message); + bool sendMessage(const QByteArray& message); bool close(); void setAppID(QString appID) diff --git a/ScreenPlayWallpaper/main.cpp b/ScreenPlayWallpaper/main.cpp index 329275c1..45690b0c 100644 --- a/ScreenPlayWallpaper/main.cpp +++ b/ScreenPlayWallpaper/main.cpp @@ -54,7 +54,7 @@ int main(int argc, char* argv[]) return -3; } - const bool debugMode = true; + const bool debugMode = false; const auto activeScreensList = ScreenPlayUtil::parseStringToIntegerList(argumentList.at(1));