From 58ec2d1110febe12bd44b8de1a4a36c7e4400fa8 Mon Sep 17 00:00:00 2001 From: Patrick Schurig Date: Thu, 27 Aug 2026 10:10:00 +0200 Subject: [PATCH] [upstream-fix] SetChargingInfo promettait un partiel, le code remplace MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit La description publiée par l'introspection JSON-RPC annonçait « Only given properties will be set, others will be untouched. » C'est faux : unpack() construit un ChargingInfo NEUF depuis la charge utile, et setChargingInfo() remplace le stocké en entier. Un champ writable omis retombe donc sur son DÉFAUT — targetPercentage à 80, repeatDays vide, endDateTime absent, assignedCarId nul — et l'appel réussit, EnergyErrorNoError. Rien ne signale la perte. Constaté sur le banc .75 le 2026-08-26 en restaurant la configuration : un appel ne portant que chargingMode a porté targetPercentage de 0 à 80. C'est la DESCRIPTION qui est corrigée, pas le comportement, et le choix mérite d'être dit : ce .so est un drop-in de l'amont. Une sémantique de fusion casserait le seul geste qui permet d'EFFACER un champ — retirer une échéance, vider repeatDays — et changerait un contrat que les clients existants suivent déjà. La description énumère désormais chaque défaut et prescrit le geste correct : lire avec GetChargingInfos, modifier, renvoyer l'objet complet. Le test épingle le comportement annoncé, en trois temps : un objet complet, un appel partiel qui aboutit et perd les champs omis, puis l'aller-retour qui les préserve — y compris quand la charge utile relue porte chargingState, propriété en lecture seule que la méthode doit ignorer sans faillir. Il porte sur le COMPORTEMENT décrit, jamais sur la non-vacuité d'une réponse. Vérifié au passage : aucun autre Set* du namespace ne porte cette promesse ni ce comportement. SetPhasePowerLimit, SetAcquisitionTolerance, SetBatteryLevelConsideration et SetLockOnUnplug sont scalaires ; SetSpotMarketConfiguration ne remet jamais un champ omis à son défaut ; SetLoadConfig annonce déjà un remplacement d'ensemble. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_017TVywtayEeoRJ2yWUW4iZj --- energyplugin/nymeaenergyjsonhandler.cpp | 8 ++- tests/auto/charging/testcharging.cpp | 86 +++++++++++++++++++++++++ tests/auto/charging/testcharging.h | 4 ++ 3 files changed, 97 insertions(+), 1 deletion(-) diff --git a/energyplugin/nymeaenergyjsonhandler.cpp b/energyplugin/nymeaenergyjsonhandler.cpp index badd471..eea0078 100644 --- a/energyplugin/nymeaenergyjsonhandler.cpp +++ b/energyplugin/nymeaenergyjsonhandler.cpp @@ -111,7 +111,13 @@ NymeaEnergyJsonHandler::NymeaEnergyJsonHandler(SpotMarketManager *spotMarketMana registerMethod("GetChargingInfos", description, params, returns, Types::PermissionScopeControlThings); params.clear(); returns.clear(); - description = "Set the charging info for an EV charger. Only given properties will be set, others will be untouched."; + description = "Set the charging info for an EV charger. The given object REPLACES the stored " + "one as a whole: a writable property omitted from the call is reset to its " + "default (assignedCarId: none, chargingMode: ChargingModeNormal, endDateTime: " + "none, repeatDays: none, targetPercentage: 80, spotMarketChargingEnabled: false, " + "dailySpotMarketPercentage: 0); omitting a property does NOT keep its current " + "value. Read the object with GetChargingInfos, change what you need, and send it " + "back complete. Read-only properties (chargingState) are ignored."; params.insert("chargingInfo", objectRef()); returns.insert("energyError", enumRef()); registerMethod("SetChargingInfo", description, params, returns, Types::PermissionScopeControlThings); diff --git a/tests/auto/charging/testcharging.cpp b/tests/auto/charging/testcharging.cpp index 1845ac0..b87e52f 100644 --- a/tests/auto/charging/testcharging.cpp +++ b/tests/auto/charging/testcharging.cpp @@ -413,6 +413,92 @@ void TestCharging::addChargingInfo() } } +void TestCharging::setChargingInfoReplacesWholesale() +{ + // [ETM] Le banc a montré le piège le 2026-08-26 : un appel ne portant que « chargingMode » + // a remonté targetPercentage de 0 à 80 — le DÉFAUT du constructeur — sous + // EnergyErrorNoError, alors que la description de la méthode promettait + // « Only given properties will be set, others will be untouched ». + // La description dit désormais la vérité (l'objet REMPLACE le stocké). Ce test épingle le + // comportement qu'elle annonce : sans lui, la promesse redeviendrait fausse en silence. + QVariant response; + + response = injectAndWait("NymeaEnergy.SetPhasePowerLimit", QVariantMap({{"phasePowerLimit", 25000}})); + verifyEnergyError(response); + + removeDevices(); + + QUuid meterThingId = addMeter(); + QVERIFY2(!meterThingId.isNull(), "Did not receive valid ThingId for the meter"); + + QUuid evChargerId = addCharger("ABC", 16); + QVERIFY2(!evChargerId.isNull(), "Did not receive valid ThingId for the charger"); + + QUuid carId = addCar(); + QVERIFY2(!carId.isNull(), "Did not receive valid ThingId for the car"); + + auto storedInfo = [this, evChargerId]() -> QVariantMap { + QVariant reply = injectAndWait("NymeaEnergy.GetChargingInfos", QVariantMap()); + foreach (const QVariant &entry, reply.toMap().value("params").toMap().value("chargingInfos").toList()) { + if (entry.toMap().value("evChargerId").toUuid() == evChargerId) + return entry.toMap(); + } + return QVariantMap(); + }; + + // 1. Un objet complet, dont AUCUNE valeur n'est un défaut. + QVariantMap full; + full.insert("evChargerId", evChargerId); + full.insert("assignedCarId", carId); + full.insert("chargingMode", "ChargingModeEco"); + full.insert("repeatDays", QVariantList() << 1 << 3); + full.insert("targetPercentage", 55); + response = injectAndWait("NymeaEnergy.SetChargingInfo", QVariantMap({{"chargingInfo", full}})); + verifyEnergyError(response); + + QVariantMap stored = storedInfo(); + QVERIFY2(!stored.isEmpty(), "Charging info was not stored at all"); + QCOMPARE(stored.value("targetPercentage").toUInt(), 55u); + QCOMPARE(stored.value("repeatDays").toList().count(), 2); + QCOMPARE(stored.value("assignedCarId").toUuid(), carId); + + // 2. Un appel PARTIEL — le geste que la description autorisait par écrit. Il aboutit + // (EnergyErrorNoError) et les champs omis retombent sur leur DÉFAUT : ils ne sont pas + // préservés. C'est exactement ce qu'a vu le banc. + QVariantMap partial; + partial.insert("evChargerId", evChargerId); + partial.insert("chargingMode", "ChargingModeNormal"); + response = injectAndWait("NymeaEnergy.SetChargingInfo", QVariantMap({{"chargingInfo", partial}})); + verifyEnergyError(response); + + stored = storedInfo(); + QCOMPARE(stored.value("chargingMode").toString(), QString("ChargingModeNormal")); + QCOMPARE(stored.value("targetPercentage").toUInt(), 80u); // défaut, PAS les 55 posés en 1. + QVERIFY2(stored.value("repeatDays").toList().isEmpty(), "repeatDays survived a partial call"); + QVERIFY2(stored.value("assignedCarId").toUuid().isNull(), "assignedCarId survived a partial call"); + + // 3. Le remède que la description prescrit : lire, modifier, renvoyer complet. Les champs + // non touchés survivent — y compris quand la charge utile relue porte « chargingState », + // propriété en lecture seule, que la méthode doit ignorer sans faillir. + QVariantMap reposted = full; + response = injectAndWait("NymeaEnergy.SetChargingInfo", QVariantMap({{"chargingInfo", reposted}})); + verifyEnergyError(response); + + QVariantMap roundTrip = storedInfo(); + QVERIFY2(roundTrip.contains("chargingState"), "GetChargingInfos no longer publishes chargingState"); + // Un mode SANS échéance : avec une échéance, le moteur retomberait de lui-même sur + // ChargingModeEco faute d'endDateTime valide, et le test mesurerait ce repli au lieu de + // l'aller-retour. + roundTrip.insert("chargingMode", "ChargingModeEcoWithMinCurrent"); + response = injectAndWait("NymeaEnergy.SetChargingInfo", QVariantMap({{"chargingInfo", roundTrip}})); + verifyEnergyError(response); + + stored = storedInfo(); + QCOMPARE(stored.value("chargingMode").toString(), QString("ChargingModeEcoWithMinCurrent")); + QCOMPARE(stored.value("targetPercentage").toUInt(), 55u); + QCOMPARE(stored.value("assignedCarId").toUuid(), carId); +} + void TestCharging::testEcoMode_data() { // Houshold info diff --git a/tests/auto/charging/testcharging.h b/tests/auto/charging/testcharging.h index 22799da..c8ed30a 100644 --- a/tests/auto/charging/testcharging.h +++ b/tests/auto/charging/testcharging.h @@ -55,6 +55,10 @@ private slots: void addChargingInfo_data(); void addChargingInfo(); + // [ETM] La description publiee de SetChargingInfo est un contrat : le test porte sur le + // COMPORTEMENT qu'elle annonce, pas sur la non-vacuite d'une reponse. + void setChargingInfoReplacesWholesale(); + // Test individual logic void testEcoMode_data(); void testEcoMode();