From 6aaab68cdc1b463cc2ae37d9bf49ee2b531f8a23 Mon Sep 17 00:00:00 2001 From: Michael Zanetti Date: Wed, 13 May 2020 22:55:29 +0200 Subject: [PATCH 1/2] Retry setup if it fails at startup (e.g. network isn't up yet or similar) --- .../thingmanagerimplementation.cpp | 51 ++++++++++++------- .../integrations/thingmanagerimplementation.h | 2 +- 2 files changed, 33 insertions(+), 20 deletions(-) diff --git a/libnymea-core/integrations/thingmanagerimplementation.cpp b/libnymea-core/integrations/thingmanagerimplementation.cpp index 62c1c304..421a6bcc 100644 --- a/libnymea-core/integrations/thingmanagerimplementation.cpp +++ b/libnymea-core/integrations/thingmanagerimplementation.cpp @@ -1565,25 +1565,7 @@ void ThingManagerImplementation::loadConfiguredThings() } Q_ASSERT(thing != nullptr); - thing->setSetupStatus(Thing::ThingSetupStatusInProgress, Thing::ThingErrorNoError); - ThingSetupInfo *info = setupThing(thing); - // Set receiving object to "thing" because at startup we load it in any case, knowing that it worked at - // some point. However, it'll be marked as non-working until the setup succeeds so the user might delete - // it in the meantime... In that case we don't want to call postsetup on it. - connect(info, &ThingSetupInfo::finished, thing, [this, info](){ - - if (info->status() != Thing::ThingErrorNoError) { - qCWarning(dcThingManager()) << "Error setting up thing" << info->thing()->name() << info->thing()->id().toString() << info->status() << info->displayMessage(); - info->thing()->setSetupStatus(Thing::ThingSetupStatusFailed, info->status(), info->displayMessage()); - emit thingChanged(info->thing()); - return; - } - - qCDebug(dcThingManager()) << "Setup complete for thing" << info->thing(); - info->thing()->setSetupStatus(Thing::ThingSetupStatusComplete, Thing::ThingErrorNoError); - emit thingChanged(info->thing()); - postSetupThing(info->thing()); - }); + trySetupThing(thing); } loadIOConnections(); @@ -2101,6 +2083,37 @@ QVariant ThingManagerImplementation::mapValue(const QVariant &value, const State return toValue; } +void ThingManagerImplementation::trySetupThing(Thing *thing) +{ + thing->setSetupStatus(Thing::ThingSetupStatusInProgress, Thing::ThingErrorNoError); + ThingSetupInfo *info = setupThing(thing); + // Set receiving object to "thing" because at startup we load it in any case, knowing that it worked at + // some point. However, it'll be marked as non-working until the setup succeeds so the user might delete + // it in the meantime... In that case we don't want to call postsetup on it. + connect(info, &ThingSetupInfo::finished, thing, [this, info, thing](){ + + if (info->status() != Thing::ThingErrorNoError) { + qCWarning(dcThingManager()) << "Error setting up thing" << info->thing()->name() << info->thing()->id().toString() << info->status() << info->displayMessage(); + info->thing()->setSetupStatus(Thing::ThingSetupStatusFailed, info->status(), info->displayMessage()); + emit thingChanged(info->thing()); + + // We know this used to work at some point... try again in a bit unless we don't have a plugin for it... + if (info->status() != Thing::ThingErrorPluginNotFound) { + QTimer::singleShot(10000, thing, [this, thing]() { + trySetupThing(thing); + }); + } + + return; + } + + qCDebug(dcThingManager()) << "Setup complete for thing" << info->thing(); + info->thing()->setSetupStatus(Thing::ThingSetupStatusComplete, Thing::ThingErrorNoError); + emit thingChanged(info->thing()); + postSetupThing(info->thing()); + }); +} + void ThingManagerImplementation::storeThingStates(Thing *thing) { NymeaSettings settings(NymeaSettings::SettingsRoleThingStates); diff --git a/libnymea-core/integrations/thingmanagerimplementation.h b/libnymea-core/integrations/thingmanagerimplementation.h index ce001e48..6e7b1d39 100644 --- a/libnymea-core/integrations/thingmanagerimplementation.h +++ b/libnymea-core/integrations/thingmanagerimplementation.h @@ -155,11 +155,11 @@ private: ThingSetupInfo *reconfigureThingInternal(Thing *thing, const ParamList ¶ms, const QString &name = QString()); ThingSetupInfo *setupThing(Thing *thing); void postSetupThing(Thing *thing); + void trySetupThing(Thing *thing); void storeThingStates(Thing *thing); void loadThingStates(Thing *thing); void storeIOConnections(); void loadIOConnections(); - void syncIOConnection(Thing *inputThing, const StateTypeId &stateTypeId); QVariant mapValue(const QVariant &value, const StateType &fromStateType, const StateType &toStateType, bool inverted) const; From 8fee1bb2e5dfa797aa1d72a9fa5d5decc61bb700 Mon Sep 17 00:00:00 2001 From: Michael Zanetti Date: Wed, 26 Aug 2020 11:21:28 +0200 Subject: [PATCH 2/2] Fix duplicate connections on case of retrying --- .../thingmanagerimplementation.cpp | 45 ++++++++++--------- .../integrations/thingmanagerimplementation.h | 4 +- libnymea-core/jsonrpc/statehandler.cpp | 1 + tests/auto/devices/testdevices.cpp | 2 +- 4 files changed, 29 insertions(+), 23 deletions(-) diff --git a/libnymea-core/integrations/thingmanagerimplementation.cpp b/libnymea-core/integrations/thingmanagerimplementation.cpp index 421a6bcc..f8bf1184 100644 --- a/libnymea-core/integrations/thingmanagerimplementation.cpp +++ b/libnymea-core/integrations/thingmanagerimplementation.cpp @@ -642,14 +642,13 @@ ThingPairingInfo *ThingManagerImplementation::confirmPairing(const PairingTransa if (addNewThing) { qCDebug(dcThingManager()) << "Thing added:" << info->thing(); - m_configuredThings.insert(info->thing()->id(), info->thing()); + registerThing(info->thing()); emit thingAdded(info->thing()); - connect(info->thing(), &Thing::eventTriggered, this, &ThingManagerImplementation::onEventTriggered); } else { emit thingChanged(info->thing()); } - storeConfiguredThings(); + postSetupThing(info->thing()); }); @@ -723,11 +722,9 @@ ThingSetupInfo* ThingManagerImplementation::addConfiguredThingInternal(const Thi info->thing()->setSetupStatus(Thing::ThingSetupStatusComplete, Thing::ThingErrorNoError); qCDebug(dcThingManager) << "Thing setup complete."; - m_configuredThings.insert(info->thing()->id(), info->thing()); + registerThing(info->thing()); storeConfiguredThings(); - emit thingAdded(info->thing()); - connect(info->thing(), &Thing::eventTriggered, this, &ThingManagerImplementation::onEventTriggered); postSetupThing(info->thing()); }); @@ -1539,11 +1536,9 @@ void ThingManagerImplementation::loadConfiguredThings() // We always add the thing to the list in this case. If it's in the stored things // it means that it was working at some point so lets still add it as there might // be rules associated with this thing. - m_configuredThings.insert(thing->id(), thing); + registerThing(thing); emit thingAdded(thing); - - connect(thing, &Thing::eventTriggered, this, &ThingManagerImplementation::onEventTriggered); } settings.endGroup(); @@ -1666,11 +1661,9 @@ void ThingManagerImplementation::onAutoThingsAppeared(const ThingDescriptors &th } info->thing()->setSetupStatus(Thing::ThingSetupStatusComplete, Thing::ThingErrorNoError); - m_configuredThings.insert(info->thing()->id(), info->thing()); + registerThing(info->thing()); storeConfiguredThings(); - emit thingAdded(info->thing()); - connect(info->thing(), &Thing::eventTriggered, this, &ThingManagerImplementation::onEventTriggered); postSetupThing(info->thing()); }); } @@ -1746,7 +1739,7 @@ void ThingManagerImplementation::slotThingStateValueChanged(const StateTypeId &s qCWarning(dcThingManager()) << "Invalid thing id in state change. Not forwarding event. Thing setup not complete yet?"; return; } - storeThingStates(thing); + storeThingState(thing, stateTypeId); emit thingStateChanged(thing, stateTypeId, value); @@ -1976,10 +1969,6 @@ ThingSetupInfo* ThingManagerImplementation::setupThing(Thing *thing) thing->setStates(states); loadThingStates(thing); - connect(thing, &Thing::stateValueChanged, this, &ThingManagerImplementation::slotThingStateValueChanged); - connect(thing, &Thing::settingChanged, this, &ThingManagerImplementation::slotThingSettingChanged); - connect(thing, &Thing::nameChanged, this, &ThingManagerImplementation::slotThingNameChanged); - ThingSetupInfo *info = new ThingSetupInfo(thing, this, 30000); if (!plugin) { @@ -2114,14 +2103,28 @@ void ThingManagerImplementation::trySetupThing(Thing *thing) }); } +void ThingManagerImplementation::registerThing(Thing *thing) +{ + m_configuredThings.insert(thing->id(), thing); + connect(thing, &Thing::eventTriggered, this, &ThingManagerImplementation::onEventTriggered); + connect(thing, &Thing::stateValueChanged, this, &ThingManagerImplementation::slotThingStateValueChanged); + connect(thing, &Thing::settingChanged, this, &ThingManagerImplementation::slotThingSettingChanged); + connect(thing, &Thing::nameChanged, this, &ThingManagerImplementation::slotThingNameChanged); +} + void ThingManagerImplementation::storeThingStates(Thing *thing) +{ + ThingClass thingClass = m_supportedThings.value(thing->thingClassId()); + foreach (const StateType &stateType, thingClass.stateTypes()) { + storeThingState(thing, stateType.id()); + } +} + +void ThingManagerImplementation::storeThingState(Thing *thing, const StateTypeId &stateTypeId) { NymeaSettings settings(NymeaSettings::SettingsRoleThingStates); settings.beginGroup(thing->id().toString()); - ThingClass thingClass = m_supportedThings.value(thing->thingClassId()); - foreach (const StateType &stateType, thingClass.stateTypes()) { - settings.setValue(stateType.id().toString(), thing->stateValue(stateType.id())); - } + settings.setValue(stateTypeId.toString(), thing->stateValue(stateTypeId)); settings.endGroup(); } diff --git a/libnymea-core/integrations/thingmanagerimplementation.h b/libnymea-core/integrations/thingmanagerimplementation.h index 6e7b1d39..ed6c4060 100644 --- a/libnymea-core/integrations/thingmanagerimplementation.h +++ b/libnymea-core/integrations/thingmanagerimplementation.h @@ -154,9 +154,11 @@ private: ThingSetupInfo *addConfiguredThingInternal(const ThingClassId &thingClassId, const QString &name, const ParamList ¶ms, const ThingId &parentId = ThingId()); ThingSetupInfo *reconfigureThingInternal(Thing *thing, const ParamList ¶ms, const QString &name = QString()); ThingSetupInfo *setupThing(Thing *thing); - void postSetupThing(Thing *thing); void trySetupThing(Thing *thing); + void registerThing(Thing *thing); + void postSetupThing(Thing *thing); void storeThingStates(Thing *thing); + void storeThingState(Thing *thing, const StateTypeId &stateTypeId); void loadThingStates(Thing *thing); void storeIOConnections(); void loadIOConnections(); diff --git a/libnymea-core/jsonrpc/statehandler.cpp b/libnymea-core/jsonrpc/statehandler.cpp index 64821d0e..b428148b 100644 --- a/libnymea-core/jsonrpc/statehandler.cpp +++ b/libnymea-core/jsonrpc/statehandler.cpp @@ -54,6 +54,7 @@ StateHandler::StateHandler(QObject *parent) : JsonHandler(parent) { registerEnum(); + registerEnum(); registerObject(); registerObject(); diff --git a/tests/auto/devices/testdevices.cpp b/tests/auto/devices/testdevices.cpp index bc2f5d4c..dc1d88d6 100644 --- a/tests/auto/devices/testdevices.cpp +++ b/tests/auto/devices/testdevices.cpp @@ -2071,7 +2071,7 @@ void TestDevices::triggerEvent() // Check for the notification on JSON API QVariantList notifications; notifications = checkNotifications(notificationSpy, "Devices.EventTriggered"); - QVERIFY2(notifications.count() == 1, "Should get Devices.EventTriggered notification"); + QVERIFY2(notifications.count() == 1, QString("Should get Devices.EventTriggered notification but got: %1").arg(qUtf8Printable(QJsonDocument::fromVariant(notifications).toJson())).toUtf8()); QVariantMap notificationContent = notifications.first().toMap().value("params").toMap(); QCOMPARE(notificationContent.value("event").toMap().value("deviceId").toUuid().toString(), device->id().toString());