Updates to store EV and EVSE limits shared in CPD and CL (-20 AC and DC). #279
Conversation
shalinnijel2
changed the title
shalinnijel2
changed the title
|
|
||
| # Specific to ISO 15118-20 DC BPT | ||
| ev_max_discharge_current: Optional[float] = None | ||
| ev_min_discharge_current: Optional[float] = None |
There was a problem hiding this comment.
…into dc_bpt_change
| ac_limits: Optional[ACLimits] = None | ||
| ac_bpt_limits: Optional[ACBPTLimits] = None |
There was a problem hiding this comment.
do we need to separate ac_limits from the ac_bpt_limits? cant we have both together?
There was a problem hiding this comment.
My thinking was as cs limits splits these, this would make it easier for implementers. This would make it evident that the bpt bit could be skipped if not interesting.
There was a problem hiding this comment.
But if I am not mistaken, if you use the BPT ones, you also need the regular ones...the BPT ones are supposed to be an extension of the regular ones, right?
And here, one could create a structure only with the BPT ones, neglecting the regular ones.
wont that be an issue?
There was a problem hiding this comment.
Yes, if picking the bpt version, the non-bpt equivalent will also be required.
We can't make ac_limits non optional here as dc_limits also lives in the same RatedLimits dataclass.
One option is to let the bpt limits live as optional ones in the ac_limits/dc_limits dataclass. But splitting them makes it slightly easier to read I think. But as these rated limits are built from cs-config we can clarify (will have to be in the documentation though) that this is needed. Hmm, do we need add a post_init() to validate?
| await self.comm_session.evse_controller.send_charging_power_limits( | ||
| self.comm_session.protocol, | ||
| control_mode, | ||
| selected_energy_service.service, | ||
| ) |
There was a problem hiding this comment.
shouldnt we guard this with try except? what if it never returns or throws an exception?
There was a problem hiding this comment.
Yes, wrapped in try catch. The idea at the moment being any sort of error would be caught in the interface implementation, re-thrown as a timeout exception and the reason passed to the EV.
|
|
||
|
|
||
| @dataclass | ||
| class EVSEACLimits(Limits): |
There was a problem hiding this comment.
can we change the name to EVSEACCPDLimits and add a description to the class expanding the name
iso15118/secc/controller/ev_data.py
Outdated
| ac_limits: Optional[EVACCLLimits] = None | ||
| dc_limits: Optional[EVDCCLLimits] = None |
There was a problem hiding this comment.
what is the purpose of these ones?
There was a problem hiding this comment.
Done - removed.
In this PR: