WIP: Refactor of message dispatching #849
Conversation
| +dispatch_call_async(const json& call, bool triggered = false): std::future<EnhancedMessage<T>> | ||
| +dispatch_call_result(const json& call_result) | ||
| +dispatch_call_error(const json& call_error) | ||
| +new_message_id(): MessageId |
There was a problem hiding this comment.
Do we even need to bother with message IDs for users of this layer? The layer should be able to take care of it itself?
There was a problem hiding this comment.
Yes, this functionality does not necessarily have to be part of this interface
There was a problem hiding this comment.
Looking at it, I think we might want to move the createMessageId functionality out of the message queue, it can be a free function. Then on construction of a Call<T> type we can generate a message id internally and essentially hide this value.
| +dispatch_call_async(const json& call, bool triggered = false): std::future<EnhancedMessage<T>> | ||
| +dispatch_call_result(const json& call_result) | ||
| +dispatch_call_error(const json& call_error) | ||
| +new_message_id(): MessageId |
There was a problem hiding this comment.
Looking at it, I think we might want to move the createMessageId functionality out of the message queue, it can be a free function. Then on construction of a Call<T> type we can generate a message id internally and essentially hide this value.
|
|
||
| interface v201::DataTransferInterface { | ||
| +data_transfer_req(request: DataTransferRequest): std::optional<DataTransferResponse> | ||
| +handle_data_transfer_req(call: Call<DataTransferRequest>) |
There was a problem hiding this comment.
Should we replace this with a handle_message function that takes an EnhancedMessage<v201::MessageType>?
For this class there is only one message to handle but ideally that should not change the API for these functional blocks. If we later have to add a message to handle we can do it purely in the functional block
|
|
||
| @startuml | ||
|
|
||
| interface MessageDispatcherInterface { |
There was a problem hiding this comment.
This could also be named IHandleMessage if we really want haha
| @@ -75,6 +75,11 @@ ChargePoint::ChargePoint(const std::map<int32_t, int32_t>& evse_connector_struct | |||
| } | |||
|
|
|||
| initialize(evse_connector_structure, message_log_path); | |||
|
|
|||
| this->message_dispatcher = | |||
There was a problem hiding this comment.
Might want to check here if message_queue exists because this constructor is sometimes called with nullptr.
| request.vendorId = "test_vendor"; | ||
| ocpp::Call<DataTransferRequest> call(request, "unique_id_123"); | ||
|
|
||
| EXPECT_CALL(mock_dispatcher, dispatch_call_result(_)).WillOnce(Invoke([](const json& call_result) { |
There was a problem hiding this comment.
I feel like this is going to be worth investing some time in making these expectations nicer. Way outside of scope for now but would be smart to do before we make more of these tests.
I don't think we really care whether dispatch_call or dispatch_call_async is called for example, as long as one of those gets called for tests where that is a thing.
Also I would like to create custom matchers to set expectations on specified fields of structs so you don't have to do the invoke construction.
| })); | ||
|
|
||
| data_transfer.handle_data_transfer_req(call); | ||
| } |
There was a problem hiding this comment.
Missing tests for data_transfer_req in here.
* Added new class MessageDispatcherInterface that can dispatch OCPP Call, CallResult, CallError * MessageDispatcherInterface became member of ChargePoint classes. The templated methods for sending/dispatching Call, CallResult, CallError have been moved from ChargePoint classes to MessageDispatcher * Added example implementation of one functional block DataTransfer including two tests Signed-off-by: Piet Gömpel <[email protected]>
Signed-off-by: Piet Gömpel <[email protected]>
Pietfried
force-pushed
the
feature/message-dispatcher
branch
from
1e47ab5
to
b532341
Compare
Describe your changes
Large refactor of message handling:
Issue ticket number and link
Checklist before requesting a review