[WIP] Fix smart charger OCTT test cases #424
Conversation
…hedule even without active transaction
|
I will add unit tests for the other 3 TC so we can easily port the fixes to the Also, according to "chargingSchedulePeriod": {
"type": "array",
"items": {
"type": "object",
"properties": {
"startPeriod": {
"type": "integer"
},
"limit": {
"type": "number",
"multipleOf" : 0.1
},
"numberPhases": {
"type": "integer"
}
},
"additionalProperties": false,
"required": [
"startPeriod",
"limit"
]
}
},... so we cannot (or should not) return BUT, to keep the library CP agnostic, after discussion with @matth-x we agreed to make those configurable as defines MO_, still |
…ine s instead of hardcoded -1 to pass the OCTT SmartCharging tests
matth-x
left a comment
matth-x
left a comment
There was a problem hiding this comment.
@razvanphp Thanks again for the valuable fixes! Only change request is to keep the SmartChargingOutput pasing -1s if no limit is defined
| #ifndef MO_MaxChargingLimitNumberPhases | ||
| #define MO_MaxChargingLimitNumberPhases -1 | ||
| #endif | ||
|
|
There was a problem hiding this comment.
Good and simple solution for most use cases
| nextChange = basis + period->startPeriod; | ||
| nextChange = std::min(nextChange, basis + period->startPeriod); | ||
| const Timestamp candidate = basis + period->startPeriod; | ||
| nextChange = std::min(nextChange, candidate); |
|
|
||
| //if no TxProfile limits charging, check the TxDefaultProfiles for this connector | ||
| if (!txLimitDefined && trackTxStart < MAX_TIME) { | ||
| if (!txLimitDefined) { |
There was a problem hiding this comment.
This fixes the composite schedules. But also affects the ordinary charge limit output if no transaction is running. I cannot think of scenarios where this would be super bad though.
The usage of the global trackTxStart in calculateLimit() seems not to work well with composite schedules. In MO v2, it would be a possibility to add a new function parameter to calculateLimit() to alter the limit determination logic depending on where the output is used
Looks good for now!
| limit.nphases != std::numeric_limits<int>::max() ? limit.nphases : -1); | ||
| limit.power != std::numeric_limits<float>::max() ? limit.power : MO_MaxChargingLimitPower, | ||
| limit.current != std::numeric_limits<float>::max() ? limit.current : MO_MaxChargingLimitCurrent, | ||
| limit.nphases != std::numeric_limits<int>::max() ? limit.nphases : MO_MaxChargingLimitNumberPhases); |
There was a problem hiding this comment.
Would be nice to preserve the -1 value for "undefined" limit. It could make a difference on a charger if no charging schedule is set at all, or if a charging schedule is set and its limit just happens to match the electric rating
| periods.back().numberPhases = limit.nphases != std::numeric_limits<int>::max() ? limit.nphases : MO_MaxChargingLimitNumberPhases; | ||
| periods.back().startPeriod = periodBegin - startSchedule; | ||
| lastLimit = limit; | ||
| } |
| limit.nphases != std::numeric_limits<int>::max() ? limit.nphases : -1); | ||
| limit.power != std::numeric_limits<float>::max() ? limit.power : MO_MaxChargingLimitPower, | ||
| limit.current != std::numeric_limits<float>::max() ? limit.current : MO_MaxChargingLimitCurrent, | ||
| limit.nphases != std::numeric_limits<int>::max() ? limit.nphases : MO_MaxChargingLimitNumberPhases); |
2 participants
This PR makes all those tests pass:
BUT, it will only work for v1.1.0 and v1.2.0, it is not compatible with the upcoming develop/remodel-api branch. I think there @matth-x already fixed those bugs.
We should also have the max A of the charging station and number of phases as configuration variables probably, as OCTT expects a positive value there instead of
-1when there is no Profile installed.I could/should also add unit tests for this...
closes #360