From d78cf182a62f8dbaa5a3bbe456ffdb86af690a24 Mon Sep 17 00:00:00 2001 From: Boernsman Date: Tue, 2 Feb 2021 15:32:53 +0100 Subject: [PATCH] Removed thread delay and used Timer instead --- idm/idm.cpp | 76 +++++++++++--------- idm/idm.h | 4 +- idm/idminfo.h | 22 +++--- idm/integrationpluginidm.cpp | 127 +++++++++++++++++----------------- idm/integrationpluginidm.h | 1 - idm/integrationpluginidm.json | 8 +-- 6 files changed, 121 insertions(+), 117 deletions(-) diff --git a/idm/idm.cpp b/idm/idm.cpp index 537c5ed..0045365 100644 --- a/idm/idm.cpp +++ b/idm/idm.cpp @@ -32,18 +32,18 @@ #include "extern-plugininfo.h" #include "../modbus/modbushelpers.h" +#include #include Idm::Idm(const QHostAddress &address, QObject *parent) : QObject(parent), m_hostAddress(address) { + qCDebug(dcIdm()) << "iDM: Creating iDM connection" << m_hostAddress.toString(); m_modbusMaster = new ModbusTCPMaster(address, 502, this); if (m_modbusMaster) { - qCDebug(dcIdm()) << "created ModbusTCPMaster"; - m_modbusMaster->connectDevice(); - + qCDebug(dcIdm()) << "iDM: Created ModbusTCPMaster"; connect(m_modbusMaster, &ModbusTCPMaster::receivedHoldingRegister, this, &Idm::onReceivedHoldingRegister); connect(m_modbusMaster, &ModbusTCPMaster::readRequestError, this, &Idm::onModbusError); connect(m_modbusMaster, &ModbusTCPMaster::writeRequestError, this, &Idm::onModbusError); @@ -52,9 +52,13 @@ Idm::Idm(const QHostAddress &address, QObject *parent) : Idm::~Idm() { - if (m_modbusMaster) { - delete m_modbusMaster; - } + qCDebug(dcIdm()) << "iDM: Deleting iDM connection" << m_hostAddress.toString(); +} + +bool Idm::connectDevice() +{ + qCDebug(dcIdm()) << "iDM: Connecting device"; + return m_modbusMaster->connectDevice(); } void Idm::onReceivedHoldingRegister(int slaveAddress, int modbusRegister, const QVector &value) @@ -62,78 +66,84 @@ void Idm::onReceivedHoldingRegister(int slaveAddress, int modbusRegister, const Q_UNUSED(slaveAddress); /* qCDebug(dcIdm()) << "receivedHoldingRegister " << modbusRegister << "(length: " << value.length() << ")"; */ - /* Introducing a delay here for testing. - * Purposely set here, so one delay works for all branches - * of the following switch statement. In fact, the delay - * is used before evaluating what was just read, which - * does not seem to make sense, but it also acts before - * the next read command is sent. */ - QThread::msleep(200); - switch (modbusRegister) { case Idm::OutsideTemperature: /* qCDebug(dcIdm()) << "received outside temperature"; */ if (value.length() == 2) { - m_info->m_outsideTemperature = ModbusHelpers::convertRegisterToFloat(&value[RegisterList::OutsideTemperature - modbusRegister]); + m_info->outsideTemperature = ModbusHelpers::convertRegisterToFloat(&value[RegisterList::OutsideTemperature - modbusRegister]); } - m_modbusMaster->readHoldingRegister(Idm::ModbusUnitID, Idm::CurrentFaultNumber, 1); + QTimer::singleShot(200, this, [this] { + m_modbusMaster->readHoldingRegister(Idm::ModbusUnitID, Idm::CurrentFaultNumber, 1); + }); break; case Idm::CurrentFaultNumber: /* qCDebug(dcIdm()) << "current fault number"; */ if (value.length() == 1) { if (value[0] > 0) { - m_info->m_error = true; + m_info->error = true; } else { - m_info->m_error = false; + m_info->error = false; } } - m_modbusMaster->readHoldingRegister(Idm::ModbusUnitID, Idm::HeatStorageTemperature, 2); + QTimer::singleShot(200, this, [this] { + m_modbusMaster->readHoldingRegister(Idm::ModbusUnitID, Idm::HeatStorageTemperature, 2); + }); break; case Idm::HeatStorageTemperature: /* qCDebug(dcIdm()) << "received storage temperature"; */ if (value.length() == 2) { - m_info->m_waterTemperature = ModbusHelpers::convertRegisterToFloat(&value[RegisterList::HeatStorageTemperature - modbusRegister]); + m_info->waterTemperature = ModbusHelpers::convertRegisterToFloat(&value[RegisterList::HeatStorageTemperature - modbusRegister]); } - m_modbusMaster->readHoldingRegister(Idm::ModbusUnitID, Idm::TargetHotWaterTemperature, 1); + QTimer::singleShot(200, this, [this] { + m_modbusMaster->readHoldingRegister(Idm::ModbusUnitID, Idm::TargetHotWaterTemperature, 1); + }); break; case Idm::TargetHotWaterTemperature: /* 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->m_targetWaterTemperature = (double)value[RegisterList::TargetHotWaterTemperature - modbusRegister]; + m_info->targetWaterTemperature = (double)value[RegisterList::TargetHotWaterTemperature - modbusRegister]; } - m_modbusMaster->readHoldingRegister(Idm::ModbusUnitID, Idm::HeatPumpOperatingMode, 1); + QTimer::singleShot(200, this, [this] { + m_modbusMaster->readHoldingRegister(Idm::ModbusUnitID, Idm::HeatPumpOperatingMode, 1); + }); break; case Idm::HeatPumpOperatingMode: /* qCDebug(dcIdm()) << "received heat pump operating mode"; */ if (value.length() == 1) { - m_info->m_mode = heatPumpOperationModeToString((Idm::IdmHeatPumpMode)value[RegisterList::HeatPumpOperatingMode-modbusRegister]); + m_info->mode = heatPumpOperationModeToString((Idm::IdmHeatPumpMode)value[RegisterList::HeatPumpOperatingMode-modbusRegister]); } - m_modbusMaster->readHoldingRegister(Idm::ModbusUnitID, Idm::RoomTemperatureHKA, 2); + QTimer::singleShot(200, this, [this] { + m_modbusMaster->readHoldingRegister(Idm::ModbusUnitID, Idm::RoomTemperatureHKA, 2); + }); break; case Idm::RoomTemperatureHKA: /* qCDebug(dcIdm()) << "received room temperature hka"; */ if (value.length() == 2) { - m_info->m_roomTemperature = ModbusHelpers::convertRegisterToFloat(&value[RegisterList::RoomTemperatureHKA - modbusRegister]); + m_info->roomTemperature = ModbusHelpers::convertRegisterToFloat(&value[RegisterList::RoomTemperatureHKA - modbusRegister]); } - m_modbusMaster->readHoldingRegister(Idm::ModbusUnitID, Idm::RoomTemperatureTargetHeatingEcoHKA, 2); + QTimer::singleShot(200, this, [this] { + m_modbusMaster->readHoldingRegister(Idm::ModbusUnitID, Idm::RoomTemperatureTargetHeatingEcoHKA, 2); + }); break; case Idm::RoomTemperatureTargetHeatingEcoHKA: /* qCDebug(dcIdm()) << "received room temprature hka eco"; */ if (value.length() == 2) { - m_info->m_targetRoomTemperature = ModbusHelpers::convertRegisterToFloat(&value[RegisterList::RoomTemperatureTargetHeatingEcoHKA - modbusRegister]); + m_info->targetRoomTemperature = ModbusHelpers::convertRegisterToFloat(&value[RegisterList::RoomTemperatureTargetHeatingEcoHKA - modbusRegister]); } - m_modbusMaster->readHoldingRegister(Idm::ModbusUnitID, Idm::CurrentPowerConsumptionHeatPump, 2); + QTimer::singleShot(200, this, [this] { + m_modbusMaster->readHoldingRegister(Idm::ModbusUnitID, Idm::CurrentPowerConsumptionHeatPump, 2); + }); break; case Idm::CurrentPowerConsumptionHeatPump: /* qCDebug(dcIdm()) << "received power consumption heat pump"; */ if (value.length() == 2) { - m_info->m_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->m_connected = true; + m_info->connected = true; emit statusUpdated(m_info); break; } @@ -141,10 +151,10 @@ void Idm::onReceivedHoldingRegister(int slaveAddress, int modbusRegister, const void Idm::onModbusError() { - qCDebug(dcIdm()) << "Received modbus error"; + qCDebug(dcIdm()) << "iDM: Received modbus error"; if (m_info != nullptr) { - m_info->m_connected = false; + m_info->connected = false; emit statusUpdated(m_info); } } diff --git a/idm/idm.h b/idm/idm.h index b2f142b..2bd5ecb 100644 --- a/idm/idm.h +++ b/idm/idm.h @@ -68,12 +68,10 @@ class Idm : public QObject { Q_OBJECT public: - /** Constructor */ explicit Idm(const QHostAddress &address, QObject *parent = nullptr); - - /** Destructor */ ~Idm(); + bool connectDevice(); QHostAddress getIdmAddress() const {return m_hostAddress;}; private: diff --git a/idm/idminfo.h b/idm/idminfo.h index 1139f8a..fcd29f2 100644 --- a/idm/idminfo.h +++ b/idm/idminfo.h @@ -40,37 +40,37 @@ struct IdmInfo { /** Set to true, if register values can be read, * false in case of communication problems */ - bool m_connected; + bool connected; - bool m_power; + bool power; /** RegisterList::OutsideTemperature */ - double m_roomTemperature; + double roomTemperature; /** RegisterList::ExternalOutsideTemperature */ - double m_outsideTemperature; + double outsideTemperature; /** RegisterList::HeatStorageTemperature */ - double m_waterTemperature; + double waterTemperature; /** RegisterList::TargetRoomTemperatureZ1R1 (zone 1, room 1) */ - double m_targetRoomTemperature; + double targetRoomTemperature; /** RegisterList::TargetHotWaterTemperature */ - double m_targetWaterTemperature; + double targetWaterTemperature; /** RegisterList::HumiditySensor */ - double m_humidity; + double humidity; /** RegisterList::CurrentPowerConsumptionHeatPump */ - double m_powerConsumptionHeatPump; + double powerConsumptionHeatPump; /** RegisterList::OperationModeSystem */ - QString m_mode; + QString mode; /** True if there is an error code set * (RegisterList::CurrentFaultNumber != 0) */ - bool m_error; + bool error; }; Q_DECLARE_METATYPE(IdmInfo); diff --git a/idm/integrationpluginidm.cpp b/idm/integrationpluginidm.cpp index 58344f8..0fa74dc 100644 --- a/idm/integrationpluginidm.cpp +++ b/idm/integrationpluginidm.cpp @@ -51,69 +51,63 @@ void IntegrationPluginIdm::discoverThings(ThingDiscoveryInfo *info) // Just report no error for now, until the above question // is clarified info->finish(Thing::ThingErrorNoError); + } else { + Q_ASSERT_X(false, "discoverThings", QString("Unhandled thingClassId: %1").arg(info->thingClassId().toString()).toUtf8()); } } void IntegrationPluginIdm::setupThing(ThingSetupInfo *info) { - qCDebug(dcIdm()) << "setupThing called"; - Thing *thing = info->thing(); + qCDebug(dcIdm()) << "setupThing called" << thing->name(); if (thing->thingClassId() == navigator2ThingClassId) { QHostAddress hostAddress = QHostAddress(thing->paramValue(navigator2ThingIpAddressParamTypeId).toString()); - if (hostAddress.isNull() == false) { - QString msg = "User entered address: "; - msg += hostAddress.toString(); - - qCDebug(dcIdm()) << msg; - - /* Check, if address is already in use for another device */ - bool found = false; - - for (QHash::iterator item=m_idmConnections.begin(); item != m_idmConnections.end(); item++) { - if (hostAddress.isEqual(item.value()->getIdmAddress())) { - msg = "Addr of thing: "; - msg += item.value()->getIdmAddress().toString(); - qCDebug(dcIdm()) << msg; - - qCDebug(dcIdm()) << "Address in use already"; - found = true; - } else { - msg = "Different addr of other thing: "; - msg += item.value()->getIdmAddress().toString(); - qCDebug(dcIdm()) << msg; - } - } - - if (found == false) { - qCDebug(dcIdm()) << "Creating Idm object"; - - /* Create new Idm object and store it in hash table */ - Idm *idm = new Idm(hostAddress, this); - m_idmConnections.insert(thing, idm); - - /* Store thing info in hash table */ - m_idmInfos.insert(thing, info); - - msg = "Added IDM heatpump with address "; - msg += hostAddress.toString(); - - info->thing()->setStateValue(navigator2ConnectedStateTypeId, true); - info->finish(Thing::ThingErrorNoError, msg.toStdString().c_str()); - } else { - info->finish(Thing::ThingErrorThingInUse, "IP address already in use"); - } - } else { + if (hostAddress.isNull()) { + qCWarning(dcIdm()) << "Setup failed, IP address not valid"; info->finish(Thing::ThingErrorInvalidParameter, "No IP address given"); + return; } + + if (m_idmConnections.contains(thing)) { + qCDebug(dcIdm()) << "Cleaning up after reconfiguration"; + m_idmConnections.take(thing)->deleteLater(); + } + + qCDebug(dcIdm()) << "User entered address: " << hostAddress.toString(); + + /* Check, if address is already in use for another device */ + Q_FOREACH (Idm *idm, m_idmConnections) { + if (hostAddress.isEqual(idm->getIdmAddress())) { + + qCWarning(dcIdm()) << "Address already in use"; + info->finish(Thing::ThingErrorSetupFailed, "IP address already in use"); + return; + } + } + + qCDebug(dcIdm()) << "Creating Idm object"; + /* Create new Idm object and store it in hash table */ + Idm *idm = new Idm(hostAddress, this); + m_idmConnections.insert(thing, idm); + connect(idm, &Idm::statusUpdated, info, [info] (IdmInfo *idmInfo) { + if (idmInfo->connected) { + info->finish(Thing::ThingErrorNoError); + } + }); + connect(idm, &Idm::destroyed, this, [thing, this] {m_idmConnections.remove(thing);}); + connect(info, &ThingSetupInfo::aborted, idm, &Idm::deleteLater); + connect(idm, &Idm::statusUpdated, this, &IntegrationPluginIdm::onStatusUpdated); + + } else { + Q_ASSERT_X(false, "setupThing", QString("Unhandled thingClassId: %1").arg(thing->thingClassId().toString()).toUtf8()); } } void IntegrationPluginIdm::postSetupThing(Thing *thing) { - qCDebug(dcIdm()) << "postSetupThing called"; + qCDebug(dcIdm()) << "postSetupThing called" << thing->name(); if (!m_refreshTimer) { m_refreshTimer = hardwareManager()->pluginTimerManager()->registerTimer(10); @@ -125,7 +119,6 @@ void IntegrationPluginIdm::postSetupThing(Thing *thing) Idm *idm = m_idmConnections.value(thing); if (idm != nullptr) { - connect(idm, &Idm::statusUpdated, this, &IntegrationPluginIdm::onStatusUpdated); qCDebug(dcIdm()) << "Thing set up, calling update"; update(thing); @@ -137,8 +130,12 @@ void IntegrationPluginIdm::postSetupThing(Thing *thing) void IntegrationPluginIdm::thingRemoved(Thing *thing) { - if (m_idmConnections.contains(thing)) { - m_idmConnections.take(thing)->deleteLater(); + qCDebug(dcIdm()) << "thingRemoved called" << thing->name(); + + if (thing->thingClassId() == navigator2ThingClassId) { + if (m_idmConnections.contains(thing)) { + m_idmConnections.take(thing)->deleteLater(); + } } } @@ -148,10 +145,10 @@ void IntegrationPluginIdm::executeAction(ThingActionInfo *info) Action action = info->action(); if (thing->thingClassId() == navigator2ThingClassId) { - if (action.actionTypeId() == navigator2PowerActionTypeId) { - - } else { - Q_ASSERT_X(false, "executeAction", QString("Unhandled action: %1").arg(action.actionTypeId().toString()).toUtf8()); + if (action.actionTypeId() == navigator2PowerActionTypeId) { + + } else { + Q_ASSERT_X(false, "executeAction", QString("Unhandled action: %1").arg(action.actionTypeId().toString()).toUtf8()); } } else { Q_ASSERT_X(false, "executeAction", QString("Unhandled thingClassId: %1").arg(thing->thingClassId().toString()).toUtf8()); @@ -164,11 +161,11 @@ void IntegrationPluginIdm::update(Thing *thing) qCDebug(dcIdm()) << "Updating thing"; Idm *idm = m_idmConnections.value(thing); - Q_UNUSED(idm); if (idm != nullptr) { idm->onRequestStatus(); } +<<<<<<< HEAD if (m_idmInfos.contains(thing)) { <<<<<<< HEAD @@ -180,6 +177,8 @@ void IntegrationPluginIdm::update(Thing *thing) /* info->finish(Thing::ThingErrorNoError); */ >>>>>>> 3b5ab5c... Another fix for previous commit } +======= +>>>>>>> 27a88b6... Removed thread delay and used Timer instead } } @@ -199,16 +198,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->m_connected); - thing->setStateValue(navigator2PowerStateTypeId, info->m_power); - thing->setStateValue(navigator2TemperatureStateTypeId, info->m_roomTemperature); - thing->setStateValue(navigator2OutsideAirTemperatureStateTypeId, info->m_outsideTemperature); - thing->setStateValue(navigator2WaterTemperatureStateTypeId, info->m_waterTemperature); - thing->setStateValue(navigator2TargetTemperatureStateTypeId, info->m_targetRoomTemperature); - thing->setStateValue(navigator2TargetWaterTemperatureStateTypeId, info->m_targetWaterTemperature); - thing->setStateValue(navigator2CurrentPowerConsumptionHeatPumpStateTypeId, info->m_powerConsumptionHeatPump); - thing->setStateValue(navigator2ModeStateTypeId, info->m_mode); - thing->setStateValue(navigator2ErrorStateTypeId, info->m_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 42b9627..6ce157e 100644 --- a/idm/integrationpluginidm.h +++ b/idm/integrationpluginidm.h @@ -62,7 +62,6 @@ private: PluginTimer *m_refreshTimer = nullptr; QHash m_idmConnections; - QHash m_idmInfos; QHash m_asyncActions; void onRefreshTimer(); diff --git a/idm/integrationpluginidm.json b/idm/integrationpluginidm.json index 9ed02ed..0b68065 100644 --- a/idm/integrationpluginidm.json +++ b/idm/integrationpluginidm.json @@ -85,18 +85,18 @@ "displayNameEvent": "Target water temperature changed", "type": "double", "unit": "DegreeCelsius", - "defaultValue": 46.00 + "defaultValue": 0.00 }, { "id": "b98fb325-100d-4eae-bf8d-97e8f7e1eb00", "name": "currentPowerConsumptionHeatPump", - "displayName": "Curr. power consumption", + "displayName": "Current power consumption", "displayNameEvent": "Current power consumption heat pump changed", "displayNameAction": "Change current power consumption het pump", "type": "double", "unit": "KiloWatt", - "defaultValue": 0 + "defaultValue": 0.00 }, { "id": "e539366b-44da-4119-b11b-497bcdb1f522", @@ -121,8 +121,6 @@ "type": "bool", "defaultValue": false } - ], - "actionTypes": [ ] } ]