From f004899127460dcd3fae1fa3be4e88a790231fcd Mon Sep 17 00:00:00 2001 From: Boernsman Date: Mon, 10 May 2021 02:16:59 +0200 Subject: [PATCH] changed interface, fixed remarks from reviewer --- idm/idm.cpp | 51 ++++++++++++++++------------------- idm/idm.h | 11 ++++---- idm/idminfo.h | 2 +- idm/integrationpluginidm.cpp | 50 +++++++++++++++++++--------------- idm/integrationpluginidm.h | 7 ++--- idm/integrationpluginidm.json | 12 +++++---- modbus/modbushelpers.cpp | 5 +--- modbus/modbushelpers.h | 6 ++--- 8 files changed, 71 insertions(+), 73 deletions(-) diff --git a/idm/idm.cpp b/idm/idm.cpp index 0045365..c21bd24 100644 --- a/idm/idm.cpp +++ b/idm/idm.cpp @@ -1,6 +1,6 @@ /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * -* Copyright 2013 - 2020, nymea GmbH +* Copyright 2013 - 2021, nymea GmbH * Contact: contact@nymea.io * * This file is part of nymea. @@ -33,7 +33,6 @@ #include "../modbus/modbushelpers.h" #include -#include Idm::Idm(const QHostAddress &address, QObject *parent) : QObject(parent), @@ -61,6 +60,11 @@ bool Idm::connectDevice() return m_modbusMaster->connectDevice(); } +QHostAddress Idm::getIdmAddress() const +{ + return m_hostAddress; +} + void Idm::onReceivedHoldingRegister(int slaveAddress, int modbusRegister, const QVector &value) { Q_UNUSED(slaveAddress); @@ -70,7 +74,7 @@ void Idm::onReceivedHoldingRegister(int slaveAddress, int modbusRegister, const case Idm::OutsideTemperature: /* qCDebug(dcIdm()) << "received outside temperature"; */ if (value.length() == 2) { - m_info->outsideTemperature = ModbusHelpers::convertRegisterToFloat(&value[RegisterList::OutsideTemperature - modbusRegister]); + m_info.outsideTemperature = ModbusHelpers::convertRegisterToFloat(&value[RegisterList::OutsideTemperature - modbusRegister]); } QTimer::singleShot(200, this, [this] { m_modbusMaster->readHoldingRegister(Idm::ModbusUnitID, Idm::CurrentFaultNumber, 1); @@ -80,9 +84,9 @@ void Idm::onReceivedHoldingRegister(int slaveAddress, int modbusRegister, const /* qCDebug(dcIdm()) << "current fault number"; */ if (value.length() == 1) { if (value[0] > 0) { - m_info->error = true; + m_info.error = true; } else { - m_info->error = false; + m_info.error = false; } } QTimer::singleShot(200, this, [this] { @@ -92,7 +96,7 @@ void Idm::onReceivedHoldingRegister(int slaveAddress, int modbusRegister, const case Idm::HeatStorageTemperature: /* qCDebug(dcIdm()) << "received storage temperature"; */ if (value.length() == 2) { - m_info->waterTemperature = ModbusHelpers::convertRegisterToFloat(&value[RegisterList::HeatStorageTemperature - modbusRegister]); + m_info.waterTemperature = ModbusHelpers::convertRegisterToFloat(&value[RegisterList::HeatStorageTemperature - modbusRegister]); } QTimer::singleShot(200, this, [this] { m_modbusMaster->readHoldingRegister(Idm::ModbusUnitID, Idm::TargetHotWaterTemperature, 1); @@ -102,7 +106,7 @@ void Idm::onReceivedHoldingRegister(int slaveAddress, int modbusRegister, const /* qCDebug(dcIdm()) << "received target hot water temperature"; */ if (value.length() == 1) { /* The hot water target temperature is stored as UCHAR (manual p. 13) */ - m_info->targetWaterTemperature = (double)value[RegisterList::TargetHotWaterTemperature - modbusRegister]; + m_info.targetWaterTemperature = (double)value[RegisterList::TargetHotWaterTemperature - modbusRegister]; } QTimer::singleShot(200, this, [this] { m_modbusMaster->readHoldingRegister(Idm::ModbusUnitID, Idm::HeatPumpOperatingMode, 1); @@ -111,7 +115,7 @@ void Idm::onReceivedHoldingRegister(int slaveAddress, int modbusRegister, const case Idm::HeatPumpOperatingMode: /* qCDebug(dcIdm()) << "received heat pump operating mode"; */ if (value.length() == 1) { - m_info->mode = heatPumpOperationModeToString((Idm::IdmHeatPumpMode)value[RegisterList::HeatPumpOperatingMode-modbusRegister]); + m_info.mode = heatPumpOperationModeToString((Idm::IdmHeatPumpMode)value[RegisterList::HeatPumpOperatingMode-modbusRegister]); } QTimer::singleShot(200, this, [this] { m_modbusMaster->readHoldingRegister(Idm::ModbusUnitID, Idm::RoomTemperatureHKA, 2); @@ -120,7 +124,7 @@ void Idm::onReceivedHoldingRegister(int slaveAddress, int modbusRegister, const case Idm::RoomTemperatureHKA: /* qCDebug(dcIdm()) << "received room temperature hka"; */ if (value.length() == 2) { - m_info->roomTemperature = ModbusHelpers::convertRegisterToFloat(&value[RegisterList::RoomTemperatureHKA - modbusRegister]); + m_info.roomTemperature = ModbusHelpers::convertRegisterToFloat(&value[RegisterList::RoomTemperatureHKA - modbusRegister]); } QTimer::singleShot(200, this, [this] { m_modbusMaster->readHoldingRegister(Idm::ModbusUnitID, Idm::RoomTemperatureTargetHeatingEcoHKA, 2); @@ -129,7 +133,7 @@ void Idm::onReceivedHoldingRegister(int slaveAddress, int modbusRegister, const case Idm::RoomTemperatureTargetHeatingEcoHKA: /* qCDebug(dcIdm()) << "received room temprature hka eco"; */ if (value.length() == 2) { - m_info->targetRoomTemperature = ModbusHelpers::convertRegisterToFloat(&value[RegisterList::RoomTemperatureTargetHeatingEcoHKA - modbusRegister]); + m_info.targetRoomTemperature = ModbusHelpers::convertRegisterToFloat(&value[RegisterList::RoomTemperatureTargetHeatingEcoHKA - modbusRegister]); } QTimer::singleShot(200, this, [this] { m_modbusMaster->readHoldingRegister(Idm::ModbusUnitID, Idm::CurrentPowerConsumptionHeatPump, 2); @@ -138,12 +142,12 @@ void Idm::onReceivedHoldingRegister(int slaveAddress, int modbusRegister, const case Idm::CurrentPowerConsumptionHeatPump: /* qCDebug(dcIdm()) << "received power consumption heat pump"; */ if (value.length() == 2) { - m_info->powerConsumptionHeatPump = ModbusHelpers::convertRegisterToFloat(&value[RegisterList::CurrentPowerConsumptionHeatPump - modbusRegister]); + m_info.powerConsumptionHeatPump = ModbusHelpers::convertRegisterToFloat(&value[RegisterList::CurrentPowerConsumptionHeatPump - modbusRegister]); } /* Everything read without an error * -> set connected to true */ - m_info->connected = true; + m_info.connected = true; emit statusUpdated(m_info); break; } @@ -152,24 +156,17 @@ void Idm::onReceivedHoldingRegister(int slaveAddress, int modbusRegister, const void Idm::onModbusError() { qCDebug(dcIdm()) << "iDM: Received modbus error"; - - if (m_info != nullptr) { - m_info->connected = false; - emit statusUpdated(m_info); - } + m_info.connected = false; + emit statusUpdated(m_info); } void Idm::onRequestStatus() { - m_info = new IdmInfo; - - QUuid reqId{}; - reqId = m_modbusMaster->readHoldingRegister(Idm::ModbusUnitID, Idm::OutsideTemperature, 2); - - /* qCDebug(dcIdm()) << "Request id: " << reqId; */ + m_modbusMaster->readHoldingRegister(Idm::ModbusUnitID, Idm::OutsideTemperature, 2); } -QString Idm::systemOperationModeToString(IdmSysMode mode) { +QString Idm::systemOperationModeToString(IdmSysMode mode) +{ QString result{}; /* Operation modes according to table of manual p. 13 */ @@ -190,13 +187,12 @@ QString Idm::systemOperationModeToString(IdmSysMode mode) { result = "Nur Heizung/Kühlung"; break; } - return result; } -QString Idm::heatPumpOperationModeToString(IdmHeatPumpMode mode) { +QString Idm::heatPumpOperationModeToString(IdmHeatPumpMode mode) +{ QString result{}; - /* Operation modes according to table of manual p. 14 */ switch (mode) { case IdmHeatPumpModeOff: @@ -215,7 +211,6 @@ QString Idm::heatPumpOperationModeToString(IdmHeatPumpMode mode) { result = "Defrost"; break; } - return result; } diff --git a/idm/idm.h b/idm/idm.h index 2bd5ecb..b9699d4 100644 --- a/idm/idm.h +++ b/idm/idm.h @@ -1,6 +1,6 @@ /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * -* Copyright 2013 - 2020, nymea GmbH +* Copyright 2013 - 2021, nymea GmbH * Contact: contact@nymea.io * * This file is part of nymea. @@ -72,12 +72,13 @@ public: ~Idm(); bool connectDevice(); - QHostAddress getIdmAddress() const {return m_hostAddress;}; + QHostAddress getIdmAddress() const; + bool setTargetTemperature; private: /** Modbus Unit ID of Idm device */ - static const quint16 ModbusUnitID = 1; + static const quint16 ModbusUnitID = 1; enum IscModus { KeineAbwarme = 0, @@ -164,7 +165,7 @@ private: /** This structure is allocated within onRequestStatus and filled * by the receivedStatusGroupx functions */ - IdmInfo *m_info = nullptr; + IdmInfo m_info; /** Converts a system operation mode code to a string (according to manual p. 13) */ QString systemOperationModeToString(IdmSysMode mode); @@ -173,7 +174,7 @@ private: QString heatPumpOperationModeToString(IdmHeatPumpMode mode); signals: - void statusUpdated(IdmInfo *info); + void statusUpdated(const IdmInfo &info); void targetRoomTemperatureChanged(); public slots: diff --git a/idm/idminfo.h b/idm/idminfo.h index fcd29f2..6e8d7f1 100644 --- a/idm/idminfo.h +++ b/idm/idminfo.h @@ -1,6 +1,6 @@ /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * -* Copyright 2013 - 2020, nymea GmbH +* Copyright 2013 - 2021, nymea GmbH * Contact: contact@nymea.io * * This file is part of nymea. diff --git a/idm/integrationpluginidm.cpp b/idm/integrationpluginidm.cpp index 66483d6..5e02ebb 100644 --- a/idm/integrationpluginidm.cpp +++ b/idm/integrationpluginidm.cpp @@ -1,6 +1,6 @@ /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * -* Copyright 2013 - 2020, nymea GmbH +* Copyright 2013 - 2021, nymea GmbH * Contact: contact@nymea.io * * This file is part of nymea. @@ -44,9 +44,10 @@ void IntegrationPluginIdm::setupThing(ThingSetupInfo *info) if (thing->thingClassId() == navigator2ThingClassId) { QHostAddress hostAddress = QHostAddress(thing->paramValue(navigator2ThingIpAddressParamTypeId).toString()); - if (hostAddress.isNull()) { + + if (!hostAddress.isNull()) { qCWarning(dcIdm()) << "Setup failed, IP address not valid"; - info->finish(Thing::ThingErrorInvalidParameter, "No IP address given"); + info->finish(Thing::ThingErrorInvalidParameter, QT_TR_NOOP("No IP address given")); return; } @@ -75,8 +76,8 @@ void IntegrationPluginIdm::setupThing(ThingSetupInfo *info) return; } m_idmConnections.insert(thing, idm); - connect(idm, &Idm::statusUpdated, info, [info] (IdmInfo *idmInfo) { - if (idmInfo->connected) { + connect(idm, &Idm::statusUpdated, info, [info] (const IdmInfo &idmInfo) { + if (idmInfo.connected) { info->finish(Thing::ThingErrorNoError); } }); @@ -94,6 +95,7 @@ void IntegrationPluginIdm::postSetupThing(Thing *thing) qCDebug(dcIdm()) << "postSetupThing called" << thing->name(); if (!m_refreshTimer) { + qCDebug(dcIdm()) << "Starting refresh timer"; m_refreshTimer = hardwareManager()->pluginTimerManager()->registerTimer(10); connect(m_refreshTimer, &PluginTimer::timeout, this, &IntegrationPluginIdm::onRefreshTimer); } @@ -121,6 +123,12 @@ void IntegrationPluginIdm::thingRemoved(Thing *thing) m_idmConnections.take(thing)->deleteLater(); } } + + if (myThings().isEmpty()) { + qCDebug(dcIdm()) << "Stopping refresh timer"; + hardwareManager()->pluginTimerManager()->unregisterTimer(m_refreshTimer); + m_refreshTimer = nullptr; + } } void IntegrationPluginIdm::executeAction(ThingActionInfo *info) @@ -129,7 +137,9 @@ void IntegrationPluginIdm::executeAction(ThingActionInfo *info) Action action = info->action(); if (thing->thingClassId() == navigator2ThingClassId) { - if (action.actionTypeId() == navigator2PowerActionTypeId) { + if (action.actionTypeId() == navigator2TargetTemperatureActionTypeId) { + double targetTemperature = thing->stateValue(navigator2TargetTemperatureStateTypeId).toDouble(); + Q_UNUSED(targetTemperature); } else { Q_ASSERT_X(false, "executeAction", QString("Unhandled action: %1").arg(action.actionTypeId().toString()).toUtf8()); @@ -152,15 +162,11 @@ void IntegrationPluginIdm::update(Thing *thing) } } -void IntegrationPluginIdm::onStatusUpdated(IdmInfo *info) +void IntegrationPluginIdm::onStatusUpdated(const IdmInfo &info) { - /* qCDebug(dcIdm()) << "onStatusUpdated"; */ - if (!info) - return; - qCDebug(dcIdm()) << "Received status from heat pump"; - Idm *idm = static_cast(sender()); + Idm *idm = qobject_cast(sender()); Thing *thing = m_idmConnections.key(idm); if (!thing) @@ -168,16 +174,16 @@ void IntegrationPluginIdm::onStatusUpdated(IdmInfo *info) /* Received a structure holding the status info of the * heat pump. Update the thing states with the individual fields. */ - thing->setStateValue(navigator2ConnectedStateTypeId, info->connected); - thing->setStateValue(navigator2PowerStateTypeId, info->power); - thing->setStateValue(navigator2TemperatureStateTypeId, info->roomTemperature); - thing->setStateValue(navigator2OutsideAirTemperatureStateTypeId, info->outsideTemperature); - thing->setStateValue(navigator2WaterTemperatureStateTypeId, info->waterTemperature); - thing->setStateValue(navigator2TargetTemperatureStateTypeId, info->targetRoomTemperature); - thing->setStateValue(navigator2TargetWaterTemperatureStateTypeId, info->targetWaterTemperature); - thing->setStateValue(navigator2CurrentPowerConsumptionHeatPumpStateTypeId, info->powerConsumptionHeatPump); - thing->setStateValue(navigator2ModeStateTypeId, info->mode); - thing->setStateValue(navigator2ErrorStateTypeId, info->error); + thing->setStateValue(navigator2ConnectedStateTypeId, info.connected); + thing->setStateValue(navigator2PowerStateTypeId, info.power); + thing->setStateValue(navigator2TemperatureStateTypeId, info.roomTemperature); + thing->setStateValue(navigator2OutsideAirTemperatureStateTypeId, info.outsideTemperature); + thing->setStateValue(navigator2WaterTemperatureStateTypeId, info.waterTemperature); + thing->setStateValue(navigator2TargetTemperatureStateTypeId, info.targetRoomTemperature); + thing->setStateValue(navigator2TargetWaterTemperatureStateTypeId, info.targetWaterTemperature); + thing->setStateValue(navigator2CurrentPowerConsumptionHeatPumpStateTypeId, info.powerConsumptionHeatPump); + thing->setStateValue(navigator2ModeStateTypeId, info.mode); + thing->setStateValue(navigator2ErrorStateTypeId, info.error); } void IntegrationPluginIdm::onRefreshTimer() diff --git a/idm/integrationpluginidm.h b/idm/integrationpluginidm.h index e44f29d..351be4c 100644 --- a/idm/integrationpluginidm.h +++ b/idm/integrationpluginidm.h @@ -46,27 +46,24 @@ class IntegrationPluginIdm: public IntegrationPlugin Q_PLUGIN_METADATA(IID "io.nymea.IntegrationPlugin" FILE "integrationpluginidm.json") Q_INTERFACES(IntegrationPlugin) - public: - /** Constructor */ explicit IntegrationPluginIdm(); void setupThing(ThingSetupInfo *info) override; void postSetupThing(Thing *thing) override; void thingRemoved(Thing *thing) override; void executeAction(ThingActionInfo *info) override; - void update(Thing *thing); private: - PluginTimer *m_refreshTimer = nullptr; QHash m_idmConnections; QHash m_asyncActions; + void update(Thing *thing); void onRefreshTimer(); private slots: - void onStatusUpdated(IdmInfo *info); + void onStatusUpdated(const IdmInfo &info); }; #endif // INTEGRATIONPLUGINIDM_H diff --git a/idm/integrationpluginidm.json b/idm/integrationpluginidm.json index 79617ff..e66b23e 100644 --- a/idm/integrationpluginidm.json +++ b/idm/integrationpluginidm.json @@ -13,7 +13,7 @@ "displayName": "Navigator 2.0", "id": "1c95ac91-4eca-4cbf-b0f4-d60d35d069ed", "createMethods": ["user"], - "interfaces": ["temperaturesensor", "connectable"], + "interfaces": ["thermostat", "connectable"], "paramTypes": [ { "id": "05714e5c-d66a-4095-bbff-a0eb96fb035b", @@ -37,10 +37,8 @@ "name": "power", "displayName": "Power", "displayNameEvent": "Power changed", - "displayNameAction": "Change power", "type": "bool", - "defaultValue": 0, - "writable": true + "defaultValue": false }, { "id": "f0f596bf-7e45-43ea-b3d4-767b82dd422a", @@ -73,10 +71,14 @@ "id": "efae7493-68c3-4cb9-853c-81011bdf09ca", "name": "targetTemperature", "displayName": "Target room temperature", + "displayNameAction": "Set target room temperature", "displayNameEvent": "Target room temperature changed", "type": "double", "unit": "DegreeCelsius", - "defaultValue": 22.00 + "minValue": "18", + "maxValue": "28", + "defaultValue": 22.00, + "writable": true }, { "id": "746244d6-dd37-4af8-b2ae-a7d8463e51e2", diff --git a/modbus/modbushelpers.cpp b/modbus/modbushelpers.cpp index ee26a03..8e69cf2 100644 --- a/modbus/modbushelpers.cpp +++ b/modbus/modbushelpers.cpp @@ -1,6 +1,6 @@ /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * -* Copyright 2013 - 2020, nymea GmbH +* Copyright 2013 - 2021, nymea GmbH * Contact: contact@nymea.io * * This file is part of nymea. @@ -30,8 +30,6 @@ #include "modbushelpers.h" -#include - float ModbusHelpers::convertRegisterToFloat(const quint16 *reg) { float result = 0.0; @@ -46,7 +44,6 @@ float ModbusHelpers::convertRegisterToFloat(const quint16 *reg) { /* needs to be done with char * to avoid pedantic compiler errors */ memcpy((char *)&result, (char *)&tmp, sizeof(result)); } - return result; } diff --git a/modbus/modbushelpers.h b/modbus/modbushelpers.h index acea6b8..540daed 100644 --- a/modbus/modbushelpers.h +++ b/modbus/modbushelpers.h @@ -1,6 +1,6 @@ /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * -* Copyright 2013 - 2020, nymea GmbH +* Copyright 2013 - 2021, nymea GmbH * Contact: contact@nymea.io * * This file is part of nymea. @@ -36,8 +36,8 @@ class ModbusHelpers { public: - static float convertRegisterToFloat(const quint16 *reg); - static void convertFloatToRegister(QVector ®, float value); + static float convertRegisterToFloat(const quint16 *reg); + static void convertFloatToRegister(QVector ®, float value); }; #endif