From 2c9d5ea4dbb8ed95377de3e1c8c8c4aab89bc56d Mon Sep 17 00:00:00 2001 From: Michael Zanetti Date: Sun, 31 Jan 2021 14:41:20 +0100 Subject: [PATCH] Fix group action executions Fixes: #522 --- libnymea-app/thinggroup.cpp | 85 ++++++++++++++------ libnymea-app/thinggroup.h | 9 +-- libnymea-app/types/device.cpp | 10 ++- libnymea-app/types/device.h | 5 ++ libnymea-app/types/interfaces.cpp | 2 +- nymea-app/ui/delegates/InterfaceTile.qml | 5 +- nymea-app/ui/devicepages/LightDevicePage.qml | 10 +-- nymea-app/ui/utils/ActionQueue.qml | 7 +- 8 files changed, 91 insertions(+), 42 deletions(-) diff --git a/libnymea-app/thinggroup.cpp b/libnymea-app/thinggroup.cpp index 8d93e143..d74379ee 100644 --- a/libnymea-app/thinggroup.cpp +++ b/libnymea-app/thinggroup.cpp @@ -36,7 +36,6 @@ ThingGroup::ThingGroup(DeviceManager *deviceManager, DeviceClass *deviceClass, DevicesProxy *devices, QObject *parent): Device(deviceManager, deviceClass, QUuid::createUuid(), parent), - m_thingManager(deviceManager), m_things(devices) { deviceClass->setParent(this); @@ -45,7 +44,7 @@ ThingGroup::ThingGroup(DeviceManager *deviceManager, DeviceClass *deviceClass, D for (int i = 0; i < deviceClass->stateTypes()->rowCount(); i++) { StateType *st = deviceClass->stateTypes()->get(i); State *state = new State(id(), st->id(), QVariant(), this); - qDebug() << "Adding state" << st->name(); + qDebug() << "Adding state" << st->name() << st->minValue() << st->maxValue(); states->addState(state); } setStates(states); @@ -56,14 +55,15 @@ ThingGroup::ThingGroup(DeviceManager *deviceManager, DeviceClass *deviceClass, D syncStates(); }); - connect(m_thingManager, &DeviceManager::executeActionReply, this, [this](int commandId, const QVariantMap &/*params*/){ - // This should maybe check the params and only emit NoError if there is at least one thing that returned without error? - foreach (int id, m_pendingActions.keys()) { - if (m_pendingActions.value(id).contains(commandId)) { - m_pendingActions[id].removeAll(commandId); - if (m_pendingActions[id].isEmpty()) { - m_pendingActions.remove(id); - emit actionExecutionFinished(id, "DeviceErrorNoError"); + connect(m_thingManager, &DeviceManager::executeActionReply, this, [this](int commandId, const QVariantMap ¶ms){ + // This should maybe check the params and create a sensible group result instead of just forwarding the result of the last reply + qDebug() << "action reply:" << commandId; + foreach (int id, m_pendingGroupActions.keys()) { + if (m_pendingGroupActions.value(id).contains(commandId)) { + m_pendingGroupActions[id].removeAll(commandId); + if (m_pendingGroupActions[id].isEmpty()) { + m_pendingGroupActions.remove(id); + emit executeActionReply(id, params); } return; } @@ -76,35 +76,51 @@ int ThingGroup::executeAction(const QString &actionName, const QVariantList &par { QList pendingIds; + ActionType *groupActionType = m_thingClass->actionTypes()->findByName(actionName); + if (!groupActionType) { + qWarning() << "Group has no action" << actionName; + return -1; + } + qDebug() << "Execute action for group:" << this; for (int i = 0; i < m_things->rowCount(); i++) { - Device *device = m_things->get(i); - if (device->setupStatus() != Device::ThingSetupStatusComplete) { + Device *thing = m_things->get(i); + if (thing->setupStatus() != Device::ThingSetupStatusComplete) { continue; } - ActionType *actionType = device->thingClass()->actionTypes()->findByName(actionName); + ActionType *actionType = thing->thingClass()->actionTypes()->findByName(actionName); if (!actionType) { + qWarning() << "Cannot send action to thing" << thing->name() << "because according action can't be found"; continue; } QVariantList finalParams; foreach (const QVariant ¶mVariant, params) { - ParamType *paramType = actionType->paramTypes()->findByName(paramVariant.toMap().value("paramName").toString()); - if (paramType) { - QVariantMap finalParam; - finalParam.insert("paramTypeId", paramType->id()); - finalParam.insert("value", paramVariant.toMap().value("value")); - finalParams.append(finalParam); + QString paramName = paramVariant.toMap().value("paramName").toString(); + ParamType *groupParamType = groupActionType->paramTypes()->findByName(paramName); + if (!groupParamType) { + qWarning() << "Not adding param" << paramName << "to action" << actionName << "because group action param can't be found"; + continue; } + ParamType *paramType = actionType->paramTypes()->findByName(paramName); + if (!paramType) { + qWarning() << "Not adding param" << paramName << "to action" << actionName << "because according action params can't be found"; + continue; + } + + QVariantMap finalParam; + finalParam.insert("paramTypeId", paramType->id()); + finalParam.insert("value", mapValue(paramVariant.toMap().value("value"), groupParamType, paramType)); + finalParams.append(finalParam); } qDebug() << "Initial params" << params; - qDebug() << "Executing" << device->id() << actionType->name() << finalParams; - int id = m_thingManager->executeAction(device->id(), actionType->id(), finalParams); + qDebug() << "Executing" << thing->id() << actionType->name() << finalParams; + int id = m_thingManager->executeAction(thing->id(), actionType->id(), finalParams); pendingIds.append(id); } - m_pendingActions.insert(++m_idCounter, pendingIds); + m_pendingGroupActions.insert(++m_idCounter, pendingIds); return m_idCounter; } @@ -114,7 +130,7 @@ void ThingGroup::syncStates() StateType *stateType = thingClass()->stateTypes()->get(i); State *state = states()->getState(stateType->id()); - qDebug() << "syncing state" << stateType->name(); + qDebug() << "syncing state" << stateType->name() << stateType->type(); QVariant value; int count = 0; @@ -142,6 +158,9 @@ void ThingGroup::syncStates() } else if (stateType->type().toLower() == "int") { value = value.toInt() + d->stateValue(ds->id()).toInt(); count++; + } else if (stateType->type().toLower() == "qcolor") { + value = d->stateValue(ds->id()); + break; } } if (count > 0) { @@ -150,3 +169,23 @@ void ThingGroup::syncStates() state->setValue(value); } } + +QVariant ThingGroup::mapValue(const QVariant &value, ParamType *fromParamType, ParamType *toParamType) const +{ + qDebug() << "Mapping:" << value << fromParamType->minValue() << fromParamType->maxValue() << toParamType->minValue() << toParamType->maxValue(); + + if (!fromParamType->minValue().isValid() + || !fromParamType->maxValue().isValid() + || !toParamType->minValue().isValid() + || !toParamType->maxValue().isValid()) { + return value; + } + double fromMin = fromParamType->minValue().toDouble(); + double fromMax = fromParamType->maxValue().toDouble(); + double toMin = toParamType->minValue().toDouble(); + double toMax = toParamType->maxValue().toDouble(); + double fromValue = value.toDouble(); + double fromPercent = (fromValue - fromMin) / (fromMax - fromMin); + double toValue = toMin + (toMax - toMin) * fromPercent; + return toValue; +} diff --git a/libnymea-app/thinggroup.h b/libnymea-app/thinggroup.h index 1da38cdd..533b8888 100644 --- a/libnymea-app/thinggroup.h +++ b/libnymea-app/thinggroup.h @@ -37,6 +37,7 @@ class DevicesProxy; class DeviceManager; +class ParamType; class ThingGroup : public Device { @@ -49,15 +50,13 @@ public: private: void syncStates(); -signals: - void actionExecutionFinished(int id, const QString &status); + QVariant mapValue(const QVariant &value, ParamType *fromParamType, ParamType *toParamType) const; -private: - DeviceManager* m_thingManager = nullptr; +private: DevicesProxy* m_things = nullptr; int m_idCounter = 0; - QHash> m_pendingActions; + QHash> m_pendingGroupActions; }; #endif // THINGGROUP_H diff --git a/libnymea-app/types/device.cpp b/libnymea-app/types/device.cpp index 038f0e6c..34b70cbd 100644 --- a/libnymea-app/types/device.cpp +++ b/libnymea-app/types/device.cpp @@ -40,6 +40,12 @@ Device::Device(DeviceManager *thingManager, DeviceClass *thingClass, const QUuid m_parentId(parentId), m_thingClass(thingClass) { + connect(thingManager, &DeviceManager::executeActionReply, this, [=](int commandId, const QVariantMap ¶ms){ + if (m_pendingActions.contains(commandId)) { + m_pendingActions.removeAll(commandId); + emit executeActionReply(commandId, params); + } + }); } QString Device::name() const @@ -229,7 +235,9 @@ int Device::executeAction(const QString &actionName, const QVariantList ¶ms) } finalParams.append(param); } - return m_thingManager->executeAction(m_id, actionType->id(), finalParams); + int commandId = m_thingManager->executeAction(m_id, actionType->id(), finalParams); + m_pendingActions.append(commandId); + return commandId; } QDebug operator<<(QDebug &dbg, Device *thing) diff --git a/libnymea-app/types/device.h b/libnymea-app/types/device.h index e9c81c5f..4f043b0a 100644 --- a/libnymea-app/types/device.h +++ b/libnymea-app/types/device.h @@ -114,6 +114,9 @@ signals: void statesChanged(); void eventTriggered(const QUuid &eventTypeId, const QVariantMap ¶ms); +signals: + void executeActionReply(int commandId, const QVariantMap ¶ms); + protected: DeviceManager *m_thingManager = nullptr; QString m_name; @@ -125,6 +128,8 @@ protected: Params *m_settings = nullptr; States *m_states = nullptr; DeviceClass *m_thingClass = nullptr; + + QList m_pendingActions; }; QDebug operator<<(QDebug &dbg, Device* thing); diff --git a/libnymea-app/types/interfaces.cpp b/libnymea-app/types/interfaces.cpp index 83e95886..9b361c64 100644 --- a/libnymea-app/types/interfaces.cpp +++ b/libnymea-app/types/interfaces.cpp @@ -417,7 +417,7 @@ void Interfaces::addStateType(const QString &interfaceName, const QString &name, st->setMinValue(min); st->setMaxValue(max); iface->stateTypes()->addStateType(st); - ParamTypes *pts = createParamTypes(name, displayName, type); + ParamTypes *pts = createParamTypes(name, displayName, type, QVariant(), min, max); addEventType(interfaceName, name, displayNameEvent, pts); if (writable) { addActionType(interfaceName, name, displayNameAction, pts); diff --git a/nymea-app/ui/delegates/InterfaceTile.qml b/nymea-app/ui/delegates/InterfaceTile.qml index fbf478fe..8c66eea4 100644 --- a/nymea-app/ui/delegates/InterfaceTile.qml +++ b/nymea-app/ui/delegates/InterfaceTile.qml @@ -200,11 +200,10 @@ MainPageTile { onClicked: { switch (iface.name) { case "light": - var group = engine.deviceManager.createGroup(Interfaces.findByName("colorlight"), devicesProxy); + var group = engine.thingManager.createGroup(Interfaces.findByName("colorlight"), devicesProxy); print("opening lights page for group", group) - pageStack.push("../devicepages/LightDevicePage.qml", {device: group}) + pageStack.push("../devicepages/LightDevicePage.qml", {thing: group}) } - } } } diff --git a/nymea-app/ui/devicepages/LightDevicePage.qml b/nymea-app/ui/devicepages/LightDevicePage.qml index 0ed37005..6b0da711 100644 --- a/nymea-app/ui/devicepages/LightDevicePage.qml +++ b/nymea-app/ui/devicepages/LightDevicePage.qml @@ -118,16 +118,16 @@ DevicePageBase { var absoluteCtValue = (model.ct * (root.ctStateType.maxValue - root.ctStateType.minValue) / 100) + root.ctStateType.minValue var params = []; var param1 = {}; - param1["paramTypeId"] = root.ctActionType.paramTypes.get(0).id; + param1["paramName"] = root.ctActionType.paramTypes.get(0).name; param1["value"] = absoluteCtValue; params.push(param1) - engine.deviceManager.executeAction(root.device.id, root.ctActionType.id, params) + root.thing.executeAction(root.ctActionType.name, params) params = []; param1 = {}; - param1["paramTypeId"] = root.brightnessActionType.paramTypes.get(0).id; + param1["paramName"] = root.brightnessActionType.paramTypes.get(0).name; param1["value"] = model.bri; params.push(param1) - engine.deviceManager.executeAction(root.device.id, root.brightnessActionType.id, params) + root.thing.executeAction(root.brightnessActionType.name, params) } } } @@ -176,7 +176,7 @@ DevicePageBase { MouseArea { anchors.fill: parent onClicked: { - engine.thingManager.executeAction(root.thing.id, root.powerStateType.id, [{paramTypeId: root.powerStateType.id, value: !root.powerState.value}]) + root.thing.executeAction("power", [{paramName: "power", value: !root.powerState.value}]) } } } diff --git a/nymea-app/ui/utils/ActionQueue.qml b/nymea-app/ui/utils/ActionQueue.qml index c1369520..a57506b5 100644 --- a/nymea-app/ui/utils/ActionQueue.qml +++ b/nymea-app/ui/utils/ActionQueue.qml @@ -16,10 +16,9 @@ Item { return; } d.pendingValue = value; - d.pendingCommand = engine.thingManager.executeAction(root.thing.id, - root.stateType.id, + d.pendingCommand = root.thing.executeAction(root.stateType.name, [{ - paramTypeId: root.stateType.id, + paramName: root.stateType.name, value: value }]) d.queuedValue = null @@ -33,7 +32,7 @@ Item { } Connections { - target: engine.thingManager + target: root.thing onExecuteActionReply: { if (d.pendingCommand == commandId) { d.pendingCommand = -1;