Skip to content

Commit 16bd96b

Browse files
dorezyukDima Dorezyuk
andauthored
EvseManager: fix race condition with the last_cp_state (#1892)
Signed-off-by: Dima Dorezyuk <ddo@qwello.eu> Co-authored-by: Dima Dorezyuk <ddo@qwello.eu>
1 parent ce19c1c commit 16bd96b

File tree

2 files changed

+20
-11
lines changed

2 files changed

+20
-11
lines changed

modules/EVSE/EvseManager/IECStateMachine.cpp

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,8 @@ IECStateMachine::IECStateMachine(const std::unique_ptr<evse_board_supportIntf>&
8383
bool lock_connector_in_state_b_) :
8484
r_bsp(r_bsp_), lock_connector_in_state_b(lock_connector_in_state_b_) {
8585
// feed the state machine whenever the timer expires
86-
timeout_state_c1.signal_reached.connect([this]() { feed_state_machine(last_cp_state); });
87-
timeout_unlock_state_F.signal_reached.connect([this]() { feed_state_machine(last_cp_state); });
86+
timeout_state_c1.signal_reached.connect([this]() { feed_state_machine(std::nullopt); });
87+
timeout_unlock_state_F.signal_reached.connect([this]() { feed_state_machine(std::nullopt); });
8888

8989
// Subscribe to bsp driver to receive BspEvents from the hardware
9090
r_bsp->subscribe_event([this](const types::board_support_common::BspEvent event) {
@@ -118,11 +118,12 @@ void IECStateMachine::process_bsp_event(const types::board_support_common::BspEv
118118
event);
119119
}
120120

121-
void IECStateMachine::feed_state_machine(RawCPState cp_state) {
122-
auto events = state_machine(cp_state);
121+
void IECStateMachine::feed_state_machine(std::optional<RawCPState> cp_state_opt) {
122+
auto events = state_machine(cp_state_opt);
123123

124124
// Process all events
125125
while (not events.empty()) {
126+
EVLOG_debug << "CPEvent " << static_cast<int>(events.front());
126127
signal_event(events.front());
127128
events.pop();
128129
}
@@ -132,8 +133,13 @@ void IECStateMachine::feed_state_machine(RawCPState cp_state) {
132133
// - CP state changes (both events from hardware as well as duty cycle changes)
133134
// - Allow power on changes
134135
// - The C1 6s timer expires
135-
std::queue<CPEvent> IECStateMachine::state_machine(RawCPState cp_state) {
136+
std::queue<CPEvent> IECStateMachine::state_machine(std::optional<RawCPState> cp_state_opt) {
136137

138+
if (cp_state_opt) {
139+
EVLOG_debug << "RawCPState " << static_cast<int>(cp_state_opt.value());
140+
} else {
141+
EVLOG_debug << "RawCPState not set";
142+
}
137143
std::queue<CPEvent> events;
138144
auto timer_state_C1 = TimerControl::do_nothing;
139145
auto timer_unlock_state_F = TimerControl::do_nothing;
@@ -142,6 +148,8 @@ std::queue<CPEvent> IECStateMachine::state_machine(RawCPState cp_state) {
142148
// mutex protected section
143149
Everest::scoped_lock_timeout lock(state_machine_mutex, Everest::MutexDescription::IEC_state_machine);
144150

151+
const auto cp_state = cp_state_opt.value_or(last_cp_state);
152+
145153
if (cp_state not_eq RawCPState::F and last_cp_state == RawCPState::F) {
146154
timer_unlock_state_F = TimerControl::stop;
147155
}
@@ -369,7 +377,7 @@ void IECStateMachine::set_pwm(double value) {
369377

370378
r_bsp->call_pwm_on(value * 100);
371379

372-
feed_state_machine(last_cp_state);
380+
feed_state_machine(std::nullopt);
373381
}
374382

375383
// High level state machine sets state X1
@@ -380,7 +388,7 @@ void IECStateMachine::set_cp_state_X1() {
380388
}
381389
r_bsp->call_cp_state_X1();
382390
// Don't run the state machine in the callers context
383-
feed_state_machine(last_cp_state);
391+
feed_state_machine(std::nullopt);
384392
}
385393

386394
// High level state machine sets state F
@@ -391,7 +399,7 @@ void IECStateMachine::set_cp_state_F() {
391399
}
392400
r_bsp->call_cp_state_F();
393401
// Don't run the state machine in the callers context
394-
feed_state_machine(last_cp_state);
402+
feed_state_machine(std::nullopt);
395403
}
396404

397405
// The higher level state machine in Charger.cpp calls this to indicate it allows contactors to be switched on
@@ -408,7 +416,7 @@ void IECStateMachine::allow_power_on(bool value, types::evse_board_support::Reas
408416
}
409417
// The actual power on will be handled in the state machine to verify it is in the correct CP state etc.
410418
// Don't run the state machine in the callers context
411-
feed_state_machine(last_cp_state);
419+
feed_state_machine(std::nullopt);
412420
}
413421

414422
// Private member function used to actually call the BSP driver's allow_power_on

modules/EVSE/EvseManager/IECStateMachine.hpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424

2525
#include <chrono>
2626
#include <mutex>
27+
#include <optional>
2728
#include <queue>
2829

2930
#include <generated/interfaces/evse_board_support/Interface.hpp>
@@ -131,8 +132,8 @@ class IECStateMachine {
131132
AsyncTimeout timeout_unlock_state_F;
132133

133134
Everest::timed_mutex_traceable state_machine_mutex;
134-
void feed_state_machine(RawCPState cp_state);
135-
std::queue<CPEvent> state_machine(RawCPState cp_state);
135+
void feed_state_machine(std::optional<RawCPState> cp_state_opt);
136+
std::queue<CPEvent> state_machine(std::optional<RawCPState> cp_state_opt);
136137

137138
types::evse_board_support::Reason power_on_reason{types::evse_board_support::Reason::PowerOff};
138139
void call_allow_power_on_bsp(bool value);

0 commit comments

Comments
 (0)