From f460847c07c52abde125308345efd6853dd21074 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 15 Apr 2026 16:26:29 +0000 Subject: [PATCH 1/3] Fix adapter device ID not incrementing when adding new devices When adding a new device via AdapterDeviceSettings, the new tab always received the adapter's hardcoded default id (e.g. 1) instead of a unique one, allowing multiple devices to share the same id. No corresponding SettingsModel device was created either, breaking the invariant that adapter-config device ids must match SettingsModel device ids. Fix handleAddTab() to call addNewDevice() for a unique id and set the adapter on the resulting SettingsModel entry. Also fix onAdapterChanged() in DeviceConfigTab to preserve the existing device id when the user switches the protocol adapter, preventing the unique id from being overwritten by the new adapter's defaults. https://claude.ai/code/session_012tQA6XhU6TKDgqzUwzJqx1 --- src/customwidgets/deviceconfigtab.cpp | 12 +++++++ src/dialogs/adapterdevicesettings.cpp | 10 ++++++ tests/dialogs/tst_adapterdevicesettings.cpp | 40 ++++++++++++++++++++- tests/dialogs/tst_adapterdevicesettings.h | 1 + 4 files changed, 62 insertions(+), 1 deletion(-) diff --git a/src/customwidgets/deviceconfigtab.cpp b/src/customwidgets/deviceconfigtab.cpp index bd0e0e90..d70efb16 100644 --- a/src/customwidgets/deviceconfigtab.cpp +++ b/src/customwidgets/deviceconfigtab.cpp @@ -84,6 +84,18 @@ void DeviceConfigTab::onAdapterChanged(int index) { defaultValues = defaultDevices.first().toObject(); } + + // Preserve the existing device id so that switching adapters does not overwrite + // the unique id assigned when the tab was created. + if (_pSchemaForm) + { + const int currentId = _pSchemaForm->values().value("id").toInt(-1); + if (currentId >= 0) + { + defaultValues["id"] = currentId; + } + } + rebuildSchemaForm(newAdapterId, defaultValues); } diff --git a/src/dialogs/adapterdevicesettings.cpp b/src/dialogs/adapterdevicesettings.cpp index 1e3bd97e..cda47875 100644 --- a/src/dialogs/adapterdevicesettings.cpp +++ b/src/dialogs/adapterdevicesettings.cpp @@ -63,6 +63,12 @@ AdapterDeviceSettings::AdapterDeviceSettings(SettingsModel* pSettingsModel, QWid } } +/*! \brief Add a new device tab with a unique, auto-incremented device ID. + * + * Creates a SettingsModel device via addNewDevice() to obtain a unique ID, + * sets its adapter, then opens a new DeviceConfigTab pre-populated with + * the adapter's default values and the assigned ID. + */ void AdapterDeviceSettings::handleAddTab() { QString defaultAdapterId; @@ -90,6 +96,10 @@ void AdapterDeviceSettings::handleAddTab() defaultValues = defaultDevices.first().toObject(); } + deviceId_t newId = _pSettingsModel->addNewDevice(); + _pSettingsModel->deviceSettings(newId)->setAdapterId(defaultAdapterId); + defaultValues["id"] = static_cast(newId); + int tabIndex = _pDeviceTabs->count() + 1; auto* tab = new DeviceConfigTab(_pSettingsModel, defaultAdapterId, defaultValues, _pDeviceTabs); _pDeviceTabs->addNewTab(constructTabName(defaultValues, tabIndex), tab); diff --git a/tests/dialogs/tst_adapterdevicesettings.cpp b/tests/dialogs/tst_adapterdevicesettings.cpp index 4fcd842d..e5b56fc1 100644 --- a/tests/dialogs/tst_adapterdevicesettings.cpp +++ b/tests/dialogs/tst_adapterdevicesettings.cpp @@ -229,7 +229,45 @@ void TestAdapterDeviceSettings::addTabUsesDeviceDefaults() auto* spin = tab->findChild(); QVERIFY(spin != nullptr); - QCOMPARE(spin->value(), 5); + // The adapter default id (5) must be overridden with a unique SettingsModel id. + const int assignedId = spin->value(); + QVERIFY(assignedId != 5); + QVERIFY(model.hasDevice(static_cast(assignedId))); + QCOMPARE(model.deviceSettings(static_cast(assignedId))->adapterId(), QStringLiteral("adapterA")); +} + +void TestAdapterDeviceSettings::addTabIncrementsDeviceId() +{ + SettingsModel model; + setupAdapter(model, "adapterA", QJsonArray()); + + AdapterDeviceSettings w(&model); + + auto* tabs = w.findChild(); + QVERIFY(tabs != nullptr); + QCOMPARE(tabs->count(), 0); + + emit tabs->addTabRequested(); + emit tabs->addTabRequested(); + + QCOMPARE(tabs->count(), 2); + + auto* tab0 = qobject_cast(tabs->tabContent(0)); + auto* tab1 = qobject_cast(tabs->tabContent(1)); + QVERIFY(tab0 != nullptr); + QVERIFY(tab1 != nullptr); + + const int id0 = tab0->values().value("id").toInt(-1); + const int id1 = tab1->values().value("id").toInt(-1); + + QVERIFY(id0 >= 1); + QVERIFY(id1 > id0); + + // Both ids must be present in the SettingsModel and linked to the adapter. + QVERIFY(model.hasDevice(static_cast(id0))); + QVERIFY(model.hasDevice(static_cast(id1))); + QCOMPARE(model.deviceSettings(static_cast(id0))->adapterId(), QStringLiteral("adapterA")); + QCOMPARE(model.deviceSettings(static_cast(id1))->adapterId(), QStringLiteral("adapterA")); } QTEST_MAIN(TestAdapterDeviceSettings) diff --git a/tests/dialogs/tst_adapterdevicesettings.h b/tests/dialogs/tst_adapterdevicesettings.h index 5dd19608..7f34ec72 100644 --- a/tests/dialogs/tst_adapterdevicesettings.h +++ b/tests/dialogs/tst_adapterdevicesettings.h @@ -18,6 +18,7 @@ private slots: void acceptValuesSavesToAdapterConfig(); void acceptValuesSavesDeviceNameToModel(); void addTabUsesDeviceDefaults(); + void addTabIncrementsDeviceId(); private: //! Populate \a model with an adapter that has a minimal device schema and From 8c86c51fc6a6eddcd9a1a9a85023982002deb8d8 Mon Sep 17 00:00:00 2001 From: Jens Geudens Date: Thu, 16 Apr 2026 16:55:33 +0200 Subject: [PATCH 2/3] Add regression test for device ID preservation on adapter switch Verify that DeviceConfigTab::onAdapterChanged keeps the assigned device ID unchanged when the user switches to a different adapter. Co-Authored-By: Claude Sonnet 4.6 --- tests/dialogs/tst_adapterdevicesettings.cpp | 36 +++++++++++++++++++++ tests/dialogs/tst_adapterdevicesettings.h | 1 + 2 files changed, 37 insertions(+) diff --git a/tests/dialogs/tst_adapterdevicesettings.cpp b/tests/dialogs/tst_adapterdevicesettings.cpp index e5b56fc1..40ab9366 100644 --- a/tests/dialogs/tst_adapterdevicesettings.cpp +++ b/tests/dialogs/tst_adapterdevicesettings.cpp @@ -5,6 +5,7 @@ #include "dialogs/adapterdevicesettings.h" #include "models/settingsmodel.h" +#include #include #include #include @@ -270,4 +271,39 @@ void TestAdapterDeviceSettings::addTabIncrementsDeviceId() QCOMPARE(model.deviceSettings(static_cast(id1))->adapterId(), QStringLiteral("adapterA")); } +void TestAdapterDeviceSettings::deviceIdPreservedWhenAdapterChanged() +{ + SettingsModel model; + setupAdapter(model, "adapterA", QJsonArray()); + setupAdapter(model, "adapterB", QJsonArray()); + + AdapterDeviceSettings w(&model); + + auto* tabs = w.findChild(); + QVERIFY(tabs != nullptr); + + emit tabs->addTabRequested(); + + QCOMPARE(tabs->count(), 1); + auto* tab = qobject_cast(tabs->tabContent(0)); + QVERIFY(tab != nullptr); + + const int originalId = tab->values().value("id").toInt(-1); + QVERIFY(originalId >= 1); + + // Trigger DeviceConfigTab::onAdapterChanged by switching the adapter combo + auto* adapterCombo = tab->findChild(); + QVERIFY(adapterCombo != nullptr); + int adapterBIdx = adapterCombo->findData(QStringLiteral("adapterB")); + QVERIFY(adapterBIdx >= 0); + adapterCombo->setCurrentIndex(adapterBIdx); + + // The device ID must be preserved across the adapter switch + QCOMPARE(tab->values().value("id").toInt(-1), originalId); + // The device must still exist in the model + QVERIFY(model.hasDevice(static_cast(originalId))); + // The tab must now report the new adapter + QCOMPARE(tab->adapterId(), QStringLiteral("adapterB")); +} + QTEST_MAIN(TestAdapterDeviceSettings) diff --git a/tests/dialogs/tst_adapterdevicesettings.h b/tests/dialogs/tst_adapterdevicesettings.h index 7f34ec72..e8c91900 100644 --- a/tests/dialogs/tst_adapterdevicesettings.h +++ b/tests/dialogs/tst_adapterdevicesettings.h @@ -19,6 +19,7 @@ private slots: void acceptValuesSavesDeviceNameToModel(); void addTabUsesDeviceDefaults(); void addTabIncrementsDeviceId(); + void deviceIdPreservedWhenAdapterChanged(); private: //! Populate \a model with an adapter that has a minimal device schema and From 541a464cb6610dd5970be63073da6ac5a877a41a Mon Sep 17 00:00:00 2001 From: Jens Geudens Date: Thu, 16 Apr 2026 19:30:20 +0200 Subject: [PATCH 3/3] Fall back to constructor device ID when schema form hides the id field When the adapter's schema marks the id field as hidden, SchemaFormWidget does not include it in values(), causing onAdapterChanged to skip the defaultValues["id"] assignment and lose the original device ID. Store _deviceId from the constructor's deviceValues so it can serve as a fallback when the form returns an invalid id. Co-Authored-By: Claude Sonnet 4.6 --- src/customwidgets/deviceconfigtab.cpp | 16 ++++++++-------- src/customwidgets/deviceconfigtab.h | 1 + 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/customwidgets/deviceconfigtab.cpp b/src/customwidgets/deviceconfigtab.cpp index d70efb16..11ce6418 100644 --- a/src/customwidgets/deviceconfigtab.cpp +++ b/src/customwidgets/deviceconfigtab.cpp @@ -21,7 +21,8 @@ DeviceConfigTab::DeviceConfigTab(SettingsModel* pSettingsModel, _pNameEdit(new QLineEdit(this)), _pAdapterCombo(new QComboBox(this)), _pSchemaForm(nullptr), - _pSettingsModel(pSettingsModel) + _pSettingsModel(pSettingsModel), + _deviceId(deviceValues.value("id").toInt(-1)) { _pLayout = new QVBoxLayout(this); setLayout(_pLayout); @@ -86,15 +87,14 @@ void DeviceConfigTab::onAdapterChanged(int index) } // Preserve the existing device id so that switching adapters does not overwrite - // the unique id assigned when the tab was created. - if (_pSchemaForm) + // the unique id assigned when the tab was created. Fall back to _deviceId when + // the id field is hidden in the schema form and not returned by values(). + int currentId = _pSchemaForm ? _pSchemaForm->values().value("id").toInt(-1) : -1; + if (currentId < 0) { - const int currentId = _pSchemaForm->values().value("id").toInt(-1); - if (currentId >= 0) - { - defaultValues["id"] = currentId; - } + currentId = _deviceId; } + defaultValues["id"] = currentId; rebuildSchemaForm(newAdapterId, defaultValues); } diff --git a/src/customwidgets/deviceconfigtab.h b/src/customwidgets/deviceconfigtab.h index d402b868..097c43e9 100644 --- a/src/customwidgets/deviceconfigtab.h +++ b/src/customwidgets/deviceconfigtab.h @@ -57,6 +57,7 @@ private slots: QComboBox* _pAdapterCombo; SchemaFormWidget* _pSchemaForm; SettingsModel* _pSettingsModel; + int _deviceId; }; #endif // DEVICECONFIGTAB_H