ETM-Schurig/etm-powersync-plugins-modbus
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

sunspec: fix a dangling capture in the device suite and widen it

Browse Source 
The suite captured a stack local of initTestCase() by reference in two
lambdas that outlive it. Once the function returned they wrote into a
stack slot reused by later frames, corrupting the running test: a
QSignalSpy reported a phantom emission, reading its recorded arguments
segfaulted, and the count changed when unrelated locals were added. Both
observed failures came from that, not from the plugin. The two flags are
members now.

Covers the two remaining claims the offline suites cannot reach: a meter
slave ID of 0 opens no second connection against a real device, and
losing the meter connection leaves the parent connected and the inverter
data flowing, marking only the meter child disconnected.

That last test drives one explicit block read first. This bench replaces
the plugin timer with a null object, on purpose, so nothing would
otherwise ever mark a child connected and the cut would prove nothing.

Run against the bench simulator: 8 tests, 8 green. Each of the two
corrections was confirmed able to fail by reintroducing its pre-F3 form.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Patrick Schurig 2026-08-02 09:51:10 +02:00
parent 1e02400acd
commit 955016cc47
2 changed files with 161 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
specs/fronius-sunspec.md
View File

@ -364,7 +364,19 @@ l'aiguillage du plugin, 10 sur le multi unit ID.
Une quatrième suite, `tests/integration`, monte le plugin face à un vrai
appareil. Elle est construite mais **jamais lancée par `make check`** : elle
exige `SUNSPEC_TEST_HOST`. **Elle n'a pas encore été exécutée sur matériel.**
exige `SUNSPEC_TEST_HOST`.
**Exécutée le 2026-08-02 contre le simulateur `froniusgen24-sunspec` du banc :
8 tests, 8 verts.** Unit 1 = modèles 1/113/120-123/160, unit 200 = 1/213, sans
batterie. Les deux corrections qu'aucune suite hors ligne ne pouvait couvrir y
sont vérifiées, et chacune a été confirmée capable d'échouer en réintroduisant
la forme pré-F3 : le balayage des orphelins non corrigé supprime **les deux**
enfants en deux cycles de découverte, et la propagation de `connected` non
scindée fait passer le parent déconnecté sur la seule chute du compteur.
Reste non exécuté sur **matériel réel** : le GEN24 client est une installation
distante, écartée pour ce lot (une découverte ratée y demanderait une coupure
AC+DC sur site).
Build : `qmake6 NYMEA_SUNSPEC_PATH=<sysroot> sunspec/tests/tests.pro && make check`.
Sans `NYMEA_SUNSPEC_PATH`, `sunspec.pri` résout `libnymea-sunspec` par

168
sunspec/tests/integration/tst_sunspecdevice.cpp
View File

@ -241,6 +241,8 @@ private slots:
void test_F3_meter_connects_only_after_the_primary_discovery();
void test_F3_inverter_and_meter_children_share_one_parent();
void test_F3_no_child_disappears_across_repeated_discoveries();
void test_F3_meter_slave_id_zero_opens_no_second_connection();
void test_F3_losing_the_meter_leaves_the_inverter_alone();
private:
QTemporaryDir m_pluginSearchDir;
@ -255,15 +257,47 @@ private:
TestableSunSpecPlugin *m_plugin = nullptr;
bool m_configured = false;
/* Ces deux temoins sont ecrits par des lambdas connectees dans
* initTestCase() et relues par un test ulterieur : ce sont donc des membres,
* jamais des locales capturees par reference. Une locale de initTestCase()
* capturee par reference serait relachee au retour de la fonction, et les
* lambdas ecriraient ensuite dans une case de pile reutilisee par les frames
* suivantes - corruption silencieuse du test lui-meme. */
bool m_primaryDiscovered = false;
/* Vrai des que la connexion compteur signale connected, pour verifier que
* cela n'arrive jamais avant la fin de la decouverte primaire. */
bool m_meterConnectedBeforePrimaryDiscovery = false;
bool skipUnlessConfigured();
QList<Thing *> childrenOfKind(const ThingClassId &thingClassId) const;
void storeConnection(const ThingId &thingId, const QString &name,
const PluginId &pluginId, int meterSlaveId);
SunSpecModel *modelOn(SunSpecConnection *connection, quint16 modelId) const;
};
static const ThingId DeviceThingId = ThingId("5d3f8a26-41bc-4e79-a05f-9b7c2e6d1483");
static const ThingId DeviceNoMeterThingId = ThingId("6a4e1b07-52d8-4c93-8e7b-1f39c04a5d62");
void SunSpecDeviceTest::storeConnection(const ThingId &thingId, const QString &name,
const PluginId &pluginId, int meterSlaveId)
{
NymeaSettings settings(NymeaSettings::SettingsRoleThings);
settings.beginGroup("ThingConfig");
settings.beginGroup(thingId.toString());
settings.setValue("thingName", name);
settings.setValue("pluginid", pluginId.toString());
settings.setValue("thingClassId", froniusConnectionThingClassId.toString());
settings.beginGroup("Params");
settings.setValue(froniusConnectionThingAddressParamTypeId.toString(), m_address.toString());
settings.setValue(froniusConnectionThingPortParamTypeId.toString(), m_port);
settings.setValue(froniusConnectionThingSlaveIdParamTypeId.toString(), m_slaveId);
settings.setValue(froniusConnectionThingMeterSlaveIdParamTypeId.toString(), meterSlaveId);
settings.endGroup();
settings.endGroup();
settings.endGroup();
settings.sync();
}
bool SunSpecDeviceTest::skipUnlessConfigured()
{
@ -322,21 +356,11 @@ void SunSpecDeviceTest::initTestCase()
const PluginMetadata metadata(document.object());
QVERIFY2(metadata.isValid(), "metadonnee du plugin invalide");
NymeaSettings settings(NymeaSettings::SettingsRoleThings);
settings.beginGroup("ThingConfig");
settings.beginGroup(DeviceThingId.toString());
settings.setValue("thingName", "Fronius");
settings.setValue("pluginid", metadata.pluginId().toString());
settings.setValue("thingClassId", froniusConnectionThingClassId.toString());
settings.beginGroup("Params");
settings.setValue(froniusConnectionThingAddressParamTypeId.toString(), m_address.toString());
settings.setValue(froniusConnectionThingPortParamTypeId.toString(), m_port);
settings.setValue(froniusConnectionThingSlaveIdParamTypeId.toString(), m_slaveId);
settings.setValue(froniusConnectionThingMeterSlaveIdParamTypeId.toString(), m_meterSlaveId);
settings.endGroup();
settings.endGroup();
settings.endGroup();
settings.sync();
storeConnection(DeviceThingId, "Fronius", metadata.pluginId(), m_meterSlaveId);
/* Le cas degenere documente au contrat : 0 = pas de compteur derriere cet
* onduleur. Meme appareil, meme unit ID onduleur, mais aucune seconde
* connexion ne doit etre ouverte. */
storeConnection(DeviceNoMeterThingId, "Fronius sans compteur", metadata.pluginId(), 0);
m_hardwareManager = new DeviceHardwareManager(m_address, this);
m_logEngine = new NullLogEngine(this);
@ -357,13 +381,12 @@ void SunSpecDeviceTest::initTestCase()
/* Arme avant toute attente, pour que l'ordre des deux evenements soit
* observe et pas reconstitue apres coup. */
bool primaryDiscovered = false;
connect(primary, &SunSpecConnection::discoveryFinished, this, [&primaryDiscovered](bool success) {
connect(primary, &SunSpecConnection::discoveryFinished, this, [this](bool success) {
if (success)
primaryDiscovered = true;
m_primaryDiscovered = true;
});
connect(meter, &SunSpecConnection::connectedChanged, this, [this, &primaryDiscovered](bool connected) {
if (connected && !primaryDiscovered)
connect(meter, &SunSpecConnection::connectedChanged, this, [this](bool connected) {
if (connected && !m_primaryDiscovered)
m_meterConnectedBeforePrimaryDiscovery = true;
});
@ -452,5 +475,110 @@ void SunSpecDeviceTest::test_F3_no_child_disappears_across_repeated_discoveries(
QCOMPARE(m_thingManager->configuredThings().filterByParentId(DeviceThingId).count(), childrenBefore);
}
SunSpecModel *SunSpecDeviceTest::modelOn(SunSpecConnection *connection, quint16 modelId) const
{
foreach (SunSpecModel *model, connection->models()) {
if (model->modelId() == modelId)
return model;
}
return nullptr;
}
/* Le cas degenere : meterSlaveId a 0 veut dire "pas de compteur derriere cet
* onduleur". Meme appareil, meme unit ID onduleur, mais une seule connexion.
*
* On n'affirme rien sur les enfants de ce second thing : le premier a deja cree
* un enfant onduleur portant le numero de serie du simulateur, et la
* deduplication par numero de serie de autocreateSunSpecModelThing() est
* globale, pas par parent. C'est la connexion qui est mesuree ici. */
void SunSpecDeviceTest::test_F3_meter_slave_id_zero_opens_no_second_connection()
{
if (skipUnlessConfigured())
QSKIP("SUNSPEC_TEST_HOST is not set");
Thing *thing = nullptr;
QTRY_VERIFY_WITH_TIMEOUT((thing = m_thingManager->findConfiguredThing(DeviceNoMeterThingId)) != nullptr, 5000);
QTRY_COMPARE_WITH_TIMEOUT(thing->setupStatus(), Thing::ThingSetupStatusComplete, 30000);
QVERIFY2(m_plugin->testPrimaryConnection(DeviceNoMeterThingId),
"la connexion onduleur n'a pas ete creee");
QVERIFY2(!m_plugin->testMeterConnection(DeviceNoMeterThingId),
"meterSlaveId a 0 ne doit ouvrir aucune connexion compteur");
}
/* Le second defaut invisible hors ligne : l'etat connected du parent suit la
* connexion primaire seule. Un Smart Meter qui tombe ne doit ni marquer
* l'onduleur injoignable, ni effacer les donnees qu'il continue de livrer.
*
* Place en dernier, parce qu'il coupe une connexion pour de bon. */
void SunSpecDeviceTest::test_F3_losing_the_meter_leaves_the_inverter_alone()
{
if (skipUnlessConfigured())
QSKIP("SUNSPEC_TEST_HOST is not set");
Thing *parent = m_thingManager->findConfiguredThing(DeviceThingId);
QVERIFY(parent);
QVERIFY2(parent->stateValue("connected").toBool(), "le parent n'etait pas connecte au depart");
QList<Thing *> inverters = childrenOfKind(sunspecThreePhaseInverterThingClassId);
QList<Thing *> meters = childrenOfKind(sunspecThreePhaseMeterThingClassId);
QVERIFY2(!inverters.isEmpty(), "pas d'enfant onduleur a surveiller");
QVERIFY2(!meters.isEmpty(), "pas d'enfant compteur a couper");
Thing *inverterThing = inverters.first();
Thing *meterThing = meters.first();
SunSpecConnection *primary = m_plugin->testPrimaryConnection(DeviceThingId);
SunSpecConnection *meter = m_plugin->testMeterConnection(DeviceThingId);
QVERIFY(primary && meter);
SunSpecModel *inverterModel = modelOn(primary, 113);
SunSpecModel *meterModel = modelOn(meter, 213);
QVERIFY2(inverterModel, "modele onduleur 113 introuvable sur la connexion primaire");
QVERIFY2(meterModel, "modele compteur 213 introuvable sur la connexion compteur");
/* Un tour d'interrogation explicite. Ce banc remplace le minuteur du plugin
* par un objet creux - on ne veut pas interroger un appareil en boucle - or
* c'est la mise a jour de bloc qui met un enfant a connected. Sans ce tour,
* les enfants n'auraient jamais ete connectes et la coupure ci-dessous ne
* prouverait rien. */
QSignalSpy inverterFirst(inverterModel, &SunSpecModel::blockUpdated);
QSignalSpy meterFirst(meterModel, &SunSpecModel::blockUpdated);
inverterModel->readBlockData();
meterModel->readBlockData();
QVERIFY2(inverterFirst.wait(30000), "pas de donnee onduleur avant la coupure");
if (meterFirst.isEmpty())
QVERIFY2(meterFirst.wait(30000), "pas de donnee compteur avant la coupure");
QVERIFY2(inverterThing->stateValue("connected").toBool(),
"l'enfant onduleur n'etait pas connecte avant la coupure");
QVERIFY2(meterThing->stateValue("connected").toBool(),
"l'enfant compteur n'etait pas connecte avant la coupure");
// Coupure franche de la seule connexion compteur.
meter->disconnectDevice();
QTRY_VERIFY_WITH_TIMEOUT(!meter->connected(), 15000);
// Le parent reste connecte : c'est la primaire qui fait foi, jamais un ET logique.
QVERIFY2(parent->stateValue("connected").toBool(),
"le parent est passe deconnecte alors que seule la connexion compteur est tombee");
QVERIFY(primary->connected());
// L'enfant onduleur n'est pas affecte...
QVERIFY2(inverterThing->stateValue("connected").toBool(),
"l'enfant onduleur a ete marque deconnecte par la chute du compteur");
QVERIFY2(inverterThing->stateValue("currentPower").isValid(),
"les donnees onduleur ont ete effacees");
// ...et ses donnees continuent d'arriver.
QSignalSpy inverterUpdated(inverterModel, &SunSpecModel::blockUpdated);
inverterModel->readBlockData();
QVERIFY2(inverterUpdated.wait(30000),
"plus aucune donnee onduleur apres la chute du compteur");
// Seul l'enfant compteur est marque deconnecte.
QVERIFY2(!meterThing->stateValue("connected").toBool(),
"l'enfant compteur aurait du etre marque deconnecte");
}
QTEST_MAIN(SunSpecDeviceTest)
#include "tst_sunspecdevice.moc"