Don't require a value for inactive load control limits - #255
Conversation
|
Analysis against spec (Claude):
▎ "If set to "false", the limit and its timePeriod and value Elements SHALL be ignored. If set to "true", the timePeriod and value Elements SHALL be applied." Value only carries meaning for an active limit — exactly the PR's rule. Erroring out on a missing value for an inactive limit contradicts a "SHALL be ignored".
Caveat on strictness Table 23 does mark value. 1: M at the CS. So Wolf omitting it is technically under-conformant. But mandatory-content-may-lag (point 5) + must-ignore-when-inactive (point 2) make the client's hard error the wrong reaction. PR premise holds. |
|
I'm not sure I agree with Claude's analysis of the spec here with the partial exception of point 5. I'm not categorically against this change, but I would need to discuss within the team on if we can merge the changes to the EG as is, or if we don't think the Wolf is being spec-compliant and we might want to feature-gate this change behind a "quirks-mode" via configuration or compile time flags. The changes to the CS usecases, though, I think are wrong in general.The EG is not allowed to delete value or send a full write without value set. Any partial write will get merged by the SPINE layer and should never show up to the eebus-go layer without value set. If it does, that write either should have been rejected or was incorrectly merged. This would be a bug elsewhere in the stacks and should be fixed there. If the issue is that a CS never set an initial limit then we should enforce that it must or provide a suitable default value so that implementations using eebus-go are always spec-compliant. For the points from Claude's analysis:
tl;dr I will discuss this issue with the team and we will come to a decision on how we want to move forward with this Thank you for bringing this issue to our attention |
@sthelen-enqs you know the spec mich better than me. It seems to me though that it is just not very clear. Not setting a value limit for inactivating a limit seems permissible as the value doesn't matter anyway. This is an in-the-wild experience from a Wolf heatpump, so whoever read the spec on the other side has done it that way.
I don't know if a disable message could be the first one received, if it is it might be incomplete if it disables. May be academic.
Either this should be an error as it is ambiguous OR as valid when merged with existing data. This is however specifically not the case I've brought to discussion here. I guess this comes down to the question if the interpretation should be hard or forgiving. I'm happy with any decision.
Yes, fubar... |
ConsumptionLimit/ProductionLimitbail out withErrDataNotAvailablewhenever the limit data carries novalue, discardingisLimitActiveandisLimitChangeablealong with it.A real Controllable System (Wolf Link heat pump module, LPC v1.0.0,
scenarioSupport [1,2,3,4]) answers aloadControlLimitListDataread like this:{"loadControlLimitData":[[{"limitId":1},{"isLimitChangeable":true},{"isLimitActive":false}]]}There is no limit in effect, so no value is stated. The information the client asks for — is a limit active? — is present and unambiguous, but the reader turns it into an error, and the Energy Guard cannot tell "no limit active" from "no data at all". Downstream this surfaces as a permanent error state for a heat pump that simply isn't being limited (evcc-io/evcc#32112, evcc-io/evcc#32126).
A value is only required to interpret an active limit. This changes the four affected readers to accept an inactive limit without a value and report it as
IsActive: false, Value: 0, while an active limit without a value still returnsErrDataNotAvailable:usecases/eg/lpcConsumptionLimitusecases/eg/lppProductionLimitusecases/cs/lpcConsumptionLimitusecases/cs/lppProductionLimitOther value guards in the code base were checked and are unaffected:
usecases/internal/loadcontrol.goalready reports the permitted maximum when a limit has no value or is inactive, and the remaining ones (ma/mpc,ma/mgcp,ma/mdt,eg/*failsafe and nominal-max readers,cem/*) read measurements, characteristics and configuration keys, where a missing value genuinely means no data.🤖 Generated with Claude Code