Skip to content

Commit c91f778

Browse files
committed
Apply code review fixes and add level-mapping tests
Guard against malformed adapter.diagnostic params in onNotificationReceived: if params is not a JSON object the signal is not emitted and a warning is logged instead of silently forwarding empty strings. Add explicit "warning" branch to onAdapterDiagnostic and an else branch that logs the unknown level so unrecognised values are visible rather than being silently promoted. New test diagnosticMalformedParams verifies no signal is emitted for non-object params. New tst_modbuspoll covers all four branches of the level-mapping logic (debug, info, warning, unknown) using a captured Qt message handler. https://claude.ai/code/session_01UhRoTiKzKdkuurvzKQBVjr
1 parent fd99904 commit c91f778

8 files changed

Lines changed: 143 additions & 5 deletions

File tree

src/ProtocolAdapter/adapterclient.cpp

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -162,12 +162,20 @@ void AdapterClient::onHandshakeTimeout()
162162

163163
void AdapterClient::onNotificationReceived(QString method, QJsonValue params)
164164
{
165-
if (method == QStringLiteral("adapter.diagnostic"))
165+
if (method != QStringLiteral("adapter.diagnostic"))
166166
{
167-
QJsonObject obj = params.toObject();
168-
emit diagnosticReceived(obj.value(QStringLiteral("level")).toString(),
169-
obj.value(QStringLiteral("message")).toString());
167+
return;
168+
}
169+
170+
if (!params.isObject())
171+
{
172+
qCWarning(scopeComm) << "AdapterClient: adapter.diagnostic params is not an object";
173+
return;
170174
}
175+
176+
QJsonObject obj = params.toObject();
177+
emit diagnosticReceived(obj.value(QStringLiteral("level")).toString(),
178+
obj.value(QStringLiteral("message")).toString());
171179
}
172180

173181
void AdapterClient::handleLifecycleResponse(const QString& method, const QJsonObject& result)

src/communication/modbuspoll.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,10 +138,14 @@ void ModbusPoll::onAdapterDiagnostic(const QString& level, const QString& messag
138138
{
139139
qCInfo(scopeComm) << message;
140140
}
141-
else
141+
else if (level == QStringLiteral("warning"))
142142
{
143143
qCWarning(scopeComm) << message;
144144
}
145+
else
146+
{
147+
qCWarning(scopeComm) << "AdapterClient: unknown diagnostic level:" << level << "-" << message;
148+
}
145149
}
146150

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

tests/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ endfunction()
4949

5050
set(CMAKE_AUTOUIC_SEARCH_PATHS ${CMAKE_CURRENT_SOURCE_DIR}/../src/dialogs ${CMAKE_CURRENT_SOURCE_DIR}/../src/customwidgets)
5151

52+
add_subdirectory(communication)
5253
add_subdirectory(customwidgets)
5354
add_subdirectory(datahandling)
5455
add_subdirectory(dialogs)

tests/ProtocolAdapter/tst_adapterclient.cpp

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -321,6 +321,19 @@ void TestAdapterClient::diagnosticNotificationDebugLevel()
321321
QCOMPARE(spyDiagnostic.at(0).at(1).toString(), QStringLiteral("polling started"));
322322
}
323323

324+
void TestAdapterClient::diagnosticMalformedParams()
325+
{
326+
auto* mock = new MockAdapterProcess();
327+
AdapterClient client(mock);
328+
329+
QSignalSpy spyDiagnostic(&client, &AdapterClient::diagnosticReceived);
330+
331+
/* params is not an object — diagnosticReceived must not be emitted */
332+
mock->injectNotification("adapter.diagnostic", QJsonValue(42));
333+
334+
QCOMPARE(spyDiagnostic.count(), 0);
335+
}
336+
324337
void TestAdapterClient::processErrorEmitsSessionError()
325338
{
326339
auto* mock = new MockAdapterProcess();

tests/ProtocolAdapter/tst_adapterclient.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ private slots:
2020
void notificationIgnored();
2121
void diagnosticNotificationForwarded();
2222
void diagnosticNotificationDebugLevel();
23+
void diagnosticMalformedParams();
2324
void processErrorEmitsSessionError();
2425
void stopSessionDuringLifecycle();
2526
void doubleStopSession();

tests/communication/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
add_xtest(tst_modbuspoll)
Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
#include "tst_modbuspoll.h"
2+
3+
#include "communication/modbuspoll.h"
4+
#include "models/settingsmodel.h"
5+
6+
#include <QLoggingCategory>
7+
#include <QTest>
8+
9+
namespace
10+
{
11+
QtMsgType g_capturedType{};
12+
QString g_capturedMessage{};
13+
14+
void captureHandler(QtMsgType type, const QMessageLogContext&, const QString& msg)
15+
{
16+
g_capturedType = type;
17+
g_capturedMessage = msg;
18+
}
19+
}
20+
21+
void TestModbusPoll::init()
22+
{
23+
_pSettingsModel = new SettingsModel;
24+
_pModbusPoll = new ModbusPoll(_pSettingsModel);
25+
/* Enable debug output for scope.comm so qCDebug calls reach the handler */
26+
QLoggingCategory::setFilterRules(QStringLiteral("scope.comm.debug=true"));
27+
}
28+
29+
void TestModbusPoll::cleanup()
30+
{
31+
QLoggingCategory::setFilterRules(QString());
32+
delete _pModbusPoll;
33+
delete _pSettingsModel;
34+
}
35+
36+
void TestModbusPoll::diagnosticDebugLevel()
37+
{
38+
QtMessageHandler previous = qInstallMessageHandler(captureHandler);
39+
QMetaObject::invokeMethod(_pModbusPoll, "onAdapterDiagnostic",
40+
Q_ARG(QString, QStringLiteral("debug")),
41+
Q_ARG(QString, QStringLiteral("polling started")));
42+
qInstallMessageHandler(previous);
43+
44+
QCOMPARE(g_capturedType, QtDebugMsg);
45+
QVERIFY(g_capturedMessage.contains(QStringLiteral("polling started")));
46+
}
47+
48+
void TestModbusPoll::diagnosticInfoLevel()
49+
{
50+
QtMessageHandler previous = qInstallMessageHandler(captureHandler);
51+
QMetaObject::invokeMethod(_pModbusPoll, "onAdapterDiagnostic",
52+
Q_ARG(QString, QStringLiteral("info")),
53+
Q_ARG(QString, QStringLiteral("session active")));
54+
qInstallMessageHandler(previous);
55+
56+
QCOMPARE(g_capturedType, QtInfoMsg);
57+
QVERIFY(g_capturedMessage.contains(QStringLiteral("session active")));
58+
}
59+
60+
void TestModbusPoll::diagnosticWarningLevel()
61+
{
62+
QtMessageHandler previous = qInstallMessageHandler(captureHandler);
63+
QMetaObject::invokeMethod(_pModbusPoll, "onAdapterDiagnostic",
64+
Q_ARG(QString, QStringLiteral("warning")),
65+
Q_ARG(QString, QStringLiteral("register read failed")));
66+
qInstallMessageHandler(previous);
67+
68+
QCOMPARE(g_capturedType, QtWarningMsg);
69+
QVERIFY(g_capturedMessage.contains(QStringLiteral("register read failed")));
70+
}
71+
72+
void TestModbusPoll::diagnosticUnknownLevel()
73+
{
74+
QtMessageHandler previous = qInstallMessageHandler(captureHandler);
75+
QMetaObject::invokeMethod(_pModbusPoll, "onAdapterDiagnostic",
76+
Q_ARG(QString, QStringLiteral("critical")),
77+
Q_ARG(QString, QStringLiteral("unexpected error")));
78+
qInstallMessageHandler(previous);
79+
80+
QCOMPARE(g_capturedType, QtWarningMsg);
81+
QVERIFY(g_capturedMessage.contains(QStringLiteral("unknown diagnostic level")));
82+
}
83+
84+
QTEST_GUILESS_MAIN(TestModbusPoll)
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
#ifndef TST_MODBUSPOLL_H
2+
#define TST_MODBUSPOLL_H
3+
4+
#include <QObject>
5+
6+
class ModbusPoll;
7+
class SettingsModel;
8+
9+
class TestModbusPoll : public QObject
10+
{
11+
Q_OBJECT
12+
private slots:
13+
void init();
14+
void cleanup();
15+
16+
void diagnosticDebugLevel();
17+
void diagnosticInfoLevel();
18+
void diagnosticWarningLevel();
19+
void diagnosticUnknownLevel();
20+
21+
private:
22+
SettingsModel* _pSettingsModel{ nullptr };
23+
ModbusPoll* _pModbusPoll{ nullptr };
24+
};
25+
26+
#endif // TST_MODBUSPOLL_H

0 commit comments

Comments
 (0)