[upstream-fix] SetChargingInfo promettait un partiel, le code remplace
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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017TVywtayEeoRJ2yWUW4iZj
This commit is contained in:
parent
21c47c621a
commit
58ec2d1110
@ -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<ChargingInfo>());
|
||||
returns.insert("energyError", enumRef<EnergyManager::EnergyError>());
|
||||
registerMethod("SetChargingInfo", description, params, returns, Types::PermissionScopeControlThings);
|
||||
|
||||
@ -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
|
||||
|
||||
@ -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();
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user