diff --git a/libnymea-core/jsonrpc/devicehandler.cpp b/libnymea-core/jsonrpc/devicehandler.cpp index 3608cb6d..811a7b9f 100644 --- a/libnymea-core/jsonrpc/devicehandler.cpp +++ b/libnymea-core/jsonrpc/devicehandler.cpp @@ -170,7 +170,11 @@ DeviceHandler::DeviceHandler(QObject *parent) : setReturns("GetConfiguredDevices", returns); params.clear(); returns.clear(); - setDescription("GetDiscoveredDevices", "Performs a device discovery and returns the results. This function may take a while to return."); + setDescription("GetDiscoveredDevices", "Performs a device discovery and returns the results. This function may take a while to return. " + "Note that this method will include all the found devices, that is, including devices that may " + "already have been added. Those devices will have deviceId set to the device id of the already " + "added device. Such results may be used to reconfigure existing devices and might be filtered " + "in cases where only unknown devices are of interest."); params.insert("deviceClassId", JsonTypes::basicTypeToString(JsonTypes::Uuid)); QVariantList discoveryParams; discoveryParams.append(JsonTypes::paramRef()); @@ -446,7 +450,7 @@ JsonReply *DeviceHandler::ConfirmPairing(const QVariantMap ¶ms) QString secret = params.value("secret").toString(); DeviceManager::DeviceError status = NymeaCore::instance()->deviceManager()->confirmPairing(pairingTransactionId, secret); - JsonReply *reply = 0; + JsonReply *reply = nullptr; if (status == DeviceManager::DeviceErrorAsync) { reply = createAsyncReply("ConfirmPairing"); connect(reply, &JsonReply::finished, [this, pairingTransactionId](){ m_asyncPairingRequests.remove(pairingTransactionId); }); diff --git a/libnymea-core/jsonrpc/jsontypes.cpp b/libnymea-core/jsonrpc/jsontypes.cpp index 9d80a86b..bae8181a 100644 --- a/libnymea-core/jsonrpc/jsontypes.cpp +++ b/libnymea-core/jsonrpc/jsontypes.cpp @@ -303,6 +303,7 @@ void JsonTypes::init() // DeviceDescriptor s_deviceDescriptor.insert("id", basicTypeToString(Uuid)); + s_deviceDescriptor.insert("deviceId", basicTypeToString(Uuid)); s_deviceDescriptor.insert("title", basicTypeToString(String)); s_deviceDescriptor.insert("description", basicTypeToString(String)); s_deviceDescriptor.insert("deviceParams", QVariantList() << paramRef()); @@ -861,6 +862,7 @@ QVariantMap JsonTypes::packDeviceDescriptor(const DeviceDescriptor &descriptor) { QVariantMap variant; variant.insert("id", descriptor.id()); + variant.insert("deviceId", descriptor.deviceId()); variant.insert("title", descriptor.title()); variant.insert("description", descriptor.description()); QVariantList params; diff --git a/libnymea/devicemanager.cpp b/libnymea/devicemanager.cpp index 21fcbf36..c7e2be1b 100644 --- a/libnymea/devicemanager.cpp +++ b/libnymea/devicemanager.cpp @@ -1317,6 +1317,8 @@ void DeviceManager::slotPairingFinished(const PairingTransactionId &pairingTrans ParamList params; QString deviceName; + DeviceId deviceId; + // Do this before checking status to make sure we clean up our hashes properly if (m_pairingsJustAdd.contains(pairingTransactionId)) { DevicePairingInfo pairingInfo = m_pairingsJustAdd.take(pairingTransactionId); @@ -1333,6 +1335,7 @@ void DeviceManager::slotPairingFinished(const PairingTransactionId &pairingTrans deviceClassId = pairingInfo.deviceClassId(); deviceName = pairingInfo.deviceName(); params = descriptor.params(); + deviceId = descriptor.deviceId(); } if (status != DeviceSetupStatusSuccess) { @@ -1348,17 +1351,33 @@ void DeviceManager::slotPairingFinished(const PairingTransactionId &pairingTrans return; } - // Ok... pairing went fine... Let consumers know about it and inform them about the ongoing setup with a deviceId. - DeviceId id = DeviceId::createDeviceId(); - emit pairingFinished(pairingTransactionId, DeviceErrorNoError, id); + // If we already have a deviceId, we're reconfiguring an existing device + bool addNewDevice = deviceId.isNull(); - QList newDevices; - Device *device = new Device(plugin->pluginId(), id, deviceClassId, this); - if (deviceName.isEmpty()) { - device->setName(deviceClass.name()); - } else { - device->setName(deviceName); + if (!addNewDevice && !m_configuredDevices.contains(deviceId)) { + qCWarning(dcDeviceManager) << "The device to be reconfigured has disappeared!"; + emit pairingFinished(pairingTransactionId, DeviceErrorDeviceNotFound, deviceId); + return; } + + // Ok... pairing went fine... Let consumers know about it and inform them about the ongoing setup with a deviceId. + Device *device = nullptr; + + if (addNewDevice) { + deviceId = DeviceId::createDeviceId(); + qCDebug(dcDeviceManager()) << "Creating new device with ID" << deviceId; + device = new Device(plugin->pluginId(), deviceId, deviceClassId, this); + if (deviceName.isEmpty()) { + device->setName(deviceClass.name()); + } else { + device->setName(deviceName); + } + } else { + qCDebug(dcDeviceManager()) << "Reconfiguring device" << device; + device = m_configuredDevices.value(deviceId); + } + emit pairingFinished(pairingTransactionId, DeviceErrorNoError, deviceId); + device->setParams(params); DeviceSetupStatus setupStatus = setupDevice(device); @@ -1372,12 +1391,17 @@ void DeviceManager::slotPairingFinished(const PairingTransactionId &pairingTrans return; case DeviceSetupStatusSuccess: qCDebug(dcDeviceManager) << "Device setup complete."; - newDevices.append(id); break; } - m_configuredDevices.insert(device->id(), device); - emit deviceAdded(device); + if (addNewDevice) { + qCDebug(dcDeviceManager()) << "Device added:" << device; + m_configuredDevices.insert(device->id(), device); + emit deviceAdded(device); + } else { + emit deviceChanged(device); + } + storeConfiguredDevices(); emit deviceSetupFinished(device, DeviceError::DeviceErrorNoError); postSetupDevice(device); diff --git a/libnymea/plugin/devicedescriptor.cpp b/libnymea/plugin/devicedescriptor.cpp index 1a42ffe9..654f5897 100644 --- a/libnymea/plugin/devicedescriptor.cpp +++ b/libnymea/plugin/devicedescriptor.cpp @@ -84,6 +84,18 @@ DeviceClassId DeviceDescriptor::deviceClassId() const return m_deviceClassId; } +/*! Returns the \a deviceId of the device matching this descriptor. */ +DeviceId DeviceDescriptor::deviceId() const +{ + return m_deviceId; +} + +/*! Set the \a deviceId of the device matching this device descriptor. */ +void DeviceDescriptor::setDeviceId(const DeviceId &deviceId) +{ + m_deviceId = deviceId; +} + /*! Returns the name of this DeviceDescriptor. */ QString DeviceDescriptor::title() const { diff --git a/libnymea/plugin/devicedescriptor.h b/libnymea/plugin/devicedescriptor.h index 7ed73dbc..3661ed65 100644 --- a/libnymea/plugin/devicedescriptor.h +++ b/libnymea/plugin/devicedescriptor.h @@ -42,6 +42,9 @@ public: DeviceDescriptorId id() const; DeviceClassId deviceClassId() const; + DeviceId deviceId() const; + void setDeviceId(const DeviceId &deviceId); + QString title() const; void setTitle(const QString &title); @@ -57,6 +60,7 @@ public: private: DeviceDescriptorId m_id; DeviceClassId m_deviceClassId; + DeviceId m_deviceId; QString m_title; QString m_description; DeviceId m_parentDeviceId; diff --git a/libnymea/plugin/deviceplugin.cpp b/libnymea/plugin/deviceplugin.cpp index f6752908..2e2fc74c 100644 --- a/libnymea/plugin/deviceplugin.cpp +++ b/libnymea/plugin/deviceplugin.cpp @@ -32,8 +32,11 @@ /*! \fn void DevicePlugin::devicesDiscovered(const DeviceClassId &deviceClassId, const QList &devices); - This signal is emitted when the discovery of a \a deviceClassId of this DevicePlugin is finished. The \a devices parameter describes the + Emit this signal when the discovery of a \a deviceClassId of this DevicePlugin is finished. The \a devices parameter describes the list of \l{DeviceDescriptor}{DeviceDescriptors} of all discovered \l{Device}{Devices}. + Note: During a discovery a plugin should always return the full result set. So even if a device is already known to the system and + a later discovery finds the device again, it should be included in the result set but the DeviceDescriptor's deviceId should be set + to the device ID. \sa discoverDevices() */ diff --git a/plugins/mock/devicepluginmock.cpp b/plugins/mock/devicepluginmock.cpp index 55d11559..7671bb4a 100644 --- a/plugins/mock/devicepluginmock.cpp +++ b/plugins/mock/devicepluginmock.cpp @@ -393,6 +393,12 @@ void DevicePluginMock::emitDevicesDiscovered() Param httpParam(mockDeviceHttpportParamTypeId, "55555"); params.append(httpParam); d1.setParams(params); + foreach (Device *d, myDevices()) { + if (d->deviceClassId() == mockDeviceClassId && d->paramValue(mockDeviceHttpportParamTypeId).toInt() == 55555) { + d1.setDeviceId(d->id()); + break; + } + } deviceDescriptors.append(d1); } @@ -402,6 +408,12 @@ void DevicePluginMock::emitDevicesDiscovered() Param httpParam(mockDeviceHttpportParamTypeId, "55556"); params.append(httpParam); d2.setParams(params); + foreach (Device *d, myDevices()) { + if (d->deviceClassId() == mockDeviceClassId && d->paramValue(mockDeviceHttpportParamTypeId).toInt() == 55556) { + d2.setDeviceId(d->id()); + break; + } + } deviceDescriptors.append(d2); } @@ -434,11 +446,24 @@ void DevicePluginMock::emitDisplayPinDevicesDiscovered() if (m_discoveredDeviceCount > 0) { DeviceDescriptor d1(mockDisplayPinDeviceClassId, "Mock Device (Display Pin)", "1"); + foreach (Device *existingDev, myDevices()) { + if (existingDev->deviceClassId() == mockDisplayPinDeviceClassId) { + d1.setDeviceId(existingDev->id()); + break; + } + } deviceDescriptors.append(d1); } if (m_discoveredDeviceCount > 1) { DeviceDescriptor d2(mockDisplayPinDeviceClassId, "Mock Device (Display Pin)", "2"); + int count = 0; + foreach (Device *existingDev, myDevices()) { + if (existingDev->deviceClassId() == mockDisplayPinDeviceClassId && ++count > 1) { + d2.setDeviceId(existingDev->id()); + break; + } + } deviceDescriptors.append(d2); } diff --git a/tests/auto/api.json b/tests/auto/api.json index 3dd4a45f..9e7440df 100644 --- a/tests/auto/api.json +++ b/tests/auto/api.json @@ -1,4 +1,4 @@ -1.12 +1.13 { "methods": { "Actions.ExecuteAction": { @@ -293,7 +293,7 @@ } }, "Devices.GetDiscoveredDevices": { - "description": "Performs a device discovery and returns the results. This function may take a while to return.", + "description": "Performs a device discovery and returns the results. This function may take a while to return. Note that this method will include all the found devices, that is, including devices that may already have been added. Those devices will have deviceId set to the device id of the already added device. Such results may be used to reconfigure existing devices and might be filtered in cases where only unknown devices are of interest.", "params": { "deviceClassId": "Uuid", "o:discoveryParams": [ @@ -1266,6 +1266,7 @@ }, "DeviceDescriptor": { "description": "String", + "deviceId": "Uuid", "deviceParams": [ "$ref:Param" ], diff --git a/tests/auto/devices/testdevices.cpp b/tests/auto/devices/testdevices.cpp index 7c1c81da..39087bff 100644 --- a/tests/auto/devices/testdevices.cpp +++ b/tests/auto/devices/testdevices.cpp @@ -90,6 +90,8 @@ private slots: void reconfigureByDiscovery_data(); void reconfigureByDiscovery(); + void reconfigureByDiscoveryAndPair(); + void removeDevice_data(); void removeDevice(); @@ -1172,6 +1174,107 @@ void TestDevices::reconfigureByDiscovery() verifyDeviceError(response); } +void TestDevices::reconfigureByDiscoveryAndPair() +{ + QVariantList discoveryParams; + QVariantMap resultCountParam; + resultCountParam.insert("paramTypeId", resultCountParamTypeId); + resultCountParam.insert("value", 1); + discoveryParams.append(resultCountParam); + + qCDebug(dcTests()) << "Discovering devices..."; + + QVariantMap params; + params.insert("deviceClassId", mockDisplayPinDeviceClassId); + params.insert("discoveryParams", discoveryParams); + QVariant response = injectAndWait("Devices.GetDiscoveredDevices", params); + + verifyDeviceError(response); + QVariantList deviceDescriptors = response.toMap().value("params").toMap().value("deviceDescriptors").toList(); + + qCDebug(dcTests()) << "Discovery result:" << qUtf8Printable(QJsonDocument::fromVariant(deviceDescriptors).toJson(QJsonDocument::Indented)); + QCOMPARE(response.toMap().value("params").toMap().value("deviceDescriptors").toList().count(), 1); + + // add Discovered Device 1 port 55555 + + QVariant descriptor = deviceDescriptors.first(); + DeviceDescriptorId descriptorId = DeviceDescriptorId(descriptor.toMap().value("id").toString()); + QVERIFY2(!descriptorId.isNull(), "DeviceDescriptorId is Null"); + + qCDebug(dcTests()) << "Pairing descriptorId:" << descriptorId; + + params.clear(); + response.clear(); + params.insert("deviceClassId", mockDisplayPinDeviceClassId); + params.insert("name", "Discoverd mock device"); + params.insert("deviceDescriptorId", descriptorId); + response = injectAndWait("Devices.PairDevice", params); + verifyDeviceError(response); + + PairingTransactionId pairingTransactionId = PairingTransactionId(response.toMap().value("params").toMap().value("pairingTransactionId").toString()); + qCDebug(dcTests()) << "PairDevice result:" << qUtf8Printable(QJsonDocument::fromVariant(response).toJson(QJsonDocument::Indented)); + + qCDebug(dcTests()) << "Confirming pairing for transaction ID" << pairingTransactionId; + params.clear(); + response.clear(); + params.insert("pairingTransactionId", pairingTransactionId.toString()); + params.insert("secret", "243681"); + response = injectAndWait("Devices.ConfirmPairing", params); + verifyDeviceError(response); + + DeviceId deviceId(response.toMap().value("params").toMap().value("deviceId").toString()); + QVERIFY(!deviceId.isNull()); + + qCDebug(dcTests()) << "Discovering again..."; + + // and now rediscover, and edit the first device with the second + params.clear(); + response.clear(); + params.insert("deviceClassId", mockDisplayPinDeviceClassId); + params.insert("discoveryParams", discoveryParams); + response = injectAndWait("Devices.GetDiscoveredDevices", params); + + deviceDescriptors = response.toMap().value("params").toMap().value("deviceDescriptors").toList(); + qCDebug(dcTests()) << "Discovery result:" << qUtf8Printable(QJsonDocument::fromVariant(deviceDescriptors).toJson(QJsonDocument::Indented)); + + verifyDeviceError(response, DeviceManager::DeviceErrorNoError); + QCOMPARE(deviceDescriptors.count(), 1); + + descriptor = deviceDescriptors.first(); + QVERIFY2(DeviceId(descriptor.toMap().value("deviceId").toString()) == deviceId, "DeviceID not set in descriptor"); + + // get the descriptor again + descriptorId = DeviceDescriptorId(descriptor.toMap().value("id").toString()); + + QVERIFY(!descriptorId.isNull()); + + qDebug() << "Reconfiguring device by pairing again" << descriptorId; + + params.clear(); + response.clear(); + params.insert("deviceClassId", mockDisplayPinDeviceClassId); + params.insert("name", "Discoverd mock device"); + params.insert("deviceDescriptorId", descriptorId); + response = injectAndWait("Devices.PairDevice", params); + verifyDeviceError(response); + + pairingTransactionId = PairingTransactionId(response.toMap().value("params").toMap().value("pairingTransactionId").toString()); + qCDebug(dcTests()) << "PairDevice result:" << qUtf8Printable(QJsonDocument::fromVariant(response).toJson(QJsonDocument::Indented)); + + + qCDebug(dcTests()) << "Confirming pairing for transaction ID" << pairingTransactionId; + params.clear(); + response.clear(); + params.insert("pairingTransactionId", pairingTransactionId.toString()); + params.insert("secret", "243681"); + response = injectAndWait("Devices.ConfirmPairing", params); + verifyDeviceError(response); + + deviceId = DeviceId(response.toMap().value("params").toMap().value("deviceId").toString()); + QVERIFY(!deviceId.isNull()); + +} + void TestDevices::removeDevice_data() {