From 43c1941ed8b4aca745c595170b6dc5e9193db8e6 Mon Sep 17 00:00:00 2001 From: Bernhard Trinnes Date: Fri, 22 Jun 2018 15:47:38 +0200 Subject: [PATCH] clean-up unipi code and implement inline comments from reviewer --- unipi/devicepluginunipi.cpp | 58 ++++++++++++++++-------------------- unipi/devicepluginunipi.h | 12 ++++---- unipi/devicepluginunipi.json | 24 --------------- 3 files changed, 32 insertions(+), 62 deletions(-) diff --git a/unipi/devicepluginunipi.cpp b/unipi/devicepluginunipi.cpp index 3f4ec75b..eed0b922 100644 --- a/unipi/devicepluginunipi.cpp +++ b/unipi/devicepluginunipi.cpp @@ -39,7 +39,6 @@ void DevicePluginUniPi::init() { connectToEvok(); - // Refresh timer for snapd checks m_refreshTimer = hardwareManager()->pluginTimerManager()->registerTimer(60); connect(m_refreshTimer, &PluginTimer::timeout, this, &DevicePluginUniPi::onRefreshTimer); @@ -79,15 +78,15 @@ DeviceManager::DeviceSetupStatus DevicePluginUniPi::setupDevice(Device *device) if (device->deviceClassId() == shutterDeviceClassId) { - if (device->paramValue(shutterOutputTypeOpenParamTypeId) == GPIOType::relay) { + if (device->paramValue(shutterOutputTypeOpenParamTypeId) == GpioType::Relay) { m_usedRelais.insert(device->paramValue(shutterOutputOpenParamTypeId).toString(), device); - } else if (device->paramValue(shutterOutputTypeOpenParamTypeId) == GPIOType::digitalOutput) { + } else if (device->paramValue(shutterOutputTypeOpenParamTypeId) == GpioType::DigitalOutput) { m_usedDigitalOutputs.insert(device->paramValue(shutterOutputOpenParamTypeId).toString(), device); } - if (device->paramValue(shutterOutputTypeCloseParamTypeId) == GPIOType::relay) { + if (device->paramValue(shutterOutputTypeCloseParamTypeId) == GpioType::Relay) { m_usedRelais.insert(device->paramValue(shutterOutputCloseParamTypeId).toString(), device); - } else if (device->paramValue(shutterOutputTypeOpenParamTypeId) == GPIOType::digitalOutput) { + } else if (device->paramValue(shutterOutputTypeOpenParamTypeId) == GpioType::DigitalOutput) { m_usedDigitalOutputs.insert(device->paramValue(shutterOutputCloseParamTypeId).toString(), device); } @@ -96,9 +95,9 @@ DeviceManager::DeviceSetupStatus DevicePluginUniPi::setupDevice(Device *device) if (device->deviceClassId() == lightDeviceClassId) { - if (device->paramValue(lightOutputTypeParamTypeId) == GPIOType::relay) { + if (device->paramValue(lightOutputTypeParamTypeId) == GpioType::Relay) { m_usedRelais.insert(device->paramValue(lightOutputParamTypeId).toString(), device); - } else if (device->paramValue(lightOutputParamTypeId) == GPIOType::digitalOutput) { + } else if (device->paramValue(lightOutputParamTypeId) == GpioType::DigitalOutput) { m_usedDigitalOutputs.insert(device->paramValue(lightOutputParamTypeId).toString(), device); } return DeviceManager::DeviceSetupStatusSuccess; @@ -193,7 +192,7 @@ DeviceManager::DeviceError DevicePluginUniPi::discoverDevices(const DeviceClassI for (int i = 0; i < m_analogInputs.count(); i++) { const QString circuit = m_analogInputs.at(i); - // Offer only analog inputs which arn't in use already + // Offer only analog inputs which aren't in use already if (m_usedAnalogInputs.contains(circuit)){ continue; } @@ -249,8 +248,8 @@ DeviceManager::DeviceError DevicePluginUniPi::discoverDevices(const DeviceClassI ParamList parameters; parameters.append(Param(shutterOutputOpenParamTypeId, openingCircuit)); parameters.append(Param(shutterOutputCloseParamTypeId, closingCircuit)); - parameters.append(Param(shutterOutputTypeOpenParamTypeId, GPIOType::relay)); - parameters.append(Param(shutterOutputTypeCloseParamTypeId, GPIOType::relay)); + parameters.append(Param(shutterOutputTypeOpenParamTypeId, GpioType::Relay)); + parameters.append(Param(shutterOutputTypeCloseParamTypeId, GpioType::Relay)); descriptor.setParams(parameters); deviceDescriptors.append(descriptor); break; @@ -276,8 +275,8 @@ DeviceManager::DeviceError DevicePluginUniPi::discoverDevices(const DeviceClassI ParamList parameters; parameters.append(Param(shutterOutputOpenParamTypeId, openingCircuit)); parameters.append(Param(shutterOutputCloseParamTypeId, closingCircuit)); - parameters.append(Param(shutterOutputTypeOpenParamTypeId, GPIOType::digitalOutput)); - parameters.append(Param(shutterOutputTypeCloseParamTypeId, GPIOType::digitalOutput)); + parameters.append(Param(shutterOutputTypeOpenParamTypeId, GpioType::DigitalOutput)); + parameters.append(Param(shutterOutputTypeCloseParamTypeId, GpioType::DigitalOutput)); descriptor.setParams(parameters); deviceDescriptors.append(descriptor); break; @@ -302,7 +301,7 @@ DeviceManager::DeviceError DevicePluginUniPi::discoverDevices(const DeviceClassI DeviceDescriptor descriptor(deviceClassId, "Light", QString("Relay %1").arg(circuit)); ParamList parameters; parameters.append(Param(lightOutputParamTypeId, circuit)); - parameters.append(Param(lightOutputTypeParamTypeId, GPIOType::relay)); + parameters.append(Param(lightOutputTypeParamTypeId, GpioType::Relay)); descriptor.setParams(parameters); deviceDescriptors.append(descriptor); } @@ -317,7 +316,7 @@ DeviceManager::DeviceError DevicePluginUniPi::discoverDevices(const DeviceClassI DeviceDescriptor descriptor(deviceClassId, "Light", QString("Digital output %1").arg(circuit)); ParamList parameters; parameters.append(Param(lightOutputParamTypeId, circuit)); - parameters.append(Param(lightOutputTypeParamTypeId, GPIOType::digitalOutput)); + parameters.append(Param(lightOutputTypeParamTypeId, GpioType::DigitalOutput)); descriptor.setParams(parameters); deviceDescriptors.append(descriptor); } @@ -366,7 +365,7 @@ void DevicePluginUniPi::connectToEvok() { if (m_webSocket == NULL) { - int port = 8080; //configValue(uniPiPortParamTypeId).toInt(); TODO + int port = 8080; //configValue(uniPiPortParamTypeId).toInt(); //FIXME plugin configuration loading currently not possible in init m_webSocket = new QWebSocket(); connect(m_webSocket, &QWebSocket::connected, this, &DevicePluginUniPi::onWebSocketConnected); @@ -394,27 +393,29 @@ void DevicePluginUniPi::deviceRemoved(Device *device) m_usedAnalogInputs.remove(device->paramValue(analogInputAnalogInputNumberParamTypeId).toString()); } else if (device->deviceClassId() == shutterDeviceClassId) { - if (device->paramValue(shutterOutputTypeOpenParamTypeId) == GPIOType::relay) { + if (device->paramValue(shutterOutputTypeOpenParamTypeId) == GpioType::Relay) { m_usedRelais.remove(device->paramValue(shutterOutputOpenParamTypeId).toString()); - } else if (device->paramValue(shutterOutputOpenParamTypeId) == GPIOType::digitalOutput) { + } else if (device->paramValue(shutterOutputOpenParamTypeId) == GpioType::DigitalOutput) { m_usedDigitalOutputs.remove(device->paramValue(shutterOutputOpenParamTypeId).toString()); } - if (device->paramValue(shutterOutputTypeCloseParamTypeId) == GPIOType::relay) { + if (device->paramValue(shutterOutputTypeCloseParamTypeId) == GpioType::Relay) { m_usedRelais.remove(device->paramValue(shutterOutputCloseParamTypeId).toString()); - } else if (device->paramValue(shutterOutputOpenParamTypeId) == GPIOType::digitalOutput) { + } else if (device->paramValue(shutterOutputOpenParamTypeId) == GpioType::DigitalOutput) { m_usedDigitalOutputs.remove(device->paramValue(shutterOutputCloseParamTypeId).toString()); } } else if (device->deviceClassId() == lightDeviceClassId) { - if (device->paramValue(lightOutputTypeParamTypeId) == GPIOType::relay) { + if (device->paramValue(lightOutputTypeParamTypeId) == GpioType::Relay) { m_usedRelais.remove(device->paramValue(lightOutputParamTypeId).toString()); - } else if (device->paramValue(lightOutputParamTypeId) == GPIOType::digitalOutput) { + } else if (device->paramValue(lightOutputParamTypeId) == GpioType::DigitalOutput) { m_usedDigitalOutputs.remove(device->paramValue(lightOutputParamTypeId).toString()); } } else if (device->deviceClassId() == dimmerSwitchDeviceClassId) { m_usedDigitalInputs.remove(device->paramValue(dimmerSwitchInputNumberParamTypeId).toString()); - m_dimmerSwitches.remove(m_dimmerSwitches.key(device)); + DimmerSwitch *dimmerSwitch = m_dimmerSwitches.key(device); + m_dimmerSwitches.remove(dimmerSwitch); + dimmerSwitch->deleteLater(); } if (myDevices().isEmpty()) { @@ -425,6 +426,8 @@ void DevicePluginUniPi::deviceRemoved(Device *device) DeviceManager::DeviceError DevicePluginUniPi::executeAction(Device *device, const Action &action) { + if (m_webSocket->state() != QAbstractSocket::ConnectedState) + return DeviceManager::DeviceErrorHardwareNotAvailable; if (device->deviceClassId() == relayOutputDeviceClassId) { @@ -695,7 +698,7 @@ void DevicePluginUniPi::onWebSocketTextMessageReceived(QString message) } } - if (obj["dev"] == "led") { //TODO cant discover leds without toggling it from another client + if (obj["dev"] == "led") { //TODO can't discover leds without toggling it from another client qCDebug(dcUniPi()) << "Led:" << obj["dev"] << "Circuit:" << obj["circuit"].toString() << "Value:" << obj["value"].toInt(); if (!m_leds.contains(obj["circuit"].toString())){ @@ -703,15 +706,6 @@ void DevicePluginUniPi::onWebSocketTextMessageReceived(QString message) m_leds.append(obj["circuit"].toString()); } } - - if (obj["dev"] == "sensor") { //TODO not yet implemented - qCDebug(dcUniPi()) << "Sensor:" << obj["dev"] << "Circuit:" << obj["circuit"].toString() << "Value:" << obj["value"].toInt(); - - if (!m_sensors.contains(obj["circuit"].toString())){ - //New Sensor detected - m_sensors.append(obj["circuit"].toString()); - } - } } } diff --git a/unipi/devicepluginunipi.h b/unipi/devicepluginunipi.h index 6e1ee629..6c47d445 100644 --- a/unipi/devicepluginunipi.h +++ b/unipi/devicepluginunipi.h @@ -51,12 +51,12 @@ public: private: - enum GPIOType { - relay, - digitalInput, - digitalOutput, - analogInput, - analogOutput + enum GpioType { + Relay, + DigitalInput, + DigitalOutput, + AnalogInput, + AnalogOutput }; QHash m_usedRelais; diff --git a/unipi/devicepluginunipi.json b/unipi/devicepluginunipi.json index 655bb873..d2730a26 100644 --- a/unipi/devicepluginunipi.json +++ b/unipi/devicepluginunipi.json @@ -319,30 +319,6 @@ "writable": true } ] - }, - { - "id": "fe3d7d2f-3cee-4f2b-b6f8-4381ff1a2825", - "name": "temperatureSensor", - "displayName": "1-Wire Temperature Sensor", - "deviceIcon": "Thermometer", - "createMethods": ["discovery"], - "interfaces": ["temperaturesensor"], - "basicTags": ["Actuator"], - "paramTypes": [ - { - } - ], - "stateTypes":[ - { - "id": "a11e8108-d16e-4972-820a-f3611ba6fe24", - "name": "temperature", - "displayName": "Temperature", - "displayNameEvent": "Temperature changed", - "type": "double", - "defaultValue": 0, - "unit": "DegreeCelsius" - } - ] } ] }