EcoG-io / iso15118

Implementation of the ISO 15118 Communication Protocol (-2, -20, -8)
Apache License 2.0
156 stars 87 forks source link

Sequence Error after Welding Detection #212

Open jpo-stx opened 1 year ago

jpo-stx commented 1 year ago

There is an issue with the welding detection processing:

This is not correct according to the specification:

[V2G20-1630] After receiving the DC_WeldingDetectionReq with EVProcessing set to ‘Ongoing’, the SECC shall respond with a DC_WeldingDetectionRes within V2G_SECC_Msg_Performance_Time according to Table 217. The next allowed request shall be DC_WeldingDetectionReq and the V2G_SECC_Sequence_Timeout is set according to Table 217.

[V2G20-1631] After receiving the DC_WeldingDetectionReq with EVProcessing set to ‘Finished’, the SECC shall respond with a DC_WeldingDetectionRes within V2G_SECC_Msg_Performance_Time according to Table 217. The next allowed request shall be SessionStopReq and the V2G_SECC_Sequence_Timeout is set according to Table 215.

As long as a WeldingDetectionReq with 'Ongoing' is sent the next request message has to be a WeldingDetctionReq

tropxy commented 1 year ago

Hi, thanks for your message.

I confirm that the EV side implementation for this interaction is incorrect. It should be easy to fix it: https://github.com/SwitchEV/iso15118/blob/master/iso15118/evcc/states/iso15118_20_states.py#L1721

one way is to save in a class attribute if we are entering the DCWeldingDetection for the first time or not. If yes, then we answer with a DCWeldingDetectionReq right away, but if not then we check if the welding_detection_has_finished or not to decide if we go to SessionStopReq or to another DCWeldingDetectionReq.

Just to be fully transparent, we use the EVCC more as a simulator, so our rigour for the EV side does not match the level of detail we put in the SECC side. Also, to ensure that our solution for the SECC is robust, we test it against a conformance testing unit provided by Keysight.

This said, if you would wish to contribute to the project with a fix for this, we would appreciate.

jpo-stx commented 1 year ago

Thank you for the promt response!

I already implemented a workaround. I can create a pull request.

I think it only affects the ISO15118-20. In the ISO15118-2 and DIN70121-2 protocol the EV can send the SessionStopReq after the first WeldingDetectionRes was received, because there is no processing parameter in the "old" protocols.

tropxy commented 1 year ago

Yes, we would be very happy with your contribution @jpo-stx