Skip to content

Commit 6587cd4

Browse files
PietfriedPietfried
authored
Fixed bug that lead to incorrect transaction handling in case StartTransaction.req could not be delivered: (#688)
* StartTransaction.conf handler retrieves a transaction based on the message id of the StartTransaction.req. In case this message is retried, the reference is updated in the message queue but not in the transaction handler * This change updates the start_transaction_message_id of the transaction of the transaction_handler and in the database in case of a retry Signed-off-by: pietfried <pietgoempel@gmail.com>
1 parent ee0f1eb commit 6587cd4

File tree

7 files changed

+89
-8
lines changed

7 files changed

+89
-8
lines changed

‎include/ocpp/common/message_queue.hpp

+37-7
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,21 @@ bool is_transaction_message(const ocpp::v16::MessageType message_type);
113113
/// \return true if MessageType is TransactionEvent or SecurityEventNotification
114114
bool is_transaction_message(const ocpp::v201::MessageType message_type);
115115

116+
/// \brief Indicates if the given \p message_type is a StartTransaction message
117+
/// \param message_type
118+
/// \return true if MessageType is a StartTransaction
119+
bool is_start_transaction_message(const ocpp::v16::MessageType message_type);
120+
121+
/// \brief Indicates if the given \p message_type is a StartTransaction message.
122+
/// \param message_type
123+
/// \return Always return false
124+
bool is_start_transaction_message(const ocpp::v201::MessageType message_type);
125+
126+
/// \brief Indicates if the given \p control_message is a start transaction message
127+
template <typename M> auto is_start_transaction_message(const ControlMessage<M>& control_message) {
128+
return is_start_transaction_message(control_message.messageType);
129+
}
130+
116131
/// \brief Indicates if the given \p control_message is a transaction related message
117132
template <typename M> auto is_transaction_message(const ControlMessage<M>& control_message) {
118133
return is_transaction_message(control_message.messageType);
@@ -186,6 +201,10 @@ template <typename M> class MessageQueue {
186201
// was queued. This can happen when the CP has not received a StartTransaction.conf from the CSMS.
187202
std::map<std::string, std::vector<std::string>> start_transaction_mid_meter_values_mid_map;
188203

204+
// This callback is called when a StartTransaction.req message could not be delivered due to a timeout or CALL_ERROR
205+
std::function<void(const std::string& new_message_id, const std::string& old_message_id)>
206+
start_transaction_message_retry_callback;
207+
189208
MessageId getMessageId(const json::array_t& json_message) {
190209
return MessageId(json_message.at(MESSAGE_ID).get<std::string>());
191210
}
@@ -363,9 +382,12 @@ template <typename M> class MessageQueue {
363382

364383
public:
365384
/// \brief Creates a new MessageQueue object with the provided \p configuration and \p send_callback
366-
MessageQueue(const std::function<bool(json message)>& send_callback, const MessageQueueConfig& config,
367-
const std::vector<M>& external_notify,
368-
std::shared_ptr<common::DatabaseHandlerCommon> database_handler) :
385+
MessageQueue(
386+
const std::function<bool(json message)>& send_callback, const MessageQueueConfig& config,
387+
const std::vector<M>& external_notify, std::shared_ptr<common::DatabaseHandlerCommon> database_handler,
388+
const std::function<void(const std::string& new_message_id, const std::string& old_message_id)>
389+
start_transaction_message_retry_callback =
390+
[](const std::string& new_message_id, const std::string& old_message_id) {}) :
369391
database_handler(std::move(database_handler)),
370392
config(config),
371393
external_notify(external_notify),
@@ -374,7 +396,8 @@ template <typename M> class MessageQueue {
374396
running(true),
375397
new_message(false),
376398
is_registration_status_accepted(false),
377-
uuid_generator(boost::uuids::random_generator()) {
399+
uuid_generator(boost::uuids::random_generator()),
400+
start_transaction_message_retry_callback(start_transaction_message_retry_callback) {
378401

379402
this->send_callback = send_callback;
380403
this->in_flight = nullptr;
@@ -446,10 +469,12 @@ template <typename M> class MessageQueue {
446469
return false;
447470
};
448471

449-
// Find the first allowed transaction message
472+
// Transaction messages must persist the order, so only check the first in the queue
450473
auto selected_transaction_message_it =
451-
std::find_if(transaction_message_queue.begin(), transaction_message_queue.end(),
452-
is_transaction_message_available);
474+
(!transaction_message_queue.empty() and
475+
is_transaction_message_available(transaction_message_queue.front()))
476+
? transaction_message_queue.begin()
477+
: transaction_message_queue.end();
453478

454479
if (selected_transaction_message_it != transaction_message_queue.end()) {
455480
message = *selected_transaction_message_it;
@@ -808,6 +833,7 @@ template <typename M> class MessageQueue {
808833
if (this->in_flight->message_attempts < this->config.transaction_message_attempts) {
809834
EVLOG_warning << "Message shall be persisted and will therefore be sent again";
810835
// Generate a new message ID for the retry
836+
const auto old_message_id = this->in_flight->message[MESSAGE_ID];
811837
this->in_flight->message[MESSAGE_ID] = this->createMessageId();
812838
if (this->config.transaction_message_retry_interval > 0) {
813839
// exponential backoff
@@ -832,6 +858,10 @@ template <typename M> class MessageQueue {
832858
} else if (queue_type == QueueType::Normal) {
833859
this->normal_message_queue.push_front(this->in_flight);
834860
}
861+
if (is_start_transaction_message(*this->in_flight)) {
862+
this->start_transaction_message_retry_callback(this->in_flight->message[MESSAGE_ID],
863+
old_message_id);
864+
}
835865
this->notify_queue_timer.at(
836866
[this]() {
837867
this->new_message = true;

‎include/ocpp/v16/database_handler.hpp

+5
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,11 @@ class DatabaseHandler : public ocpp::common::DatabaseHandlerCommon {
7272
/// table
7373
void update_transaction_csms_ack(const int32_t transaction_id);
7474

75+
/// \brief Updates the START_TRANSACTION_MESSAGE_ID column for the transaction with the given \p session_id in the
76+
/// TRANSACTIONS table
77+
void update_start_transaction_message_id(const std::string& session_id,
78+
const std::string& start_transaction_message_id);
79+
7580
/// \brief Updates the METER_LAST and METER_LAST_TIME column for the transaction with the given \p session_id in the
7681
/// TRANSACTIONS table
7782
void update_transaction_meter_value(const std::string& session_id, const int32_t value,

‎lib/ocpp/v16/charge_point_impl.cpp

+21-1
Original file line numberDiff line numberDiff line change
@@ -155,14 +155,34 @@ ChargePointImpl::ChargePointImpl(const std::string& config, const fs::path& shar
155155
}
156156

157157
std::unique_ptr<ocpp::MessageQueue<v16::MessageType>> ChargePointImpl::create_message_queue() {
158+
159+
// The StartTransaction.conf handler attempts to get the transaction based on the message id. The message id changes
160+
// in case of a message retry attempt, so we need to update it for the transaction as well
161+
const auto start_transaction_message_retry_callback = [this](const std::string& new_message_id,
162+
const std::string& old_message_id) {
163+
auto transaction = this->transaction_handler->get_transaction(old_message_id);
164+
if (transaction != nullptr) {
165+
transaction->set_start_transaction_message_id(new_message_id);
166+
try {
167+
this->database_handler->update_start_transaction_message_id(transaction->get_session_id(),
168+
new_message_id);
169+
} catch (const QueryExecutionException& e) {
170+
EVLOG_warning << "Could not update start transaction message id";
171+
}
172+
} else {
173+
EVLOG_warning << "Could not find transaction with start_transaction_message_id: " << old_message_id
174+
<< " and could therefore not replace it with: " << new_message_id;
175+
}
176+
};
177+
158178
return std::make_unique<ocpp::MessageQueue<v16::MessageType>>(
159179
[this](json message) -> bool { return this->websocket->send(message.dump()); },
160180
MessageQueueConfig{
161181
this->configuration->getTransactionMessageAttempts(),
162182
this->configuration->getTransactionMessageRetryInterval(),
163183
this->configuration->getMessageQueueSizeThreshold().value_or(DEFAULT_MESSAGE_QUEUE_SIZE_THRESHOLD),
164184
this->configuration->getQueueAllMessages().value_or(false)},
165-
this->external_notify, this->database_handler);
185+
this->external_notify, this->database_handler, start_transaction_message_retry_callback);
166186
}
167187

168188
void ChargePointImpl::init_websocket() {

‎lib/ocpp/v16/database_handler.cpp

+14
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,20 @@ void DatabaseHandler::update_transaction_csms_ack(const int32_t transaction_id)
136136
}
137137
}
138138

139+
void DatabaseHandler::update_start_transaction_message_id(const std::string& session_id,
140+
const std::string& start_transaction_message_id) {
141+
std::string sql = "UPDATE TRANSACTIONS SET START_TRANSACTION_MESSAGE_ID=@start_transaction_message_id, "
142+
"LAST_UPDATE=@last_update WHERE ID==@session_id";
143+
auto stmt = this->database->new_statement(sql);
144+
145+
stmt->bind_text("@last_update", ocpp::DateTime().to_rfc3339(), SQLiteString::Transient);
146+
stmt->bind_text("@start_transaction_message_id", start_transaction_message_id);
147+
148+
if (stmt->step() != SQLITE_DONE) {
149+
throw QueryExecutionException(this->database->get_error_message());
150+
}
151+
}
152+
139153
void DatabaseHandler::update_transaction_meter_value(const std::string& session_id, const int32_t value,
140154
const std::string& last_meter_time) {
141155
std::string sql = "UPDATE TRANSACTIONS SET METER_LAST=@meter_last, METER_LAST_TIME=@meter_last_time, "

‎lib/ocpp/v16/message_queue.cpp

+4
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,10 @@ bool is_transaction_message(const ocpp::v16::MessageType message_type) {
2222
(message_type == v16::MessageType::SecurityEventNotification);
2323
}
2424

25+
bool is_start_transaction_message(const ocpp::v16::MessageType message_type) {
26+
return message_type == v16::MessageType::StartTransaction;
27+
}
28+
2529
bool is_boot_notification_message(const ocpp::v16::MessageType message_type) {
2630
return message_type == ocpp::v16::MessageType::BootNotification;
2731
}

‎lib/ocpp/v201/message_queue.cpp

+4
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@ bool is_transaction_message(const ocpp::v201::MessageType message_type) {
1212
(message_type == v201::MessageType::SecurityEventNotification);
1313
}
1414

15+
bool is_start_transaction_message(const ocpp::v201::MessageType message_type) {
16+
return false;
17+
}
18+
1519
bool is_boot_notification_message(const ocpp::v201::MessageType message_type) {
1620
return message_type == ocpp::v201::MessageType::BootNotification;
1721
}

‎tests/lib/ocpp/common/test_message_queue.cpp

+4
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,10 @@ bool is_transaction_message(const TestMessageType message_type) {
124124
return (message_type == TestMessageType::TRANSACTIONAL) || (message_type == TestMessageType::TRANSACTIONAL_UPDATE);
125125
}
126126

127+
bool is_start_transaction_message(const TestMessageType message_type) {
128+
return false;
129+
}
130+
127131
template <> bool ControlMessage<TestMessageType>::is_transaction_update_message() const {
128132
return this->messageType == TestMessageType::TRANSACTIONAL_UPDATE;
129133
}

0 commit comments

Comments
 (0)