Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed the EVSE CM_SLAC_PARAM.CNF response to match the ISO 15118-3 standard #134

Merged
merged 1 commit into from
Oct 7, 2020

Conversation

pedrorivera
Copy link

I wasn't able to use the demo SLAC procedure implemented in evse.c with a real ECU. It turns out at least this big name EVCC implementation requires the CM_SLAC_PARAM.CNF MME to exactly comply with the table A.2 provided in ISO 15118-3. This involves:

  • Setting FORWARDING_STA to the EV HLE address
  • Setting RESP_TYPE, NUM_SOUNDS, and TIME_OUT to the values indicated in the std. (1, 10, 6 resp.)

…C_PARAM.CNF response, modified evse.ini to match the config established by table A.2 of ISO 15118-3
@mhei
Copy link
Contributor

mhei commented Jul 8, 2020

When you change EVSE side here, isn't a chance to PEV side necessary then, too?

@pedrorivera
Copy link
Author

@mhei there is no adaptation necessary on the PEV side, since the code in pev_cm_slac_param.c:150 already takes the FORWARDING_STA from the EVSE confirmation and copies it to the session variable:

memcpy (session->FORWARDING_STA, confirm->FORWARDING_STA, sizeof (session->FORWARDING_STA));

An assertion would be nice, but being a demo, I don't think that level of error handling is justified. Oder?

@aduskett
Copy link

aduskett commented Oct 6, 2020

I just wanted to chime in and also say this fixed my issues with SLAC negotiation. I'm perplexed as to why Qualcomm hasn't merged this simple fix in yet.

@n1000
Copy link
Member

n1000 commented Oct 7, 2020

Thanks for the update on this issue... I don't currently have any way to fully test this (this is the main reason this hasn't been merged yet). Based on feedback from multiple parties showing this fix is good, I will merge this now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants