- Fix TC_SECC_CMN_VTB_CmSlacParm_004/5/6 #928
Conversation
corneliusclaussen
added
the
post-release
- Fix TC_SECC_CMN_VTB_CmSlacMatch_003 - Add reset event handling while waiting for Link - Add link detection to matching state and reset if link got lost Signed-off-by: Cornelius Claussen <cc@pionix.de>
corneliusclaussen
force-pushed
the
bugfix/slac_tests
branch
from
bdf3672 to
0062b97
Compare
| FSMSimpleState::CallbackReturnType MatchedState::callback() { | ||
| const auto& cfg = ctx.slac_config; | ||
|
|
||
| if (not ctx.slac_config.link_status.do_detect) { |
There was a problem hiding this comment.
Why not using cfg here, already? Could even use const auto& link_status = ctx.slac_config.link_status
| } else { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
would drop the else here. Just return false; at the end of the function
| @@ -212,6 +213,7 @@ struct Context { | |||
| void log_info(const std::string& text); | |||
|
|
|||
| ModemVendor modem_vendor{ModemVendor::Unknown}; | |||
| slac::messages::cm_slac_match_cnf match_cnf_message; | |||
There was a problem hiding this comment.
This doesn't really belong here, as it is shared information only between Matching and WaitForLink state. Matching state should keep it for itself and pass it to WaitForLink state with create_simple<WaitForLink>(ctx, std::move(match_cnf_message)). You would need to modify the constructor of WaitForLink state.
Signed-off-by: aw <aw@pionix.de>
There was a problem hiding this comment.
Style-wise I'm fine. But it looks like the only necessary change for the 4/5/6 tests is the checking for a valid cm_slac_match_req message.
Everything else seems to be unrelated and I would appreciate if we could add this in a separate commit with corresponding documentation (i.e. what the WaitForLinkState is for and why it can resend the slac_match_cnf message).
| @@ -72,7 +72,8 @@ FSMSimpleState::CallbackReturnType MatchingState::callback() { | |||
|
|
|||
| if (!seen_slac_parm_req) { | |||
| if (now_tp >= timeout_slac_parm_req) { | |||
| return Event::RETRY_MATCHING; | |||
| ctx.log_info("CM_SLAC_PARM_REQ timed out -> FAILED"); | |||
| return Event::FAILED; | |||
There was a problem hiding this comment.
This change should not be necessary for the tests 4/5/6 to succeed, because it still won't sent any confirm message.
Nevertheless, according to [V2G3-A09-13] it shall consider, that no SLAC will be performed.
@corneliusclaussen might this be overpessimistic for slower cars?
While looking at the implementation, it also seems we don't handle [V2G3-A09-45].
Describe your changes
Issue ticket number and link
Checklist before requesting a review