diff --git a/libnymea-core/integrations/thingmanagerimplementation.cpp b/libnymea-core/integrations/thingmanagerimplementation.cpp index 2a60222d..e9238265 100644 --- a/libnymea-core/integrations/thingmanagerimplementation.cpp +++ b/libnymea-core/integrations/thingmanagerimplementation.cpp @@ -454,6 +454,10 @@ ThingSetupInfo *ThingManagerImplementation::reconfigureThingInternal(Thing *thin // try to setup the thing with the new params ThingSetupInfo *info = new ThingSetupInfo(thing, this, 30000); plugin->setupThing(info); + connect(info, &ThingSetupInfo::destroyed, thing, [=](){ + m_pendingSetups.remove(thing->id()); + }); + connect(info, &ThingSetupInfo::finished, this, [this, info](){ if (info->status() != Thing::ThingErrorNoError) { @@ -824,6 +828,11 @@ ThingSetupInfo* ThingManagerImplementation::addConfiguredThingInternal(const Thi Thing::ThingError ThingManagerImplementation::removeConfiguredThing(const ThingId &thingId) { + // We're checking thingSetupStatus and abort any pending setup here. As setup finished() + // comes in as a QueuedConnection, make sure to process all events before going on so we + // don't end up aborting an already finished setup instead of calling thingRemoved() on it. + qApp->processEvents(); + Thing *thing = m_configuredThings.take(thingId); if (!thing) { return Thing::ThingErrorThingNotFound; @@ -831,7 +840,11 @@ Thing::ThingError ThingManagerImplementation::removeConfiguredThing(const ThingI IntegrationPlugin *plugin = m_integrationPlugins.value(thing->pluginId()); if (!plugin) { qCWarning(dcThingManager()).nospace() << "Plugin not loaded for thing " << thing->name() << ". Not calling thingRemoved on plugin."; - } else { + } else if (thing->setupStatus() == Thing::ThingSetupStatusInProgress) { + qCWarning(dcThingManager()).nospace() << "Thing " << thing->name() << " is still being set up. Aborting setup."; + ThingSetupInfo *setupInfo = m_pendingSetups.value(thingId); + emit setupInfo->aborted(); + } else if (thing->setupStatus() == Thing::ThingSetupStatusComplete) { plugin->thingRemoved(thing); } @@ -2070,6 +2083,11 @@ ThingSetupInfo* ThingManagerImplementation::setupThing(Thing *thing) plugin->setupThing(info); + m_pendingSetups.insert(thing->id(), info); + connect(info, &ThingSetupInfo::destroyed, thing, [=](){ + m_pendingSetups.remove(thing->id()); + }); + return info; } diff --git a/libnymea-core/integrations/thingmanagerimplementation.h b/libnymea-core/integrations/thingmanagerimplementation.h index 61f87407..33e539d5 100644 --- a/libnymea-core/integrations/thingmanagerimplementation.h +++ b/libnymea-core/integrations/thingmanagerimplementation.h @@ -194,6 +194,7 @@ private: QString thingName; }; QHash m_pendingPairings; + QHash m_pendingSetups; QHash m_ioConnections; diff --git a/plugins/mock/integrationpluginmock.cpp b/plugins/mock/integrationpluginmock.cpp index 7acca96f..f1ae5b7d 100644 --- a/plugins/mock/integrationpluginmock.cpp +++ b/plugins/mock/integrationpluginmock.cpp @@ -134,7 +134,7 @@ void IntegrationPluginMock::setupThing(ThingSetupInfo *info) { if (info->thing()->thingClassId() == mockThingClassId || info->thing()->thingClassId() == autoMockThingClassId) { if (m_daemons.contains(info->thing())) { - // We already have a daemon, seem's we're reconfiguring + // We already have a daemon, seems we're reconfiguring delete m_daemons.take(info->thing()); } @@ -174,8 +174,7 @@ void IntegrationPluginMock::setupThing(ThingSetupInfo *info) if (async) { - Thing *device = info->thing(); - QTimer::singleShot(1000, device, [info](){ + QTimer::singleShot(1000, info, [info](){ qCDebug(dcMock()) << "Finishing thing setup for mocked thing" << info->thing()->name(); if (info->thing()->paramValue(mockThingBrokenParamTypeId).toBool()) { info->finish(Thing::ThingErrorSetupFailed, QT_TR_NOOP("This mocked thing is intentionally broken.")); @@ -183,6 +182,12 @@ void IntegrationPluginMock::setupThing(ThingSetupInfo *info) info->finish(Thing::ThingErrorNoError); } }); + + connect(info, &ThingSetupInfo::aborted, this, [=](){ + qCDebug(dcMock()) << "Setup aborted. Destroying webserver" << info->thing()->name(); + delete m_daemons.take(info->thing()); + }); + return; } qCDebug(dcMock()) << "Setup complete" << info->thing()->name(); @@ -260,24 +265,25 @@ void IntegrationPluginMock::setupThing(ThingSetupInfo *info) info->finish(Thing::ThingErrorThingClassNotFound); } -void IntegrationPluginMock::postSetupThing(Thing *device) +void IntegrationPluginMock::postSetupThing(Thing *thing) { - qCDebug(dcMock()) << "Postsetup mock" << device->name(); - if (device->thingClassId() == parentMockThingClassId) { + qCDebug(dcMock()) << "Postsetup mock" << thing->name(); + if (thing->thingClassId() == parentMockThingClassId) { foreach (Thing *d, myThings()) { - if (d->thingClassId() == childMockThingClassId && d->parentId() == device->id()) { + if (d->thingClassId() == childMockThingClassId && d->parentId() == thing->id()) { return; } } - ThingDescriptor mockDescriptor(childMockThingClassId, "Mocked Thing Child (Auto created)", "Mocked Thing Child (Auto created)", device->id()); + ThingDescriptor mockDescriptor(childMockThingClassId, "Mocked Thing Child (Auto created)", "Mocked Thing Child (Auto created)", thing->id()); emit autoThingsAppeared(ThingDescriptors() << mockDescriptor); } } -void IntegrationPluginMock::thingRemoved(Thing *device) +void IntegrationPluginMock::thingRemoved(Thing *thing) { - delete m_daemons.take(device); + qCDebug(dcMock()) << "Thing removed" << thing->name(); + delete m_daemons.take(thing); } void IntegrationPluginMock::startMonitoringAutoThings() diff --git a/plugins/mock/integrationpluginmock.h b/plugins/mock/integrationpluginmock.h index b91d45a6..8e09c13e 100644 --- a/plugins/mock/integrationpluginmock.h +++ b/plugins/mock/integrationpluginmock.h @@ -51,8 +51,8 @@ public: void discoverThings(ThingDiscoveryInfo *info) override; void setupThing(ThingSetupInfo *info) override; - void postSetupThing(Thing *device) override; - void thingRemoved(Thing *device) override; + void postSetupThing(Thing *thing) override; + void thingRemoved(Thing *thing) override; void startMonitoringAutoThings() override; diff --git a/tests/auto/devices/testdevices.cpp b/tests/auto/devices/testdevices.cpp index 1c65300b..f310edf2 100644 --- a/tests/auto/devices/testdevices.cpp +++ b/tests/auto/devices/testdevices.cpp @@ -160,7 +160,7 @@ void TestDevices::initTestCase() NymeaTestBase::initTestCase(); QLoggingCategory::setFilterRules("*.debug=false\n" "Tests.debug=true\n" - "MockDevice.debug=true\n" + "Mock.debug=true\n" ); // Adding an async mock device to be used in tests below @@ -619,6 +619,7 @@ void TestDevices::storedDevices() params.clear(); params.insert("deviceId", addedDeviceId); + qCDebug(dcTests()) << "Cleaning up mock device again"; response = injectAndWait("Devices.RemoveConfiguredDevice", params); verifyDeviceError(response); }