Skip to content

Commit fa118d6

Browse files
committed
Remove buildExpression relay from ModbusPoll
AddRegisterWidget and RegisterDialog now reference AdapterManager directly instead of going through ModbusPoll. ModbusPoll gains an adapterManager() getter so MainWindow can pass the manager to RegisterDialog without knowing its internals. AdapterManager::buildExpression is marked virtual to allow test subclassing. MockModbusPoll in tst_addregisterwidget is replaced by MockAdapterManager, which overrides the virtual method and injects results via the buildExpressionResult signal. https://claude.ai/code/session_01AtVwF9u3BD3kqhLUfubEaX
1 parent a4147b1 commit fa118d6

10 files changed

Lines changed: 47 additions & 63 deletions

File tree

src/ProtocolAdapter/adaptermanager.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ class AdapterManager : public QObject
6060
* \param dataType Data type identifier (e.g. "16b"). Pass empty string for the adapter default.
6161
* \param deviceId Device identifier. Pass 0 for the adapter default.
6262
*/
63-
void buildExpression(const QJsonObject& addressFields, const QString& dataType, deviceId_t deviceId);
63+
virtual void buildExpression(const QJsonObject& addressFields, const QString& dataType, deviceId_t deviceId);
6464

6565
/*!
6666
* \brief Route an adapter.diagnostic notification to the diagnostics log.

src/communication/modbuspoll.cpp

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
#include "util/formatdatetime.h"
66
#include "util/scopelogging.h"
77

8-
#include <QJsonArray>
98

109
ModbusPoll::ModbusPoll(SettingsModel* pSettingsModel, QObject* parent) : QObject(parent), _bPollActive(false)
1110
{
@@ -20,7 +19,6 @@ ModbusPoll::ModbusPoll(SettingsModel* pSettingsModel, QObject* parent) : QObject
2019

2120
connect(_pAdapterManager, &AdapterManager::sessionStarted, this, &ModbusPoll::triggerRegisterRead);
2221
connect(_pAdapterManager, &AdapterManager::readDataResult, this, &ModbusPoll::onReadDataResult);
23-
connect(_pAdapterManager, &AdapterManager::buildExpressionResult, this, &ModbusPoll::buildExpressionResult);
2422
connect(_pAdapterManager, &AdapterManager::sessionStopped, this, &ModbusPoll::initAdapter);
2523
connect(_pAdapterManager, &AdapterManager::sessionError, this, [this](QString message) {
2624
qCWarning(scopeComm) << "AdapterClient error:" << message;
@@ -109,15 +107,11 @@ void ModbusPoll::onReadDataResult(ResultDoubleList results)
109107
}
110108
}
111109

112-
/*!
113-
* \brief Request the adapter to construct a register expression from its component parts.
114-
* \param addressFields Address field values from the register schema form.
115-
* \param dataType Data type identifier; empty string uses the adapter default.
116-
* \param deviceId Device identifier; 0 uses the adapter default.
117-
*/
118-
void ModbusPoll::buildExpression(const QJsonObject& addressFields, const QString& dataType, deviceId_t deviceId)
110+
111+
/*! \brief Returns the AdapterManager owned by this instance. */
112+
AdapterManager* ModbusPoll::adapterManager() const
119113
{
120-
_pAdapterManager->buildExpression(addressFields, dataType, deviceId);
114+
return _pAdapterManager;
121115
}
122116

123117
QStringList ModbusPoll::buildRegisterExpressions(const QList<DataPoint>& registerList)

src/communication/modbuspoll.h

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
#include "communication/datapoint.h"
66
#include "util/result.h"
77

8-
#include <QJsonObject>
98
#include <QStringList>
109
#include <QTimer>
1110

@@ -27,25 +26,16 @@ class ModbusPoll : public QObject
2726
void resetCommunicationStats();
2827

2928
/*!
30-
* \brief Request the adapter to construct a register expression string from its component parts.
29+
* \brief Returns the AdapterManager owned by this poll instance.
3130
*
32-
* Delegates to AdapterManager::buildExpression(). Emits buildExpressionResult() on response.
33-
*
34-
* \param addressFields Address field values as returned by the register schema form.
35-
* \param dataType Data type identifier (e.g. "16b"). Pass empty string to use the adapter default.
36-
* \param deviceId Device identifier. Pass 0 to use the adapter default.
31+
* Callers that need to interact with the adapter directly (e.g. to call
32+
* buildExpression()) should use this accessor rather than going through ModbusPoll.
3733
*/
38-
virtual void buildExpression(const QJsonObject& addressFields, const QString& dataType, deviceId_t deviceId);
34+
AdapterManager* adapterManager() const;
3935

4036
signals:
4137
void registerDataReady(ResultDoubleList registers);
4238

43-
/*!
44-
* \brief Emitted when an adapter.buildExpression response has been received.
45-
* \param expression The constructed register expression string (e.g. \c ${h0:f32b}).
46-
*/
47-
void buildExpressionResult(QString expression);
48-
4939
private slots:
5040
void triggerRegisterRead();
5141
void onReadDataResult(ResultDoubleList results);

src/dialogs/addregisterwidget.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
#include "addregisterwidget.h"
22
#include "ui_addregisterwidget.h"
33

4-
#include "communication/modbuspoll.h"
4+
#include "ProtocolAdapter/adaptermanager.h"
55
#include "customwidgets/schemaformwidget.h"
66
#include "models/adapterdata.h"
77
#include "models/device.h"
@@ -14,18 +14,18 @@
1414
* \brief Constructs the widget and populates it from the adapter's register schema.
1515
* \param pSettingsModel Pointer to the application settings model.
1616
* \param adapterId Identifier of the adapter whose register schema to use.
17-
* \param pModbusPoll Pointer to the Modbus poll instance used to build expression strings.
17+
* \param pAdapterManager Pointer to the adapter manager used to build expression strings.
1818
* \param parent Optional parent widget.
1919
*/
2020
AddRegisterWidget::AddRegisterWidget(SettingsModel* pSettingsModel,
2121
const QString& adapterId,
22-
ModbusPoll* pModbusPoll,
22+
AdapterManager* pAdapterManager,
2323
QWidget* parent)
2424
: QWidget(parent),
2525
_pUi(new Ui::AddRegisterWidget),
2626
_pAddressForm(new SchemaFormWidget(this)),
2727
_pSettingsModel(pSettingsModel),
28-
_pModbusPoll(pModbusPoll)
28+
_pAdapterManager(pAdapterManager)
2929
{
3030
_pUi->setupUi(this);
3131

@@ -77,7 +77,7 @@ AddRegisterWidget::AddRegisterWidget(SettingsModel* pSettingsModel,
7777
}
7878

7979
connect(_pUi->btnAdd, &QPushButton::clicked, this, &AddRegisterWidget::handleResultAccept);
80-
connect(_pModbusPoll, &ModbusPoll::buildExpressionResult, this, &AddRegisterWidget::onBuildExpressionResult);
80+
connect(_pAdapterManager, &AdapterManager::buildExpressionResult, this, &AddRegisterWidget::onBuildExpressionResult);
8181

8282
_axisGroup.setExclusive(true);
8383
_axisGroup.addButton(_pUi->radioPrimary);
@@ -111,7 +111,7 @@ void AddRegisterWidget::handleResultAccept()
111111
}
112112

113113
_pUi->btnAdd->setEnabled(false);
114-
_pModbusPoll->buildExpression(addressValues, typeId, deviceId);
114+
_pAdapterManager->buildExpression(addressValues, typeId, deviceId);
115115
}
116116

117117
void AddRegisterWidget::onBuildExpressionResult(const QString& expression)

src/dialogs/addregisterwidget.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
#include <QJsonObject>
99
#include <QWidget>
1010

11-
class ModbusPoll;
11+
class AdapterManager;
1212
class SchemaFormWidget;
1313
class SettingsModel;
1414

@@ -25,7 +25,7 @@ class AddRegisterWidget : public QWidget
2525
public:
2626
explicit AddRegisterWidget(SettingsModel* pSettingsModel,
2727
const QString& adapterId,
28-
ModbusPoll* pModbusPoll,
28+
AdapterManager* pAdapterManager,
2929
QWidget* parent = nullptr);
3030
~AddRegisterWidget();
3131

@@ -46,7 +46,7 @@ private slots:
4646
int _defaultTypeIndex{ 0 };
4747

4848
SettingsModel* _pSettingsModel;
49-
ModbusPoll* _pModbusPoll;
49+
AdapterManager* _pAdapterManager;
5050

5151
QButtonGroup _axisGroup;
5252

src/dialogs/mainwindow.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -397,7 +397,7 @@ void MainWindow::showRegisterDialog()
397397
_pGuiModel->setGuiState(GuiState::INIT);
398398
}
399399

400-
RegisterDialog registerDialog(_pGraphDataModel, _pSettingsModel, _pModbusPoll, this);
400+
RegisterDialog registerDialog(_pGraphDataModel, _pSettingsModel, _pModbusPoll->adapterManager(), this);
401401
registerDialog.exec();
402402
}
403403

src/dialogs/registerdialog.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11

22
#include "registerdialog.h"
33

4-
#include "communication/modbuspoll.h"
4+
#include "ProtocolAdapter/adaptermanager.h"
55
#include "customwidgets/actionbuttondelegate.h"
66
#include "dialogs/addregisterwidget.h"
77
#include "dialogs/expressionsdialog.h"
@@ -14,7 +14,7 @@
1414

1515
RegisterDialog::RegisterDialog(GraphDataModel* pGraphDataModel,
1616
SettingsModel* pSettingsModel,
17-
ModbusPoll* pModbusPoll,
17+
AdapterManager* pAdapterManager,
1818
QWidget* parent)
1919
: QDialog(parent), _pUi(new Ui::RegisterDialog)
2020
{
@@ -25,7 +25,7 @@ RegisterDialog::RegisterDialog(GraphDataModel* pGraphDataModel,
2525

2626
_pGraphDataModel = pGraphDataModel;
2727
_pSettingsModel = pSettingsModel;
28-
_pModbusPoll = pModbusPoll;
28+
_pAdapterManager = pAdapterManager;
2929

3030
// Setup registerView
3131
_pUi->registerView->setModel(_pGraphDataModel);
@@ -76,7 +76,7 @@ RegisterDialog::RegisterDialog(GraphDataModel* pGraphDataModel,
7676
const QString adapterId = ids.isEmpty() ? QString() : ids.first();
7777
if (!adapterId.isEmpty())
7878
{
79-
auto registerPopupMenu = new AddRegisterWidget(_pSettingsModel, adapterId, _pModbusPoll, this);
79+
auto registerPopupMenu = new AddRegisterWidget(_pSettingsModel, adapterId, _pAdapterManager, this);
8080
connect(registerPopupMenu, &AddRegisterWidget::graphDataConfigured, this, &RegisterDialog::addRegister);
8181

8282
_registerPopupAction = std::make_unique<QWidgetAction>(this);

src/dialogs/registerdialog.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
/* Forward declaration */
1010
class GraphDataModel;
11-
class ModbusPoll;
11+
class AdapterManager;
1212
class SettingsModel;
1313
class RegisterValueAxisDelegate;
1414
class ActionButtonDelegate;
@@ -25,7 +25,7 @@ class RegisterDialog : public QDialog
2525
public:
2626
explicit RegisterDialog(GraphDataModel* pGraphDataModel,
2727
SettingsModel* pSettingsModel,
28-
ModbusPoll* pModbusPoll,
28+
AdapterManager* pAdapterManager,
2929
QWidget* parent = nullptr);
3030
~RegisterDialog();
3131

@@ -45,7 +45,7 @@ private slots:
4545

4646
GraphDataModel* _pGraphDataModel;
4747
SettingsModel* _pSettingsModel;
48-
ModbusPoll* _pModbusPoll;
48+
AdapterManager* _pAdapterManager;
4949

5050
CenteredBoxProxyStyle _centeredBoxStyle;
5151

tests/dialogs/tst_addregisterwidget.cpp

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -66,16 +66,16 @@ void TestAddRegisterWidget::init()
6666
_settingsModel.setAdapterRegisterSchema("modbus", buildTestRegisterSchema());
6767
_settingsModel.deviceSettings(Device::cFirstDeviceId)->setAdapterId("modbus");
6868

69-
_pMockModbusPoll = new MockModbusPoll(&_settingsModel);
70-
_pRegWidget = new AddRegisterWidget(&_settingsModel, QStringLiteral("modbus"), _pMockModbusPoll);
69+
_pMockAdapterManager = new MockAdapterManager(&_settingsModel);
70+
_pRegWidget = new AddRegisterWidget(&_settingsModel, QStringLiteral("modbus"), _pMockAdapterManager);
7171
}
7272

7373
void TestAddRegisterWidget::cleanup()
7474
{
7575
delete _pRegWidget;
7676
_pRegWidget = nullptr;
77-
delete _pMockModbusPoll;
78-
_pMockModbusPoll = nullptr;
77+
delete _pMockAdapterManager;
78+
_pMockAdapterManager = nullptr;
7979
}
8080

8181
void TestAddRegisterWidget::registerDefault()
@@ -94,11 +94,11 @@ void TestAddRegisterWidget::registerDefault()
9494
QCOMPARE(graphData.expression(), QStringLiteral("${h100}"));
9595
QVERIFY(graphData.isActive());
9696

97-
QCOMPARE(_pMockModbusPoll->buildCalls.size(), 1);
98-
QCOMPARE(_pMockModbusPoll->buildCalls[0].fields["objectType"].toString(), QStringLiteral("holding-register"));
99-
QCOMPARE(_pMockModbusPoll->buildCalls[0].fields["address"].toInt(), 100);
100-
QCOMPARE(_pMockModbusPoll->buildCalls[0].dataType, QStringLiteral("16b"));
101-
QCOMPARE(_pMockModbusPoll->buildCalls[0].deviceId, Device::cFirstDeviceId);
97+
QCOMPARE(_pMockAdapterManager->buildCalls.size(), 1);
98+
QCOMPARE(_pMockAdapterManager->buildCalls[0].fields["objectType"].toString(), QStringLiteral("holding-register"));
99+
QCOMPARE(_pMockAdapterManager->buildCalls[0].fields["address"].toInt(), 100);
100+
QCOMPARE(_pMockAdapterManager->buildCalls[0].dataType, QStringLiteral("16b"));
101+
QCOMPARE(_pMockAdapterManager->buildCalls[0].deviceId, Device::cFirstDeviceId);
102102
}
103103

104104
void TestAddRegisterWidget::registerType()
@@ -114,7 +114,7 @@ void TestAddRegisterWidget::registerType()
114114
addRegister(graphData, QStringLiteral("${h0:32b}"));
115115

116116
QCOMPARE(graphData.expression(), QStringLiteral("${h0:32b}"));
117-
QCOMPARE(_pMockModbusPoll->buildCalls[0].dataType, QStringLiteral("32b"));
117+
QCOMPARE(_pMockAdapterManager->buildCalls[0].dataType, QStringLiteral("32b"));
118118
}
119119

120120
void TestAddRegisterWidget::registerObjectType()
@@ -127,7 +127,7 @@ void TestAddRegisterWidget::registerObjectType()
127127
addRegister(graphData, QStringLiteral("${i0}"));
128128

129129
QCOMPARE(graphData.expression(), QStringLiteral("${i0}"));
130-
QCOMPARE(_pMockModbusPoll->buildCalls[0].fields["objectType"].toString(), QStringLiteral("input-register"));
130+
QCOMPARE(_pMockAdapterManager->buildCalls[0].fields["objectType"].toString(), QStringLiteral("input-register"));
131131
}
132132

133133
void TestAddRegisterWidget::registerDevice()
@@ -138,7 +138,7 @@ void TestAddRegisterWidget::registerDevice()
138138
const deviceId_t devId2 = _settingsModel.addNewDevice();
139139
_settingsModel.deviceSettings(devId2)->setAdapterId("modbus");
140140

141-
_pRegWidget = new AddRegisterWidget(&_settingsModel, QStringLiteral("modbus"), _pMockModbusPoll);
141+
_pRegWidget = new AddRegisterWidget(&_settingsModel, QStringLiteral("modbus"), _pMockAdapterManager);
142142

143143
_pRegWidget->_pAddressForm->setSchema(
144144
buildAddressSchema(), QJsonObject{ { QStringLiteral("objectType"), QStringLiteral("holding-register") },
@@ -151,7 +151,7 @@ void TestAddRegisterWidget::registerDevice()
151151
addRegister(graphData, QStringLiteral("${h0@2}"));
152152

153153
QCOMPARE(graphData.expression(), QStringLiteral("${h0@2}"));
154-
QCOMPARE(_pMockModbusPoll->buildCalls[0].deviceId, devId2);
154+
QCOMPARE(_pMockAdapterManager->buildCalls[0].deviceId, devId2);
155155
}
156156

157157
void TestAddRegisterWidget::registerValueAxis()
@@ -171,7 +171,7 @@ void TestAddRegisterWidget::buildExpressionEmptyResponseIgnored()
171171
clickAdd();
172172

173173
/* Adapter returns empty expression — graphDataConfigured must not be emitted */
174-
_pMockModbusPoll->injectBuildExpressionResult(QString());
174+
_pMockAdapterManager->injectBuildExpressionResult(QString());
175175

176176
QCOMPARE(spy.count(), 0);
177177
/* Button should be re-enabled even on empty response */
@@ -190,7 +190,7 @@ void TestAddRegisterWidget::addRegister(GraphData& graphData, const QString& exp
190190
clickAdd();
191191

192192
/* Simulate the adapter returning the expression string */
193-
_pMockModbusPoll->injectBuildExpressionResult(expression);
193+
_pMockAdapterManager->injectBuildExpressionResult(expression);
194194

195195
QCOMPARE(spyGraphDataConfigured.count(), 1);
196196

tests/dialogs/tst_addregisterwidget.h

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
#ifndef TST_ADDREGISTERWIDGET_H
33
#define TST_ADDREGISTERWIDGET_H
44

5-
#include "communication/modbuspoll.h"
5+
#include "ProtocolAdapter/adaptermanager.h"
66
#include "models/graphdata.h"
77
#include "models/settingsmodel.h"
88

@@ -12,12 +12,12 @@
1212
class AddRegisterWidget;
1313

1414
/*!
15-
* \brief Test double for ModbusPoll.
15+
* \brief Test double for AdapterManager.
1616
*
1717
* Captures buildExpression() calls and provides an inject helper to simulate
1818
* the adapter.buildExpression response without a real adapter process.
1919
*/
20-
class MockModbusPoll : public ModbusPoll
20+
class MockAdapterManager : public AdapterManager
2121
{
2222
Q_OBJECT
2323

@@ -29,8 +29,8 @@ class MockModbusPoll : public ModbusPoll
2929
deviceId_t deviceId;
3030
};
3131

32-
explicit MockModbusPoll(SettingsModel* pSettingsModel, QObject* parent = nullptr)
33-
: ModbusPoll(pSettingsModel, parent)
32+
explicit MockAdapterManager(SettingsModel* pSettingsModel, QObject* parent = nullptr)
33+
: AdapterManager(pSettingsModel, parent)
3434
{
3535
}
3636

@@ -70,7 +70,7 @@ private slots:
7070
static QJsonObject buildTestRegisterSchema();
7171

7272
SettingsModel _settingsModel;
73-
MockModbusPoll* _pMockModbusPoll{ nullptr };
73+
MockAdapterManager* _pMockAdapterManager{ nullptr };
7474
AddRegisterWidget* _pRegWidget{ nullptr };
7575
};
7676

0 commit comments

Comments
 (0)