Add a deviceId to DeviceDescriptors to allow reconfiguring them

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 &params)
     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); });

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;

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.
+    // If we already have a deviceId, we're reconfiguring an existing device
-    DeviceId id = DeviceId::createDeviceId();
+    bool addNewDevice = deviceId.isNull();
-    emit pairingFinished(pairingTransactionId, DeviceErrorNoError, id);
 
-    QList<DeviceId> newDevices;
+    if (!addNewDevice && !m_configuredDevices.contains(deviceId)) {
-    Device *device = new Device(plugin->pluginId(), id, deviceClassId, this);
+        qCWarning(dcDeviceManager) << "The device to be reconfigured has disappeared!";
-    if (deviceName.isEmpty()) {
+        emit pairingFinished(pairingTransactionId, DeviceErrorDeviceNotFound, deviceId);
-        device->setName(deviceClass.name());
+        return;
-    } else {
-        device->setName(deviceName);
     }
+
+    // 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);
+    if (addNewDevice) {
-    emit deviceAdded(device);
+        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);

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
 {

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;

libnymea/plugin/deviceplugin.cpp

@@ -32,8 +32,11 @@
 
 
 /*! \fn void DevicePlugin::devicesDiscovered(const DeviceClassId &deviceClassId, const QList<DeviceDescriptor> &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()
 */
 

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);
     }
 

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"
             ],

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()
 {