Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
42df9e0
Fix connection ID not incrementing when adding a third connection
jgeudens Apr 17, 2026
004d132
Fix conditional fields not shown when if block has no required array
jgeudens Apr 18, 2026
4635473
Fix violations
jgeudens Apr 18, 2026
e5357cf
Update connection name when adding new connection
jgeudens Apr 18, 2026
296f9b8
Drive connection tab name from the name field with live updates
jgeudens Apr 18, 2026
fed85bd
Drive device tab name from the name field with live updates
jgeudens Apr 18, 2026
f736f80
Add fail-safe name
jgeudens Apr 18, 2026
8107c9b
Remove trailing 's' from property field
jgeudens Apr 18, 2026
2accd6f
Guard formatTabName against empty label and short keys
jgeudens Apr 19, 2026
5ce93db
Fall back to auto-generated name when device name is cleared
jgeudens Apr 19, 2026
df7b400
Derive new item id from max existing id to avoid collisions
jgeudens Apr 19, 2026
107134b
Extract tab-name update logic into onSchemaFieldNameChanged helper
jgeudens Apr 19, 2026
dc21962
Replace multi-capture lambda with QSignalSpy in fieldChanged test
jgeudens Apr 19, 2026
b1b88ea
Add deviceId() accessor to DeviceConfigTab
jgeudens Apr 19, 2026
1a991bc
Emit QWidget* from tabClosed to remove ordering dependency
jgeudens Apr 19, 2026
9ed943b
Remove orphaned Device from model when device tab is closed
jgeudens Apr 19, 2026
506d699
Fix three code-review warnings in DeviceForm and DeviceConfigTab
jgeudens Apr 19, 2026
9029d78
Fix device name reset to empty on settings dialog reopen
jgeudens Apr 19, 2026
dd30fec
Fix new connection name not matching its ID for non-contiguous IDs
jgeudens Apr 19, 2026
4bf5cac
Refactor addItemTab() to use local nameIndex instead of mutating _nex…
jgeudens Apr 19, 2026
2654cc4
Fix name when adding device
jgeudens Apr 19, 2026
0d91c2d
Simplify and refactor module
jgeudens Apr 19, 2026
d27fd7e
Apply code reviewer follow-up fixes to AdapterDeviceSettings
jgeudens Apr 19, 2026
86ac968
Fix new device getting gap-fill ID instead of max+1
jgeudens Apr 20, 2026
80eb9f8
Improve device adding/removing
jgeudens Apr 20, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion adapters/describe.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
{
"id": 1,
"ip": "127.0.0.1",
"name": "Connection 1",
"persistent": true,
"port": 502,
"timeout": 1000,
Expand All @@ -21,6 +22,7 @@
"baudrate": 115200,
"databits": 8,
"id": 2,
"name": "Connection 2",
"parity": "N",
"persistent": true,
"portName": "COM1",
Expand Down Expand Up @@ -147,6 +149,11 @@
"title": "Connection ID",
"type": "integer"
},
"name": {
"description": "Human-readable connection name (default: \"Connection <id>\")",
"title": "Connection Name",
"type": "string"
},
"persistent": {
"description": "Keep connection open between reads",
"title": "Keep Connection Persistent",
Expand Down Expand Up @@ -258,5 +265,5 @@
},
"type": "object"
},
"version": "0.0.1-master"
"version": "0.0.1-master:ac7eec7"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Keep the version format aligned with the JSON-RPC spec.

Line 268 now advertises 0.0.1-master:ac7eec7, but the spec documents debug versions as "<semver>-<git-branch>". The extra :<hash> suffix is undocumented and can break consumers that parse or compare the version using the published format.

Proposed format alignment
-    "version": "0.0.1-master:ac7eec7"
+    "version": "0.0.1-master"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@adapters/describe.json` at line 268, The "version" field currently contains
an undocumented suffix ("0.0.1-master:ac7eec7"); update the value of the
"version" property in adapters/describe.json to follow the JSON-RPC spec debug
format "<semver>-<git-branch>" (e.g., "0.0.1-master") by removing the ":<hash>"
suffix so consumers that parse or compare the "version" key will match the
documented format.

}
Binary file modified adapters/dummymodbusadapter
Binary file not shown.
Binary file modified adapters/dummymodbusadapter.exe
Binary file not shown.
1 change: 1 addition & 0 deletions adapters/json-rpc-spec.md
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ Adapter-wide settings. Currently empty; reserved for future use.
| Field | Type | Required | Description |
| --- | --- | --- | --- |
| `id` | integer | yes | Connection index: `0`, `1`, or `2` |
| `name` | string | no | Human-readable label (default: `"Connection <id>"`) |
| `type` | string | yes | `"tcp"` or `"serial"` |
| `timeout` | integer | no | Timeout in milliseconds (default: `1000`) |
| `persistent` | boolean | no | Keep connection open between reads (default: `false`) |
Expand Down
Binary file modified adapters/modbusadapter
Binary file not shown.
Binary file modified adapters/modbusadapter.exe
Binary file not shown.
3 changes: 1 addition & 2 deletions src/customwidgets/addabletabwidget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,8 @@ void AddableTabWidget::handleCloseTab(int index)
return;
}

emit tabClosed(index);

QWidget* page = widget(index);
emit tabClosed(page);
removeTab(index);
if (page)
{
Expand Down
2 changes: 1 addition & 1 deletion src/customwidgets/addabletabwidget.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class AddableTabWidget : public QTabWidget
QWidget* tabContent(int index) const;

signals:
void tabClosed(int index);
void tabClosed(QWidget* widget);
void addTabRequested(); // Emitted when user clicks the "+" button

public slots:
Expand Down
30 changes: 24 additions & 6 deletions src/customwidgets/deviceconfigtab.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,13 @@ DeviceConfigTab::DeviceConfigTab(SettingsModel* pSettingsModel,
const QJsonObject& deviceValues,
QWidget* parent)
: QWidget(parent),
_pLayout(nullptr),
_pLayout(new QVBoxLayout(this)),
_pNameEdit(new QLineEdit(this)),
_pAdapterCombo(new QComboBox(this)),
_pSchemaForm(nullptr),
_pSettingsModel(pSettingsModel),
_deviceId(deviceValues.value("id").toInt(-1))
{
_pLayout = new QVBoxLayout(this);
setLayout(_pLayout);

auto* nameRow = new QHBoxLayout;
Expand All @@ -33,10 +32,9 @@ DeviceConfigTab::DeviceConfigTab(SettingsModel* pSettingsModel,
nameRow->addStretch();
_pLayout->addLayout(nameRow);

int deviceId = deviceValues.value("id").toInt(-1);
if (deviceId >= 0 && pSettingsModel->deviceList().contains(static_cast<deviceId_t>(deviceId)))
if (_deviceId >= 0 && pSettingsModel->hasDevice(static_cast<deviceId_t>(_deviceId)))
{
_pNameEdit->setText(pSettingsModel->deviceSettings(static_cast<deviceId_t>(deviceId))->name());
_pNameEdit->setText(pSettingsModel->deviceSettings(static_cast<deviceId_t>(_deviceId))->name());
}

auto* adapterRow = new QHBoxLayout;
Expand Down Expand Up @@ -66,6 +64,7 @@ DeviceConfigTab::DeviceConfigTab(SettingsModel* pSettingsModel,
_pAdapterCombo->setCurrentIndex(idx);
}

connect(_pNameEdit, &QLineEdit::textChanged, this, &DeviceConfigTab::onNameChanged);
connect(_pAdapterCombo, QOverload<int>::of(&QComboBox::currentIndexChanged), this,
&DeviceConfigTab::onAdapterChanged);

Expand Down Expand Up @@ -96,15 +95,29 @@ void DeviceConfigTab::onAdapterChanged(int index)
}
defaultValues["id"] = currentId;

if (_deviceId >= 0 && _pSettingsModel->hasDevice(static_cast<deviceId_t>(_deviceId)))
{
_pSettingsModel->deviceSettings(static_cast<deviceId_t>(_deviceId))->setAdapterId(newAdapterId);
}

rebuildSchemaForm(newAdapterId, defaultValues);
}

void DeviceConfigTab::onNameChanged(const QString& name)
{
if (_deviceId >= 0 && _pSettingsModel->hasDevice(static_cast<deviceId_t>(_deviceId)))
{
_pSettingsModel->deviceSettings(static_cast<deviceId_t>(_deviceId))->setName(name);
}
emit nameChanged(name);
}
Comment on lines +106 to +113
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Name edits and tab close write to a stale _deviceId after the user changes the id field.

_deviceId is captured once in the constructor from deviceValues["id"] and never updated, but the schema form exposes id as an editable spinbox (test addTabAfterIdEditDoesNotDuplicate exercises that path). Once the user edits the id:

  • onNameChanged writes setName(name) to the model entry keyed by the old _deviceId rather than the currently displayed id.
  • AdapterDeviceSettings::handleCloseTab calls removeDevice(tab->deviceId()), so closing the tab removes the wrong (old) device from the model while acceptValues() persists the new id into adapter config.
  • The tab title from constructTabName is also resolved against the stale id.

Consider keeping _deviceId in sync with the schema form (e.g. listen to SchemaFormWidget::fieldChanged for the id field, or always re-read _pSchemaForm->values().value("id") before using it), or disable editing of the id field in the UI so the two sources of truth cannot diverge.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/customwidgets/deviceconfigtab.cpp` around lines 106 - 113, The issue is
that _deviceId is captured once and becomes stale causing onNameChanged,
constructTabName and AdapterDeviceSettings::handleCloseTab to operate on the
wrong device; fix by keeping the tab's id in sync with the schema form: either
subscribe to SchemaFormWidget::fieldChanged for the "id" field (update the tab's
_deviceId whenever that field changes) or, alternatively, stop using the stored
_deviceId in onNameChanged, constructTabName and where handleCloseTab reads
tab->deviceId() and instead read the current id from
_pSchemaForm->values().value("id") (cast to deviceId_t) immediately before any
model operations (also ensure acceptValues() still writes the new id to the
adapter config).


void DeviceConfigTab::rebuildSchemaForm(const QString& adapterId, const QJsonObject& deviceValues)
{
if (_pSchemaForm)
{
_pLayout->removeWidget(_pSchemaForm);
delete _pSchemaForm;
_pSchemaForm->deleteLater();
_pSchemaForm = nullptr;
}

Expand Down Expand Up @@ -137,3 +150,8 @@ QString DeviceConfigTab::deviceName() const
{
return _pNameEdit->text();
}

int DeviceConfigTab::deviceId() const
{
return _deviceId;
}
11 changes: 11 additions & 0 deletions src/customwidgets/deviceconfigtab.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,19 @@ class DeviceConfigTab : public QWidget
*/
QString deviceName() const;

/*!
* \brief Return the device ID assigned to this tab, or -1 if none.
* \return Device ID as int.
*/
int deviceId() const;

signals:
//! Emitted when the device name field changes.
void nameChanged(const QString& name);

private slots:
void onAdapterChanged(int index);
void onNameChanged(const QString& name);

private:
void rebuildSchemaForm(const QString& adapterId, const QJsonObject& deviceValues);
Expand Down
17 changes: 10 additions & 7 deletions src/customwidgets/deviceform.cpp
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
#include "deviceform.h"
#include "ui_deviceform.h"

DeviceForm::DeviceForm(SettingsModel* pSettingsModel, deviceId_t _deviceId, QWidget* parent)
: QWidget(parent), _pUi(new Ui::DeviceForm), _pSettingsModel(pSettingsModel), _deviceId(_deviceId)
DeviceForm::DeviceForm(SettingsModel* pSettingsModel, deviceId_t deviceId, QWidget* parent)
: QWidget(parent), _pUi(new Ui::DeviceForm), _pSettingsModel(pSettingsModel), _deviceId(deviceId)
{
_pUi->setupUi(this);

Expand All @@ -11,11 +11,7 @@ DeviceForm::DeviceForm(SettingsModel* pSettingsModel, deviceId_t _deviceId, QWid
_pUi->lineName->setText(dev->name());
_pUi->spinId->setValue(static_cast<int>(this->_deviceId));

connect(_pUi->lineName, &QLineEdit::textChanged, this, [this](const QString& newName) {
Device* device = _pSettingsModel->deviceSettings(this->_deviceId);
device->setName(newName);
emit deviceIdentifiersChanged(this->_deviceId);
});
connect(_pUi->lineName, &QLineEdit::textChanged, this, &DeviceForm::onNameChanged);
connect(_pUi->spinId, QOverload<int>::of(&QSpinBox::valueChanged), this, [this](int newId) {
_pSettingsModel->updateDeviceId(this->_deviceId, static_cast<deviceId_t>(newId));
this->_deviceId = newId;
Expand All @@ -32,3 +28,10 @@ deviceId_t DeviceForm::deviceId()
{
return _deviceId;
}

void DeviceForm::onNameChanged(const QString& newName)
{
Device* device = _pSettingsModel->deviceSettings(_deviceId);
device->setName(newName);
emit deviceIdentifiersChanged(_deviceId);
}
5 changes: 4 additions & 1 deletion src/customwidgets/deviceform.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,17 @@ class DeviceForm : public QWidget
Q_OBJECT

public:
explicit DeviceForm(SettingsModel* pSettingsModel, deviceId_t _deviceId, QWidget* parent = nullptr);
explicit DeviceForm(SettingsModel* pSettingsModel, deviceId_t deviceId, QWidget* parent = nullptr);
~DeviceForm();

deviceId_t deviceId();

signals:
void deviceIdentifiersChanged(deviceId_t devId);

private slots:
void onNameChanged(const QString& newName);

private:
Ui::DeviceForm* _pUi;

Expand Down
26 changes: 22 additions & 4 deletions src/customwidgets/schemaformwidget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ void SchemaFormWidget::setSchema(const QJsonObject& schema, const QJsonObject& v

QString label = propSchema.value("title").toString(key);
QWidget* widget = createWidgetForProperty(propSchema, currentValue);
wireFieldChanged(key, widget);
_fields.append({ key, widget });
_pFormLayout->addRow(label + ":", widget);
}
Expand All @@ -90,6 +91,7 @@ void SchemaFormWidget::setSchema(const QJsonObject& schema, const QJsonObject& v
QJsonObject propSchema = thenProps.value(key).toObject();
QString label = propSchema.value("title").toString(key);
QWidget* widget = createWidgetForProperty(propSchema, values.value(key));
wireFieldChanged(key, widget);
_fields.append({ key, widget });
_pFormLayout->addRow(label + ":", widget);
}
Expand All @@ -100,6 +102,7 @@ void SchemaFormWidget::setSchema(const QJsonObject& schema, const QJsonObject& v
QJsonObject propSchema = elseProps.value(key).toObject();
QString label = propSchema.value("title").toString(key);
QWidget* widget = createWidgetForProperty(propSchema, values.value(key));
wireFieldChanged(key, widget);
_fields.append({ key, widget });
_pFormLayout->addRow(label + ":", widget);
}
Expand Down Expand Up @@ -212,6 +215,16 @@ QWidget* SchemaFormWidget::createWidgetForProperty(const QJsonObject& propSchema
}
}

void SchemaFormWidget::wireFieldChanged(const QString& key, QWidget* widget)
{
auto* edit = qobject_cast<QLineEdit*>(widget);
if (edit == nullptr)
{
return;
}
connect(edit, &QLineEdit::textChanged, this, [this, key](const QString& text) { emit fieldChanged(key, text); });
}

bool SchemaFormWidget::parseConditional(const QJsonObject& schema)
{
const QJsonObject ifObj = schema.value("if").toObject();
Expand All @@ -223,19 +236,24 @@ bool SchemaFormWidget::parseConditional(const QJsonObject& schema)
return false;
}

const QJsonObject ifProperties = ifObj.value("properties").toObject();

QString triggerKey;
const QJsonArray required = ifObj.value("required").toArray();
if (required.size() != 1)
if (required.size() == 1)
{
return false;
triggerKey = required.at(0).toString();
}
else if (ifProperties.size() == 1)
{
triggerKey = ifProperties.constBegin().key();
}

const QString triggerKey = required.at(0).toString();
if (triggerKey.isEmpty())
{
return false;
}

const QJsonObject ifProperties = ifObj.value("properties").toObject();
const QJsonObject constObj = ifProperties.value(triggerKey).toObject();
if (!constObj.contains("const"))
{
Expand Down
7 changes: 7 additions & 0 deletions src/customwidgets/schemaformwidget.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,20 @@ class SchemaFormWidget : public QWidget
*/
QJsonObject values() const;

signals:
//! Emitted when a string field's value changes; \a key is the property name.
void fieldChanged(const QString& key, const QString& value);

private slots:
//! Called when the trigger combo selection changes; re-evaluates conditional visibility.
void onTriggerChanged(int index);

private:
QWidget* createWidgetForProperty(const QJsonObject& propSchema, const QJsonValue& value);

//! If \a widget is a QLineEdit, connects its textChanged to emit fieldChanged(\a key).
void wireFieldChanged(const QString& key, QWidget* widget);

/*!
* \brief Parse the top-level \c if/then/else block and populate conditional state.
*
Expand Down
Loading