Add AuthorizationSetup to EV side state machine, including a feedback mechanism to report EVSE parameters to the EV.#165
Conversation
Signed-off-by: Roger Bedell <rogerbedell@hotmail.com>
cienporcien
requested review from
SebaLukas,
a-w50,
james-ctc and
mlitre
as code owners
|
@cienporcien I'll review this PR this week. |
SebaLukas
left a comment
SebaLukas
left a comment
There was a problem hiding this comment.
Overall looks good!
Only some small changes needed :)
| if (pnc_auth_mode.supported_providers.has_value()) { | ||
| m_ctx.evse_session_info.supported_providers = pnc_auth_mode.supported_providers.value(); | ||
| } |
There was a problem hiding this comment.
Because m_ctx.evse_session_info.supported_providers is also optional, it is enough to write:
m_ctx.evse_session_info.supported_providers = pnc_auth_mode.supported_providers;
Then you dont need the if.
There was a problem hiding this comment.
Your changes are here not correct: m_ctx.evse_session_info.supported_providers = pnc_auth_mode.supported_providers.value();
Please remove .value()
| bool check_response_code(ResponseCode response_code) { | ||
| switch (response_code) { | ||
| case ResponseCode::OK: | ||
| return true; | ||
| [[fallthrough]]; | ||
| default: | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Other OK_* and Warning_* response codes are also acceptable here and should not lead to immediate termination. Only Failed_* response codes should lead to termination.
In the future every state should check if the OK_* or Warning_* response_code is really used in the state according to the standard and react accordingly.
There was a problem hiding this comment.
I was looking at Table 224 in the standard, and for AuthorizationSetupRes it only has OK, FAILED, FAILED_SequenceError, and FAILED_UnknownSession. So I think it is ok as is.
There was a problem hiding this comment.
The problem is with this approach, that if the charger sends a different OK_ or Warning_ ResponseCode as expected then the ev state machine abort the session. Thats not ideal. I think it is okay that the car can continue the session
Signed-off-by: Roger Bedell <rogerbedell@hotmail.com>
2 participants
Describe your changes
Add AuthorizationSetup to EV side state machine, including a feedback mechanism to report EVSE parameters to the EV.
Issue ticket number and link
#163
Checklist before requesting a review