fix(sunspec): ne pas confondre « pas encore lu » et « pas de batterie »

Constaté au banc à 14:41:58 : l'avertissement « has a model 160 but no
StCha/StDisCha block » a été émis une fois, au démarrage, sur une machine dont
les deux blocs existent et dont le currentPower a ensuite suivi la batterie
correctement en charge, en décharge et au repos.

Cause. libnymea-sunspec crée les blocs répétés d'un modèle depuis
`initFinished`, émis pendant la PREMIÈRE lecture, juste avant `blockUpdated`.
Sur ce premier signal les blocs existent mais n'ont jamais été remplis : tous
les `IDStr` sont vides, rien ne se classe, et `hasBatteryBlocks()` répond faux.
Cette réponse est exacte et hors sujet — la question n'était pas « pas de
batterie » mais « pas encore lu ».

Ce que le faux positif coûtait. Ce n'est pas du bruit : le README dit à
l'exploitation de chercher cette ligne EN PREMIER pour distinguer une batterie
cassée d'une batterie non mesurable. Une ligne qui sort à chaque démarrage sain
ne distingue plus rien, et le diagnostic que ce lot venait d'installer était
mort-né.

Les deux questions sont maintenant posées séparément : `blocksAreIdentified()`
avant `hasBatteryBlocks()`. Sans identification, on se tait et on attend le tour
suivant — `readBlockData()` passe à chaque rafraîchissement de toute façon.

Le garde en double au moment du binding disparaît aussi : les deux updaters se
gardent eux-mêmes, un seul endroit à tenir à jour plutôt que deux qui peuvent
diverger.

Deux tests figent la distinction, dont un sur le cas où l'avertissement DOIT
sortir — un GEN24 sans batterie, lu, identifié, sans bloc batterie.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y3gzsuwENJfJtvuNDT3xGn
This commit is contained in:
Patrick Schurig 2026-08-26 16:18:20 +02:00
parent 122df17a98
commit 2bb3ff570b
5 changed files with 109 additions and 5 deletions

19
debian/changelog vendored
View File

@ -1,3 +1,22 @@
etm-powersync-plugins-modbus (1.15.2+etm11) trixie; urgency=medium
* sunspec: stop warning about a missing battery channel on a machine that has
simply not been read yet. libnymea-sunspec creates the repeating blocks of a
model from initFinished, which fires during the FIRST block read, just
before blockUpdated. On that first signal the blocks exist but carry no
data: every IDStr is empty, nothing classifies, and hasBatteryBlocks()
answered false about an inverter whose StCha/StDisCha blocks were there all
along. Measured on the bench at 14:41:58: the warning fired once at startup
on a storage whose currentPower then tracked the battery correctly through
charge, discharge and rest.
What that false positive cost is not noise. The README tells operations to
grep that exact line first to tell a broken storage from an unmeasurable
one; a line that appears at every healthy start tells them nothing. The two
questions are now asked separately - blocksAreIdentified() before
hasBatteryBlocks() - and the warning waits for the next refresh instead.
-- Patrick Schurig <patrick@etm-schurig.eu> Wed, 26 Aug 2026 16:10:00 +0200
etm-powersync-plugins-modbus (1.15.2+etm10) trixie; urgency=medium
* sunspec: make the SunSpec storage visible to the energy engine. The

View File

@ -211,6 +211,15 @@ double batteryPower(const QList<ModuleReading> &readings)
return -storageCurrentPower(readings);
}
bool blocksAreIdentified(const QList<ModuleReading> &readings)
{
foreach (const ModuleReading &reading, readings) {
if (!reading.inputIdString.trimmed().isEmpty())
return true;
}
return false;
}
bool hasBatteryBlocks(const QList<ModuleReading> &readings)
{
foreach (const ModuleReading &reading, readings) {

View File

@ -242,6 +242,21 @@ double storageCurrentPower(const QList<ModuleReading> &readings);
* two zeros mean opposite things to a reserve rule. */
bool hasBatteryBlocks(const QList<ModuleReading> &readings);
/* Whether the blocks have been read at all, i.e. whether any of them carries an
* ID string. This is NOT the same question as hasBatteryBlocks(), and confusing
* the two produces a false alarm on every healthy start.
*
* libnymea-sunspec creates the repeating blocks of a model from `initFinished`,
* which fires during the FIRST block read, just before `blockUpdated`. On that
* first signal the blocks therefore exist but their data points have never been
* filled: every inputIdSting() is empty, nothing classifies, and a caller that
* only asked hasBatteryBlocks() would conclude "this inverter has no battery
* channel" about a machine it has not read yet.
*
* Measured: the warning fired once at 14:41:58 on a bench whose StCha/StDisCha
* blocks were present and whose currentPower worked from the next cycle on. */
bool blocksAreIdentified(const QList<ModuleReading> &readings);
}
#endif // FRONIUSRULES_H

View File

@ -2190,11 +2190,11 @@ void IntegrationPluginSunSpec::bindStorageCompanionModels(Thing *thing, SunSpecM
return;
}
/* Publish straight away when the companion models already carry data, so a
* thing bound after a discovery does not wait a whole refresh cycle. */
if (!m_storageMpptModels.value(thing)->repeatingBlocks().isEmpty())
updateStorageCurrentPower(thing);
/* Publish straight away, so a thing bound after a discovery does not wait a
* whole refresh cycle. Both updaters return quietly when their model has
* nothing readable yet - a single guard, inside them, rather than a second
* one here that would have to be kept in step. */
updateStorageCurrentPower(thing);
updateStorageCapacity(thing);
}
@ -2257,6 +2257,15 @@ void IntegrationPluginSunSpec::updateStorageCurrentPower(Thing *thing)
{
const QList<FroniusRules::ModuleReading> readings = storageModuleReadings(thing);
/* Not read yet, not "no battery". On the first blockUpdated the repeating
* blocks have just been created and carry no data, so nothing classifies.
* Warning here would put a false alarm in the journal of every healthy
* machine at every start - on the very line operations is told to trust to
* tell a broken storage from an unmeasurable one. Silence, and wait for the
* next cycle: readBlockData() runs on every refresh anyway. */
if (!FroniusRules::blocksAreIdentified(readings))
return;
if (!FroniusRules::hasBatteryBlocks(readings)) {
/* An inverter with no battery publishes N=2 and no StCha/StDisCha at
* all. Summing an empty set would answer a confident 0 W. */

View File

@ -171,6 +171,8 @@ private slots:
void test_D9_current_power_ignores_the_pv_strings();
void test_D9_battery_blocks_are_detected_by_id_string();
void test_D9_an_inverter_without_battery_has_no_battery_block();
void test_D9_unread_blocks_are_not_mistaken_for_a_missing_battery();
void test_D9_read_blocks_are_identified();
void test_D9_capacity_converts_watthours_to_kilowatthours();
void test_D9_a_nameplate_without_capacity_is_refused();
@ -1059,6 +1061,56 @@ void SunSpecFroniusTest::test_D9_an_inverter_without_battery_has_no_battery_bloc
QVERIFY(!hasBatteryBlocks({}));
}
/* La distinction que ce test protege est celle qui a produit un faux positif au
* banc le 2026-08-26 a 14:41:58 : l'avertissement « pas de bloc StCha/StDisCha »
* a ete emis sur une machine dont les deux blocs existaient et dont le
* currentPower a fonctionne des le cycle suivant.
*
* Cause : libnymea-sunspec cree les blocs repetes depuis `initFinished`, emis
* pendant la PREMIERE lecture, juste avant `blockUpdated`. Sur ce premier
* signal les blocs existent mais n'ont jamais ete remplis - tous les IDStr sont
* vides. hasBatteryBlocks() repond faux, ce qui est exact et hors sujet : la
* question n'est pas « pas de batterie » mais « pas encore lu ».
*
* Ce que coutait le faux positif : la ligne de journal est celle que le README
* dit a l'exploitation de chercher en premier pour distinguer une batterie
* cassee d'une batterie non mesurable. Une ligne qui apparait a CHAQUE demarrage
* sain ne distingue plus rien. */
void SunSpecFroniusTest::test_D9_unread_blocks_are_not_mistaken_for_a_missing_battery()
{
using namespace FroniusRules;
// Blocs crees mais jamais remplis : la puissance est a zero et l'IDStr vide.
const QList<ModuleReading> jamaisLus = {
{"", 0.0}, {"", 0.0}, {"", 0.0}, {"", 0.0}
};
QVERIFY(!blocksAreIdentified(jamaisLus));
// hasBatteryBlocks() repond faux aussi - c'est bien pour ca qu'il ne suffit
// pas a lui seul a decider qu'il n'y a pas de batterie.
QVERIFY(!hasBatteryBlocks(jamaisLus));
QVERIFY(!blocksAreIdentified({}));
QVERIFY(!blocksAreIdentified({{" ", 0.0}}));
}
void SunSpecFroniusTest::test_D9_read_blocks_are_identified()
{
using namespace FroniusRules;
// Un GEN24 sans batterie : lu, identifie, et sans bloc batterie. C'est LE
// cas ou l'avertissement doit bien sortir.
const QList<ModuleReading> sansBatterie = {{"MPPT 1", 1200.0}, {"MPPT 2", 800.0}};
QVERIFY(blocksAreIdentified(sansBatterie));
QVERIFY(!hasBatteryBlocks(sansBatterie));
const QList<ModuleReading> avecBatterie = {
{"MPPT 1", 1200.0}, {"StCha 3", 0.0}, {"StDisCha 4", 0.0}
};
QVERIFY(blocksAreIdentified(avecBatterie));
QVERIFY(hasBatteryBlocks(avecBatterie));
}
void SunSpecFroniusTest::test_D9_capacity_converts_watthours_to_kilowatthours()
{
QCOMPARE(SunSpecStorageLogic::nameplateCapacityKwh(10000.0), 10.0);