Skip to content

Commit

Permalink
Do not show invalid devices in the Device List
Browse files Browse the repository at this point in the history
Invalid devices may still have a valid /DeviceInstance, so tighten the
device validity check to also test for /ProductId and /ProductName.

Do the validity check in BaseDevice so that the test condition is the
same for all devices.

Fixes #1233
  • Loading branch information
blammit committed Jun 26, 2024
1 parent 47a56f1 commit 6308d2a
Show file tree
Hide file tree
Showing 15 changed files with 83 additions and 21 deletions.
11 changes: 7 additions & 4 deletions components/Device.qml
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,7 @@ import Victron.VenusOS
BaseDevice {
id: root

property bool valid: deviceInstance >= 0

readonly property string customName: _customName.value || ""
readonly property string productName: _productName.value || ""

readonly property VeQuickItem _deviceInstance: VeQuickItem {
uid: root.serviceUid ? root.serviceUid + "/DeviceInstance" : ""
Expand All @@ -22,11 +19,17 @@ BaseDevice {
uid: root.serviceUid ? root.serviceUid + "/CustomName" : ""
}

readonly property VeQuickItem _productId: VeQuickItem {
uid: root.serviceUid ? root.serviceUid + "/ProductId" : ""
}

readonly property VeQuickItem _productName: VeQuickItem {
uid: root.serviceUid ? root.serviceUid + "/ProductName" : ""
}

deviceInstance: _deviceInstance.value === undefined ? -1 : _deviceInstance.value
deviceInstance: _deviceInstance.isValid ? _deviceInstance.value : -1
productId: _productId.isValid ? _productId.value : 0
productName: _productName.value || ""
name: _customName.value || _productName.value || ""
description: name
}
5 changes: 0 additions & 5 deletions data/common/InverterCharger.qml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ Device {
numberOfAcInputs: _numberOfAcInputs.value || 0
}

readonly property int productId: _productId.value === undefined ? -1 : _productId.value
readonly property int productType: _productUpperByte === 0x19 || _productUpperByte === 0x26
? VenusOS.VeBusDevice_ProductType_EuProduct
: (_productUpperByte === 0x20 || _productUpperByte === 0x27 ? VenusOS.VeBusDevice_ProductType_UsProduct : -1)
Expand All @@ -43,10 +42,6 @@ Device {
uid: inverterCharger.serviceUid + "/State"
}

readonly property VeQuickItem _productId: VeQuickItem {
uid: inverterCharger.serviceUid + "/ProductId"
}

readonly property VeQuickItem _nominalInverterPower: VeQuickItem {
uid: inverterCharger.serviceUid + "/Ac/Out/NominalInverterPower"
onValueChanged: if (!!Global.inverterChargers) Global.inverterChargers.refreshNominalInverterPower()
Expand Down
3 changes: 1 addition & 2 deletions data/common/Tank.qml
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,11 @@ Device {
Component.onCompleted: Qt.callLater(tank._updateMeasurements)
}

valid: deviceInstance >= 0 && type >= 0
onValidChanged: Qt.callLater(tank._updateModel)
onTypeChanged: Qt.callLater(tank._updateModel)

function _updateModel() {
if (valid) {
if (valid && type >= 0) {
if (_tankModel && _tankModel.type !== type) {
_tankModel.removeDevice(tank.serviceUid)
}
Expand Down
4 changes: 0 additions & 4 deletions data/mock/AcInputsImpl.qml
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,6 @@ QtObject {
required property var modelData

readonly property Device device: Device {
readonly property VeQuickItem _productId: VeQuickItem {
uid: device.serviceUid ? device.serviceUid + "/ProductId" : ""
}

Component.onCompleted: {
serviceUid = "mock/" + input.modelData["serviceName"]
_deviceInstance.setValue(input.index)
Expand Down
1 change: 1 addition & 0 deletions data/mock/AcSystemDevicesImpl.qml
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ Item {
serviceUid = "mock/com.victronenergy.acsystem.ttyUSB" + deviceInstanceNum
_deviceInstance.setValue(deviceInstanceNum)
_productName.setValue("RS 48/6000")
_productId.setValue(123)
_customName.setValue("AC System " + deviceInstanceNum)
acSystem.setMockValue("/Ac/NumberOfPhases", 3)
acSystem.setMockValue("/Ac/NumberOfAcInputs", 2)
Expand Down
1 change: 1 addition & 0 deletions data/mock/ChargersImpl.qml
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ Item {
serviceUid = "mock/com.victronenergy.charger.ttyUSB" + deviceInstanceNum
_deviceInstance.setValue(deviceInstanceNum)
_customName.setValue("AC Charger " + deviceInstanceNum)
_productId.setValue(123)
charger.setMockValue("/Mode", 1)
charger.setMockValue("/State", Math.floor(Math.random() * VenusOS.System_State_FaultCondition))
charger.setMockValue("/Ac/In/CurrentLimit", Math.random() * 30)
Expand Down
10 changes: 4 additions & 6 deletions data/mock/DcInputsImpl.qml
Original file line number Diff line number Diff line change
Expand Up @@ -91,12 +91,9 @@ QtObject {
}
}

property VeQuickItem _productId: VeQuickItem {
uid: input.serviceUid + "/ProductId"
onValueChanged: {
if (value === 0xA3F0) {
initOrionXSValues()
}
onProductIdChanged: {
if (productId === 0xA3F0) {
initOrionXSValues()
}
}

Expand Down Expand Up @@ -142,6 +139,7 @@ QtObject {
serviceUid = "mock/com.victronenergy." + serviceType + ".ttyUSB" + deviceInstanceNum
_deviceInstance.setValue(deviceInstanceNum)
_productName.setValue("DC device (%1)".arg(serviceType))
_productId.setValue(123)
setMockValue("/State", 4)
}
}
Expand Down
1 change: 1 addition & 0 deletions data/mock/DcLoadsImpl.qml
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ QtObject {
serviceUid = "mock/com.victronenergy." + serviceType + ".ttyUSB" + deviceInstanceNum
_deviceInstance.setValue(deviceInstanceNum)
_productName.setValue("DC Load (%1)".arg(serviceType))
_productId.setValue(123)
setMockValue("/Mode", 4)
setMockValue("/State", 5)
setMockValue("/Error", 0)
Expand Down
1 change: 1 addition & 0 deletions data/mock/EnvironmentInputsImpl.qml
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ Item {
serviceUid = "mock/com.victronenergy.temperature.ttyUSB" + deviceInstanceNum
_deviceInstance.setValue(deviceInstanceNum)
_productName.setValue("Generic Temperature Sensor")
_productId.setValue(123)
_status.setValue(VenusOS.EnvironmentInput_Status_Ok)
}

Expand Down
1 change: 1 addition & 0 deletions data/mock/GeneratorsImpl.qml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ QtObject {
Component.onCompleted: {
_deviceInstance.setValue(0)
_productName.setValue("Start/Stop generator")
_productId.setValue(123)
_state.setValue(VenusOS.Generators_State_Running)
_runningBy.setValue(VenusOS.Generators_RunningBy_Soc)
setAutoStart(true)
Expand Down
1 change: 1 addition & 0 deletions data/mock/PvInvertersImpl.qml
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ QtObject {
serviceUid = "mock/com.victronenergy.pvinverter.ttyUSB" + deviceInstanceNum
_deviceInstance.setValue(deviceInstanceNum)
_productName.setValue("PV Inverter")
_productId.setValue(123)
_customName.setValue("My PV Inverter " + deviceInstanceNum)
_statusCode.setValue(Math.random() * VenusOS.PvInverter_StatusCode_Error)

Expand Down
1 change: 1 addition & 0 deletions data/mock/SolarChargersImpl.qml
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ QtObject {
serviceUid = "mock/com.victronenergy.solarcharger.ttyUSB" + deviceInstanceNum
_deviceInstance.setValue(deviceInstanceNum)
_productName.setValue("SmartSolar Charger MPPT 100/50")
_productId.setValue(123)
_customName.setValue("My Solar Charger " + deviceInstanceNum)
_state.setValue(VenusOS.SolarCharger_State_ExternalControl)
_errorCode.setValue(0)
Expand Down
1 change: 1 addition & 0 deletions data/mock/TanksImpl.qml
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ QtObject {
serviceUid = "mock/com.victronenergy.tank.ttyUSB" + deviceInstanceNum
_deviceInstance.setValue(deviceInstanceNum)
_productName.setValue("Generic Tank Input")
_productId.setValue(123)
_status.setValue(VenusOS.Tank_Status_Ok)
}

Expand Down
47 changes: 47 additions & 0 deletions src/basedevicemodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ BaseDevice::BaseDevice(QObject *parent)
{
}

bool BaseDevice::isValid() const
{
return !m_serviceUid.isEmpty() && !m_productName.isEmpty() && m_deviceInstance >= 0 && m_productId > 0;
}

QString BaseDevice::serviceUid() const
{
return m_serviceUid;
Expand All @@ -23,8 +28,12 @@ QString BaseDevice::serviceUid() const
void BaseDevice::setServiceUid(const QString &serviceUid)
{
if (m_serviceUid != serviceUid) {
const bool prevValid = isValid();
m_serviceUid = serviceUid;
emit serviceUidChanged();
if (prevValid != isValid()) {
emit validChanged();
}
}
}

Expand All @@ -36,8 +45,46 @@ int BaseDevice::deviceInstance() const
void BaseDevice::setDeviceInstance(int deviceInstance)
{
if (m_deviceInstance != deviceInstance) {
const bool prevValid = isValid();
m_deviceInstance = deviceInstance;
emit deviceInstanceChanged();
if (prevValid != isValid()) {
emit validChanged();
}
}
}

int BaseDevice::productId() const
{
return m_productId;
}

void BaseDevice::setProductId(int productId)
{
if (m_productId != productId) {
const bool prevValid = isValid();
m_productId = productId;
emit productIdChanged();
if (prevValid != isValid()) {
emit validChanged();
}
}
}

QString BaseDevice::productName() const
{
return m_productName;
}

void BaseDevice::setProductName(const QString &productName)
{
if (m_productName != productName) {
const bool prevValid = isValid();
m_productName = productName;
emit productNameChanged();
if (prevValid != isValid()) {
emit validChanged();
}
}
}

Expand Down
16 changes: 16 additions & 0 deletions src/basedevicemodel.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,37 +18,53 @@ class BaseDevice : public QObject
{
Q_OBJECT
QML_ELEMENT
Q_PROPERTY(bool valid READ isValid NOTIFY validChanged)
Q_PROPERTY(QString serviceUid READ serviceUid WRITE setServiceUid NOTIFY serviceUidChanged)
Q_PROPERTY(int deviceInstance READ deviceInstance WRITE setDeviceInstance NOTIFY deviceInstanceChanged)
Q_PROPERTY(int productId READ productId WRITE setProductId NOTIFY productIdChanged)
Q_PROPERTY(QString productName READ productName WRITE setProductName NOTIFY productNameChanged)
Q_PROPERTY(QString name READ name WRITE setName NOTIFY nameChanged)
Q_PROPERTY(QString description READ description WRITE setDescription NOTIFY descriptionChanged)

public:
explicit BaseDevice(QObject *parent = nullptr);

bool isValid() const;

QString serviceUid() const;
void setServiceUid(const QString &serviceUid);

int deviceInstance() const;
void setDeviceInstance(int deviceInstance);

int productId() const;
void setProductId(int productId);

QString productName() const;
void setProductName(const QString &productName);

QString name() const;
void setName(const QString &name);

QString description() const;
void setDescription(const QString &description);

Q_SIGNALS:
void validChanged();
void serviceUidChanged();
void deviceInstanceChanged();
void productNameChanged();
void productIdChanged();
void nameChanged();
void descriptionChanged();

private:
QString m_serviceUid;
QString m_name;
QString m_description;
QString m_productName;
int m_deviceInstance = -1;
int m_productId = 0;
};


Expand Down

0 comments on commit 6308d2a

Please sign in to comment.