diff --git a/debian-qt5/changelog b/debian-qt5/changelog index 5bfa05a..e2d5301 100644 --- a/debian-qt5/changelog +++ b/debian-qt5/changelog @@ -1,3 +1,40 @@ +powersync-energy-plugin-nymea (1.15.2+etm36) trixie; urgency=medium + + * ECS-415 — UN REFUS DOIT SE DISTINGUER D'UNE APPLICATION, par la valeur de retour. + `SgReadyAdapter::applyAction()` renvoyait, quand le verrou minStateHold refusait la + transition, l'action DEMANDÉE telle quelle : un appelant qui relisait `state` y retrouvait + l'état qu'il venait de demander et concluait au succès, alors que rien n'avait été écrit et + que les contacts n'avaient pas bougé. + * CE N'EST PAS UN DÉFAUT DU DÉLESTAGE, C'EST UN DÉFAUT DE CONTRAT. N'importe quelle descente + d'état pouvait échouer en silence, et `available` restait vrai — la charge avait l'air saine + pendant que la PAC tirait 3 kW. Même famille qu'ECS-111 : une issue indiscernable de son + contraire, transposée cette fois au RETOUR d'une commande au lieu de la télémétrie. + * Toute sortie de `applyAction()` décrit désormais l'état RÉELLEMENT TENU — `state` et + `estimatedPowerW`, ou `powerW` et `estimatedPowerW` pour les mécanismes en watts. Six + sorties ramenées à une forme unique côté SG-Ready ; l'idempotence comprise, parce qu'un + appelant qui demande « état 4, 0 W » sur une PAC qui en tire 3000 doit se voir rendre 3000 + et non son propre zéro. Même correction sur la branche « motif vide » du RelayRouter et de + l'EtmVariableLoadAdapter ; leurs branches de défaut étaient déjà justes. + * DÉFAUT LATENT, DEVENU GRAVE À L'USAGE. Aucun appelant ne lisait ce retour — le dispatch + l'ignore — donc rien ne le révélait. Le premier à s'y fier fut le délestage L4 en cours + d'écriture, qui a publié 3 kW rendus par une PAC restée en état 4. Un contrat qu'aucun + appelant n'exerce n'est pas un contrat tenu : c'est un contrat non testé. + * `m_currentState` n'était PAS en cause : il est écrit synchroniquement, sans attendre aucun + acquittement, et le repli sur échec d'écriture passe par settleTransition(). L'hypothèse + d'une écriture conditionnée à un retour tardif était fausse, et c'est la sonde qui l'a dit. + * ECS-411-b ne s'applique pas ici : la relecture des contacts (construction, ClearLoadFault) + répond à une divergence entre état interne et matériel. Il n'y en avait aucune — l'état + interne était juste, c'est le RETOUR qui mentait. + * Trois pièges de l'étape 4 consignés dans DESIGN_DELESTAGE §6bis, y compris les deux + placements écartés : après le dispatch on lit un état périmé (aucune boucle d'événements + dans un même update()), et avant syncAdapters() l'écriture part avec l'adaptateur reconstruit + (ECS-412). L'étape 4 elle-même reste NON LIVRÉE. + * Suite complète : simulation 53/53, charging 48/48, loadmodel 21/21, spotmarket 32/32, + doxygen 0 avertissement. Contre-épreuve : sur l'ancien retour, `refuse.state` vaut 2 — + l'état demandé — au lieu de 4. + + -- Patrick Schurig Sun, 30 Aug 2026 23:30:00 +0200 + powersync-energy-plugin-nymea (1.15.2+etm35) trixie; urgency=medium * Délestage §3 — LA CASCADE TIENT DANS DEUX RESSOURCES. Le surplus dit ce qu'on peut dépenser diff --git a/docs/DESIGN_DELESTAGE.md b/docs/DESIGN_DELESTAGE.md index c5a8a47..84623a0 100644 --- a/docs/DESIGN_DELESTAGE.md +++ b/docs/DESIGN_DELESTAGE.md @@ -227,6 +227,34 @@ résolution, elle, dit la vérité (`-1200` veut dire quelque chose que `0` a pe --- +## 6bis. TROIS PIÈGES RENCONTRÉS À L'ÉTAPE 4 — à garder tels quels + +L'étape 4 a été arrêtée avant livraison : son propre test l'a prise à publier un délestage de +3 kW pendant que les contacts de la PAC restaient fermés. Trois hypothèses successives se sont +révélées fausses. Les deux premières sont des pièges de placement qui se représenteront ; la +troisième était la bonne, et elle a donné **ECS-415**. + +**Piège 1 — après le dispatch, on lit un état périmé.** Dans un même `update()` **aucune boucle +d'événements ne tourne**. Un adaptateur qui vient d'être commandé rapporte encore son état +précédent. Le délestage demandait alors un état que l'adaptateur croyait déjà tenir, se le +voyait accorder comme un non-geste, et se créditait des watts que personne n'avait rendus. +*« Le dernier mot » ne veut pas dire « le dernier appel du cycle » : cela veut dire « sur un +état posé ».* + +**Piège 2 — avant `syncAdapters()`, l'écriture part avec l'adaptateur.** La reconstruction +incrémentale (ECS-412) remplace l'objet, et la commande en cours disparaît avec lui. Les +contacts restaient fermés pendant qu'on publiait un délestage. La fenêtre correcte est +**étroite** : après `syncAdapters()`, avant tout dispatch. + +**Piège 3 (la vraie cause) — un refus qui se lisait comme un succès.** Le verrou refusait bien +la descente, mais `applyAction()` renvoyait l'action **demandée**, donc `state == 2`, donc +« cible atteinte ». Voir **ECS-415** : le retour décrit désormais l'état réellement tenu. + +> **Ce que ces trois pièges ont en commun.** Aucun n'était visible dans le code : chacun se +> déduisait d'un ordre d'exécution ou d'une valeur de retour, et les trois produisaient le même +> symptôme — un délestage annoncé sans effet. Ce sont les **sondes**, pas la relecture, qui les +> ont départagés. Une hypothèse plausible et non mesurée en a coûté deux. + ## 7. Ce que ce lot ne fait PAS - **Il ne crée aucun transport §14a.** `GridOperator` sera une source qu'aucun émetteur ne diff --git a/energyplugin/etm/adapters/etmvariableloadadapter.cpp b/energyplugin/etm/adapters/etmvariableloadadapter.cpp index 7ce086d..97db1ec 100644 --- a/energyplugin/etm/adapters/etmvariableloadadapter.cpp +++ b/energyplugin/etm/adapters/etmvariableloadadapter.cpp @@ -99,7 +99,10 @@ LoadAction EtmVariableLoadAdapter::applyAction(const LoadAction &action, const Q if (action.reason.isEmpty()) { qCWarning(dcNymeaEnergy()) << "[EtmVariableLoadAdapter]" << m_label << "— LoadAction sans reason rejetée."; - return action; + // ECS-415 — un refus se lit comme un refus : le retour décrit la consigne TENUE. + LoadAction refuse = action; + refuse.powerW = refuse.estimatedPowerW = m_currentSetpointW; + return refuse; } // ECS-414 — en défaut, plus aucune consigne écrite, y compris forcée. diff --git a/energyplugin/etm/adapters/relayrouter.cpp b/energyplugin/etm/adapters/relayrouter.cpp index 3994a5a..7bcaf77 100644 --- a/energyplugin/etm/adapters/relayrouter.cpp +++ b/energyplugin/etm/adapters/relayrouter.cpp @@ -241,7 +241,11 @@ LoadAction RelayRouter::applyAction(const LoadAction &action, const QDateTime &n if (action.reason.isEmpty()) { qCWarning(dcNymeaEnergy()) << "[RelayRouter]" << m_label << "— LoadAction sans reason rejetée."; - return action; + // ECS-415 — un refus se lit comme un refus : le retour décrit le palier TENU, jamais + // celui qu'on demandait. Rendre l'enveloppe telle quelle ferait conclure au succès. + LoadAction refuse = action; + refuse.powerW = refuse.estimatedPowerW = currentSetpointW(); + return refuse; } // ECS-410 — en défaut, on CESSE TOUTE COMMANDE, y compris les replis forcés : le matériel diff --git a/energyplugin/etm/adapters/sgreadyadapter.cpp b/energyplugin/etm/adapters/sgreadyadapter.cpp index d4c8e4a..3a8d678 100644 --- a/energyplugin/etm/adapters/sgreadyadapter.cpp +++ b/energyplugin/etm/adapters/sgreadyadapter.cpp @@ -205,6 +205,21 @@ LoadContext SgReadyAdapter::toLoadContext(const QDateTime &now) const return ctx; } +/*! + * \brief Retour décrivant l'état RÉELLEMENT tenu — forme unique de tout refus (ECS-415). + * + * Un appelant doit pouvoir distinguer « appliqué » de « refusé » en lisant la valeur de retour, + * et par le MÊME test dans les deux cas. Renvoyer l'action demandée telle quelle rendait un + * refus indiscernable d'un succès : le lecteur y retrouvait l'état qu'il venait de demander. + */ +LoadAction SgReadyAdapter::inchange(const LoadAction &action) const +{ + LoadAction refuse = action; + refuse.state = m_currentState; + refuse.estimatedPowerW = m_estimatedPowerW.value(m_currentState, 0.0); + return refuse; +} + LoadAction SgReadyAdapter::applyAction(const LoadAction &action, const QDateTime &now) { if (action.kind != LoadAction::State) @@ -213,22 +228,16 @@ LoadAction SgReadyAdapter::applyAction(const LoadAction &action, const QDateTime if (action.reason.isEmpty()) { qCWarning(dcNymeaEnergy()) << "[SgReadyAdapter]" << m_label << "— LoadAction sans reason rejetée."; - return action; + return inchange(action); } - if (!m_usable) { - LoadAction refused = action; - refused.state = m_currentState; - return refused; // encodage sans état 2 : aucune commande, cf. constructeur - } + if (!m_usable) + return inchange(action); // encodage sans état 2 : aucune commande, cf. constructeur // ECS-414 — en défaut, plus aucune commande, y compris forcée : trois tentatives ont // déjà échoué, en réémettre masquerait l'état sans rien réparer. - if (m_faulted) { - LoadAction refused = action; - refused.state = m_currentState; - return refused; - } + if (m_faulted) + return inchange(action); // ECS-412 — l'armement paresseux se DÉSARME ici, dans le chemin NON-const qui reçoit le // temps de cycle : au premier \c now vu, on estampille, et le verrou expire alors @@ -242,7 +251,7 @@ LoadAction SgReadyAdapter::applyAction(const LoadAction &action, const QDateTime if (!m_stateRelays.contains(newState)) { qCWarning(dcNymeaEnergy()) << "[SgReadyAdapter]" << m_label << "— état non déclaré:" << action.state << "→ ignoré."; - return action; + return inchange(action); } // Règle 7-c — même défaut que celui corrigé au RelayRouter : ce retour était MUET, si bien @@ -253,7 +262,11 @@ LoadAction SgReadyAdapter::applyAction(const LoadAction &action, const QDateTime << "→ état" << newState << "INCHANGÉ (aucune commutation)" << (action.force ? "(force)" : "") << "|" << renderFr(action.reason, m_label); - return action; // Idempotent + // ECS-415 — même pour l'idempotence : l'état est bien celui-là, mais la puissance + // estimée doit venir de l'adaptateur, pas de ce que l'appelant avait mis dans son + // enveloppe. Un appelant qui demande « état 4, 0 W » et se voit rendre « 0 W » sur une + // PAC qui en tire 3000 lit son propre chiffre, pas une réponse. + return inchange(action); // Idempotent } // Verrou minStateHold évalué au temps de cycle (même fenêtre que le scheduler) — @@ -267,7 +280,7 @@ LoadAction SgReadyAdapter::applyAction(const LoadAction &action, const QDateTime qCInfo(dcNymeaEnergy()) << "[SgReadyAdapter]" << m_label << "— verrou minStateHold actif, état" << newState << "REFUSÉ (état" << m_currentState << "maintenu)."; - return action; + return inchange(action); } qCInfo(dcNymeaEnergy()) << "[SgReadyAdapter]" << m_label diff --git a/energyplugin/etm/adapters/sgreadyadapter.h b/energyplugin/etm/adapters/sgreadyadapter.h index e6a1694..fd9e9cf 100644 --- a/energyplugin/etm/adapters/sgreadyadapter.h +++ b/energyplugin/etm/adapters/sgreadyadapter.h @@ -161,6 +161,13 @@ public: int currentState() const { return m_currentState; } private: + /*! + * \brief Retour décrivant l'état RÉELLEMENT tenu — forme unique de tout refus (ECS-415). + * \param action Action refusée, dont l'enveloppe (motif, kind) est conservée. + * \return La même action, mais avec \c state et \c estimatedPowerW pris sur l'adaptateur. + */ + LoadAction inchange(const LoadAction &action) const; + /*! * \brief Fenêtre d'états autorisée à \p now par le verrou minStateHold (symétrique). * \param now Temps de cycle (\c ctx.timestamp) — jamais l'horloge. diff --git a/specs/spec_ecs.md b/specs/spec_ecs.md index e5ece16..fa6cd7d 100644 --- a/specs/spec_ecs.md +++ b/specs/spec_ecs.md @@ -996,3 +996,38 @@ protocole dans le même lot.** état-à-état avec pour seule transformation un booléen `inverted` — il ne peut exprimer ni le N→1, ni l'arithmétique, ni l'ordre de commutation, ni les temporisations. + + +### ECS-415 — un refus doit se distinguer d'une application, par la valeur de retour + +**Relevé le 2026-08-30, en tirant le fil d'un délestage annoncé qui n'avait pas lieu.** + +`SgReadyAdapter::applyAction()` renvoyait, quand le verrou `minStateHold` refusait la +transition, **l'action demandée telle quelle**. Un appelant qui relisait `state` y retrouvait +l'état qu'il venait de demander et concluait au succès — alors que rien n'avait été écrit et +que les contacts n'avaient pas bougé. + +**Ce n'est pas un défaut du délestage, c'est un défaut de contrat.** N'importe quelle descente +d'état pouvait échouer en silence, et `available` restait vrai : la charge avait l'air saine +pendant que la PAC tirait 3 kW. C'est la famille d'**ECS-111** — une issue indiscernable de son +contraire — transposée cette fois au **retour d'une commande** au lieu de la télémétrie. + +**La règle.** Toute sortie de `applyAction()` décrit l'état **réellement tenu par l'adaptateur**, +jamais celui qu'on lui demandait — `state` *et* `estimatedPowerW`, ou `powerW` *et* +`estimatedPowerW` pour les mécanismes en watts. Refus et application se lisent alors par le +**même test**. + +L'idempotence n'y échappe pas : un appelant qui demande « état 4, 0 W » sur une PAC qui en tire +3000 doit se voir rendre 3000, pas son propre zéro. Sinon il lit son enveloppe, pas une réponse. + +**Ce qui était déjà juste, et ce qui ne l'était pas.** Les branches `!m_usable` et `m_faulted` +corrigeaient bien `state` — mais aucune ne corrigeait `estimatedPowerW`. Les branches « motif +vide », « état non déclaré » et **« verrou actif »** ne corrigeaient rien. Le même travers +existait sur la branche « motif vide » du `RelayRouter` et de l'`EtmVariableLoadAdapter` ; +leurs branches de défaut, elles, étaient correctes. + +> **Ce que le défaut a rendu invisible.** Aucun appelant ne lisait ce retour : le dispatch +> l'ignore. Le défaut était donc **latent** — sans conséquence tant que personne ne faisait +> confiance à la valeur rendue, et immédiatement grave dès qu'un appelant s'y fiait. Le premier +> à le faire fut le délestage L4, qui a publié 3 kW rendus par une PAC restée en état 4. Un +> contrat qu'aucun appelant n'exerce n'est pas un contrat tenu : c'est un contrat non testé. diff --git a/tests/auto/simulation/simulation.cpp b/tests/auto/simulation/simulation.cpp index dd7a4f2..65c1927 100644 --- a/tests/auto/simulation/simulation.cpp +++ b/tests/auto/simulation/simulation.cpp @@ -6313,6 +6313,105 @@ void Simulation::testDrawCapBoundsTheCascade() #endif } +/*! + * \brief [ECS-415] Un refus doit se DISTINGUER d'une application, par la valeur de retour. + * + * \par Le défaut + * `SgReadyAdapter::applyAction()` renvoyait, en cas de refus par le verrou `minStateHold`, + * **l'action demandée telle quelle**. Un appelant qui relisait `state` y retrouvait l'état + * qu'il venait de demander, et concluait au succès — alors que rien n'avait été écrit et que + * les contacts n'avaient pas bougé. Deux autres sorties avaient le même travers (motif vide, + * état non déclaré), et les deux qui corrigeaient bien `state` (\c !m_usable, \c m_faulted) + * laissaient `estimatedPowerW` à la valeur de l'appelant. + * + * \par Pourquoi c'est plus grave qu'un délestage manqué + * N'IMPORTE QUELLE descente d'état pouvait échouer en silence, et `available` restait vrai : + * la charge avait l'air saine pendant que la PAC tirait 3 kW. C'est la famille d'ECS-111 — + * une issue indiscernable de son contraire — appliquée cette fois au retour d'une commande. + * + * \par Ce qui est épinglé + * Le refus et l'application se lisent DÉSORMAIS par le même test, et le retour décrit toujours + * l'état réellement tenu, puissance estimée comprise. + */ +void Simulation::testARefusedStateIsDistinguishableFromAnApplied() +{ +#ifndef ETM_ARBITRATOR + QSKIP("testARefusedStateIsDistinguishableFromAnApplied nécessite ETM_ARBITRATOR."); +#else + const QString cfgPath = QDir::tempPath() + "/etm-loadcfg-415.json"; + QFile::remove(cfgPath); + qputenv("NYMEA_ENERGY_LOAD_CONFIG", cfgPath.toUtf8()); + + cleanupTestCase(); + m_energyLogDbFilePath = ":/databases/2022-06-22-energylogs.sqlite"; + initTestCase(); + + ThingManager *tm = NymeaCore::instance()->thingManager(); + EnergyArbitrator *arb = dynamic_cast(m_experiencePlugin->smartChargingManager()); + QVERIFY(arb); + + QUuid k1 = addPowerSwitch(0, 26696); + QUuid k2 = addPowerSwitch(0, 26697); + Thing *tK1 = tm->findConfiguredThing(k1); + Thing *tK2 = tm->findConfiguredThing(k2); + QVERIFY(tK1 && tK2); + + const QDateTime t0 = utcDateTime(QDate(2026, 8, 30), QTime(13, 0, 0)); + const QHash> encodage({ {1, {k1.toString()}}, {2, {}}, + {3, {k2.toString()}}, + {4, {k1.toString(), k2.toString()}} }); + const QHash estim({ {1, 0.0}, {2, 0.0}, {3, 1500.0}, {4, 3000.0} }); + + // Contacts fermés : l'adaptateur neuf LIT l'état 4 (ECS-411-b). Le verrou minStateHold est + // armé à froid (ECS-412), donc toute descente est refusée ce cycle. + tK1->setStateValue("power", true); + tK2->setStateValue("power", true); + SgReadyAdapter *pac = new SgReadyAdapter(tm, "pac-415", "PAC refus lisible", + encodage, estim, 300, 1, arb); + QCOMPARE(pac->currentState(), 4); + + LoadAction descente; + descente.loadId = "pac-415"; + descente.kind = LoadAction::State; + descente.state = 2; + descente.estimatedPowerW = 0; // ce que l'APPELANT croit — c'est le piège + descente.reason = { DecisionCode::SgNormal, {{"budgetW", 0}} }; + + // --- 1. REFUSÉ par le verrou : le retour doit décrire l'état TENU, pas l'état demandé --- + const LoadAction refuse = pac->applyAction(descente, t0); + QCOMPARE(refuse.state, 4); + QCOMPARE(qRound(refuse.estimatedPowerW), 3000); + QCOMPARE(tK1->stateValue("power").toBool(), true); + QCOMPARE(tK2->stateValue("power").toBool(), true); + QCOMPARE(pac->currentState(), 4); + + // --- 2. FORCÉ : le même appel, verrou franchi, doit se lire autrement ------------------- + // C'est la moitié qui donne son sens à la première : si les deux retours étaient égaux, + // épingler la valeur du refus ne prouverait rien. + LoadAction forcee = descente; + forcee.force = true; + const LoadAction applique = pac->applyAction(forcee, t0); + QCOMPARE(applique.state, 2); + QCOMPARE(qRound(applique.estimatedPowerW), 0); + QCOMPARE(tK1->stateValue("power").toBool(), false); + QCOMPARE(tK2->stateValue("power").toBool(), false); + QCOMPARE(pac->currentState(), 2); + + // --- 3. IDEMPOTENCE : l'état est bien celui demandé, mais la puissance vient de + // l'adaptateur. Un appelant qui remonte à 4 avec 0 W dans son enveloppe ne doit pas + // se voir rendre son propre zéro sur une PAC qui en tirerait 3000. + LoadAction remonte; + remonte.loadId = "pac-415"; + remonte.kind = LoadAction::State; + remonte.state = 2; // état DÉJÀ tenu + remonte.estimatedPowerW = 12345; // valeur absurde de l'appelant + remonte.reason = { DecisionCode::SgNormal, {{"budgetW", 0}} }; + const LoadAction idem = pac->applyAction(remonte, t0.addSecs(1)); + QCOMPARE(idem.state, 2); + QCOMPARE(qRound(idem.estimatedPowerW), 0); +#endif +} + void Simulation::testCountsSumToTarget() { #ifndef ETM_ARBITRATOR diff --git a/tests/auto/simulation/simulation.h b/tests/auto/simulation/simulation.h index 9f1b6e1..330e3c9 100644 --- a/tests/auto/simulation/simulation.h +++ b/tests/auto/simulation/simulation.h @@ -193,6 +193,7 @@ private slots: void testLevelAbsenceAndZeroDifferInMeaning(); void testPhaseAllowanceReadsTheStateItTests(); void testDrawCapBoundsTheCascade(); + void testARefusedStateIsDistinguishableFromAnApplied(); // ── §12 / LM-1209 ────────────────────────────────────────────────────── // Une borne détectée après la mise en service entre EN QUEUE : elle ne passe devant