SmartCharging use cases K09 and K10 #743
Conversation
Pietfried
force-pushed
the
feature/k09-k10-get-profiles-clear-profiles
branch
from
9fd64ec to
17f65d1
Compare
Pietfried
force-pushed
the
feature/k09-k10-get-profiles-clear-profiles
branch
3 times, most recently
from
277c004 to
2655505
Compare
|
|
||
| // Functional Block K: Smart Charging | ||
| void handle_set_charging_profile_req(Call<SetChargingProfileRequest> call); | ||
| void handle_clear_charging_profile_req(Call<ClearChargingProfileRequest> call); |
There was a problem hiding this comment.
Why do we pass these by value? We pass by value for all calls but I think it could very well be const &
| EVLOG_debug << "Rejecting SetChargingProfileRequest:\n reasonCode: " << response.statusInfo->reasonCode.get() | ||
| << "\nadditionalInfo: " << response.statusInfo->additionalInfo->get(); | ||
| } else { | ||
| response = this->smart_charging_handler->clear_profiles(msg); |
There was a problem hiding this comment.
I am a bit worried about the responsibilities of the smart charging handler. We filter out some edge cases but then still hand over the whole message to clear_profiles and get_reported_profiles. I think we should either move the whole handling of messages there or put an in between layer that handles the full message here. This feels like a kind of halfway solution.
There was a problem hiding this comment.
Also this is based on the behavior of handle_set_charging_profile_req. We are actually checking a requirement for the CSMS (K10.FR.06) here and then we hand it over to the SmartChargingHandler. If we change it, I would change it for both occurences
There was a problem hiding this comment.
For both of these I probably missed it earlier then or it was not as apparent. Maybe we don't need to do anything with it now but we are increasing coupling this way so certainly something to look out for later.
marcemmers
left a comment
marcemmers
left a comment
There was a problem hiding this comment.
In general looks quite good I think. There are some things I would improve but they are more nice to haves and should not change the functionality so up to you to do them or not.
Pietfried
force-pushed
the
feature/k09-k10-get-profiles-clear-profiles
branch
from
814fb55 to
eee6e17
Compare
Pietfried
force-pushed
the
feature/k09-k10-get-profiles-clear-profiles
branch
from
eee6e17 to
02f0193
Compare
…oint Signed-off-by: pietfried <pietgoempel@gmail.com>
Signed-off-by: pietfried <pietgoempel@gmail.com>
Signed-off-by: pietfried <pietgoempel@gmail.com>
…it path to inside if statement Signed-off-by: pietfried <pietgoempel@gmail.com>
Co-authored-by: Marc Emmers <35759328+marcemmers@users.noreply.github.com> Signed-off-by: Piet Gömpel <37657534+Pietfried@users.noreply.github.com> Signed-off-by: pietfried <pietgoempel@gmail.com>
Signed-off-by: pietfried <pietgoempel@gmail.com>
Signed-off-by: pietfried <pietgoempel@gmail.com>
Signed-off-by: pietfried <pietgoempel@gmail.com>
Signed-off-by: pietfried <pietgoempel@gmail.com>
Signed-off-by: pietfried <pietgoempel@gmail.com>
Signed-off-by: pietfried <pietgoempel@gmail.com>
Pietfried
force-pushed
the
feature/k09-k10-get-profiles-clear-profiles
branch
from
4375639 to
eb6b064
Compare
Describe your changes
Implements SmartCharging use cases K09 and K10.
Adds support for the following messages:
Tested Cases tested with OCTT:
TC_K_05
TC_K_06
TC_K_07
TC_K_08
TC_K_09 fails because GetCompositeSchedule handling is a prerequisite.
TC_K_24
TC_K_29
TC_K_30
TC_K_31
TC_K_32
TC_K_33
TC_K_34
TC_K_35
TC_K_36
This PR is branched off #711 .
Additional requires some changes in the code generator, which will be applied in this PR: #741
Issue ticket number and link
#309
#369
Checklist before requesting a review