Session cost add message #1079
Session cost add message #1079
Conversation
maaikez
requested review from
hikinggrass,
Pietfried,
corneliusclaussen,
SebaLukas,
AssemblyJohn,
MarzellT and
andistorm
as code owners
maaikez
force-pushed
the
feature/session-cost-add-message
branch
from
275079e to
85d1835
Compare
| @@ -67,7 +67,7 @@ libevse-security: | |||
| # OCPP | |||
| libocpp: | |||
| git: https://github.com/EVerest/libocpp.git | |||
| git_tag: 6533694e8a25925985079bda33d5610f65c6d58f | |||
| git_tag: feature/session-cost-add-message | |||
There was a problem hiding this comment.
TODO: update when libocpp PR is merged.
…cing information. Signed-off-by: Maaike Zijderveld, iolar <[email protected]>
… to session cost message. Signed-off-by: Maaike Zijderveld, iolar <[email protected]>
Signed-off-by: Maaike Zijderveld, iolar <[email protected]>
maaikez
force-pushed
the
feature/session-cost-add-message
branch
from
9073f7f to
c5a5814
Compare
Signed-off-by: Maaike Zijderveld, iolar <[email protected]>
…ction. Signed-off-by: Maaike Zijderveld, iolar <[email protected]>
| session_cost_message: | ||
| description: Message to show to the user with information about the session cost. | ||
| type: object | ||
| $ref: /session_cost#/SessionCostMessage |
There was a problem hiding this comment.
Does it make sense to send the session cost and session cost message independently? Would it not be easier to extend the existing session cost with an optional "text" field so the price and the info for a display are delivered at once?
also the name session_cost_message is really not ideal since we already have session_cost as message
There was a problem hiding this comment.
I do agree that the naming is not ideal. The messages are seperated because they serve different purposes and are triggered at different times from the CSMS. This is required by the california pricing requirements and OCPP2.0.1. Im not sure if all required data is available at every time to actually combine the messages. @maaikez what do you think?
| for (const ocpp::DisplayMessageContent& message : running_cost.cost_messages.value()) { | ||
| types::display_message::MessageContent m = to_everest_display_message_content(message); | ||
| types::text_message::MessageContent m = to_everest_display_message_content(message); |
There was a problem hiding this comment.
Just a minor note from me but wouldn't:
const auto m = to_everest_display_message_content(message);
be cleaner?
| @@ -61,64 +61,64 @@ ocpp::v2::MessageStateEnum to_ocpp_201_display_message_state(const types::displa | |||
| throw std::out_of_range("Could not convert types::display_message::MessageStateEnum to ocpp::v2::MessageStateEnum"); | |||
| } | |||
|
|
|||
| types::display_message::MessageFormat | |||
| types::text_message::MessageFormat | |||
There was a problem hiding this comment.
I think you are explicitly using types::text_message::MessageFormat and types::display_message::IdentifierType everywhere. Wouldn't it be better to just declare them with using?
Describe your changes
Do not use the display message interface to send session cost messages, but use a specific session cost message type.
Issue ticket number and link
Checklist before requesting a review