Skip to content

Commit dfad2e3

Browse files
committed
ucsi/ppm/state_machine: Fix handling of reset in Idle(true)
The position of the reset catch all meant that resets in the `Idle(true)` state weren't processed correctly. Fix this and a similar issue that could happen with ACK_CC_CI. This commit also adds tests around processing commands that need special handling by the state machine.
1 parent c3ad493 commit dfad2e3

File tree

1 file changed

+218
-5
lines changed

1 file changed

+218
-5
lines changed

src/ucsi/ppm/state_machine.rs

Lines changed: 218 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ pub type GlobalOutput<'a> = Output<'a, GlobalPortId>;
6262
pub type LocalOutput<'a> = Output<'a, LocalPortId>;
6363

6464
/// Attempted transition that is not allowed by the state machine
65-
#[derive(Copy, Clone, Debug)]
65+
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
6666
#[cfg_attr(feature = "defmt", derive(defmt::Format))]
6767
pub struct InvalidTransition<'a, T: PortId> {
6868
/// The current state of the state machine
@@ -102,6 +102,9 @@ impl<T: PortId> StateMachine<T> {
102102
use State::*;
103103

104104
let (next_state, output) = match (self.state, input) {
105+
// Reset transitions
106+
(_, Command(ucsi::Command::PpmCommand(ucsi::ppm::Command::PpmReset))) => (Idle(false), Some(ResetComplete)),
107+
105108
// Idle(false) transitions
106109
(Idle(false), Command(cmd @ ucsi::Command::PpmCommand(ucsi::ppm::Command::SetNotificationEnable(_)))) => {
107110
(ProcessingCommand, Some(ExecuteCommand(cmd)))
@@ -113,8 +116,19 @@ impl<T: PortId> StateMachine<T> {
113116
(Busy(false), CommandComplete) => (Busy(false), None),
114117
(Busy(true), CommandComplete) => (Busy(true), Some(OpmNotifyBusy)),
115118

116-
// Idle(true) transitions
119+
// Idle(true) successful transitions
117120
(Idle(true), BusyChanged) => (Busy(true), None),
121+
(Idle(true), Command(cmd @ ucsi::Command::PpmCommand(ucsi::ppm::Command::AckCcCi(args)))) => {
122+
if args.ack.command_complete() {
123+
// This should only happen in WaitForCommandCompleteAck
124+
return Err(InvalidTransition {
125+
state: self.state,
126+
input,
127+
});
128+
} else {
129+
(ProcessingCommand, Some(ExecuteCommand(cmd)))
130+
}
131+
}
118132
(Idle(true), Command(cmd)) => (ProcessingCommand, Some(ExecuteCommand(cmd))),
119133

120134
// ProcessingCommand transitions
@@ -134,9 +148,6 @@ impl<T: PortId> StateMachine<T> {
134148
}
135149
}
136150

137-
// Reset transitions
138-
(_, Command(ucsi::Command::PpmCommand(ucsi::ppm::Command::PpmReset))) => (Idle(false), Some(ResetComplete)),
139-
140151
// Invalid transition
141152
_ => {
142153
return Err(InvalidTransition {
@@ -159,3 +170,205 @@ impl<T: PortId> Default for StateMachine<T> {
159170

160171
pub type GlobalStateMachine = StateMachine<GlobalPortId>;
161172
pub type LocalStateMachine = StateMachine<LocalPortId>;
173+
174+
#[cfg(test)]
175+
mod test {
176+
use super::*;
177+
use crate::ucsi::{lpm, ppm, Command};
178+
179+
#[test]
180+
fn test_reset_all() {
181+
let mut sm = GlobalStateMachine::new();
182+
183+
// Test reset from all states
184+
sm.state = State::Idle(false);
185+
let res = sm.consume(Input::Command(&Command::PpmCommand(ppm::Command::PpmReset)));
186+
assert_eq!(res, Ok(Some(Output::ResetComplete)));
187+
assert_eq!(sm.state(), State::Idle(false));
188+
189+
// Test reset from Idle(true)
190+
sm.state = State::Idle(true);
191+
let res = sm.consume(Input::Command(&Command::PpmCommand(ppm::Command::PpmReset)));
192+
assert_eq!(res, Ok(Some(Output::ResetComplete)));
193+
assert_eq!(sm.state(), State::Idle(false));
194+
195+
// Test reset from Busy(false)
196+
sm.state = State::Busy(false);
197+
let res = sm.consume(Input::Command(&Command::PpmCommand(ppm::Command::PpmReset)));
198+
assert_eq!(res, Ok(Some(Output::ResetComplete)));
199+
assert_eq!(sm.state(), State::Idle(false));
200+
201+
// Test reset from Busy(true)
202+
sm.state = State::Busy(true);
203+
let res = sm.consume(Input::Command(&Command::PpmCommand(ppm::Command::PpmReset)));
204+
assert_eq!(res, Ok(Some(Output::ResetComplete)));
205+
assert_eq!(sm.state(), State::Idle(false));
206+
207+
// Test reset from ProcessingCommand
208+
sm.state = State::ProcessingCommand;
209+
let res = sm.consume(Input::Command(&Command::PpmCommand(ppm::Command::PpmReset)));
210+
assert_eq!(res, Ok(Some(Output::ResetComplete)));
211+
assert_eq!(sm.state(), State::Idle(false));
212+
213+
// Test reset from WaitForCommandCompleteAck
214+
sm.state = State::WaitForCommandCompleteAck;
215+
let res = sm.consume(Input::Command(&Command::PpmCommand(ppm::Command::PpmReset)));
216+
assert_eq!(res, Ok(Some(Output::ResetComplete)));
217+
assert_eq!(sm.state(), State::Idle(false));
218+
}
219+
220+
/// Test that only SET_NOTIFICATION_ENABLE works from Idle(false)
221+
#[test]
222+
fn test_idle_false_commands() {
223+
let mut sm = GlobalStateMachine::new();
224+
sm.state = State::Idle(false);
225+
226+
// Valid command
227+
let cmd = Command::PpmCommand(ppm::Command::SetNotificationEnable(
228+
ucsi::ppm::set_notification_enable::Args::default(),
229+
));
230+
let res = sm.consume(Input::Command(&cmd));
231+
assert_eq!(res, Ok(Some(Output::ExecuteCommand(&cmd))));
232+
assert_eq!(sm.state(), State::ProcessingCommand);
233+
234+
// Invalid PPM command
235+
sm.state = State::Idle(false);
236+
let cmd = Command::PpmCommand(ppm::Command::AckCcCi(ucsi::ppm::ack_cc_ci::Args::default()));
237+
let res = sm.consume(Input::Command(&cmd));
238+
assert_eq!(
239+
res,
240+
Err(InvalidTransition {
241+
state: State::Idle(false),
242+
input: Input::Command(&cmd)
243+
})
244+
);
245+
assert_eq!(sm.state(), State::Idle(false));
246+
247+
// Invalid LPM command
248+
sm.state = State::Idle(false);
249+
let cmd = Command::LpmCommand(ucsi::lpm::Command::new(
250+
GlobalPortId(0),
251+
lpm::CommandData::GetPdos(lpm::get_pdos::Args::default()),
252+
));
253+
let res = sm.consume(Input::Command(&cmd));
254+
assert_eq!(
255+
res,
256+
Err(InvalidTransition {
257+
state: State::Idle(false),
258+
input: Input::Command(&cmd)
259+
})
260+
);
261+
assert_eq!(sm.state(), State::Idle(false));
262+
}
263+
264+
/// Test that cancel works while processing a command, but no other commands are accepted
265+
#[test]
266+
fn test_processing_commands() {
267+
let mut sm = GlobalStateMachine::new();
268+
sm.state = State::ProcessingCommand;
269+
270+
// State machine is already in processing command state, should fail
271+
let cmd = Command::LpmCommand(lpm::Command::new(
272+
GlobalPortId(0),
273+
lpm::CommandData::GetPdos(lpm::get_pdos::Args::default()),
274+
));
275+
let res = sm.consume(Input::Command(&cmd));
276+
assert_eq!(
277+
res,
278+
Err(InvalidTransition {
279+
state: State::ProcessingCommand,
280+
input: Input::Command(&cmd)
281+
})
282+
);
283+
assert_eq!(sm.state(), State::ProcessingCommand);
284+
285+
let res = sm.consume(Input::Command(&Command::PpmCommand(ppm::Command::Cancel)));
286+
assert_eq!(res, Ok(Some(Output::OpmNotifyCommandComplete)));
287+
assert_eq!(sm.state(), State::WaitForCommandCompleteAck)
288+
}
289+
290+
/// Test idle true command transitions
291+
#[test]
292+
fn test_idle_true_commands() {
293+
let mut sm = GlobalStateMachine::new();
294+
sm.state = State::Idle(true);
295+
296+
// Test simple command execution
297+
let cmd = Command::LpmCommand(lpm::Command::new(
298+
GlobalPortId(0),
299+
lpm::CommandData::GetPdos(lpm::get_pdos::Args::default()),
300+
));
301+
let res = sm.consume(Input::Command(&cmd));
302+
assert_eq!(res, Ok(Some(Output::ExecuteCommand(&cmd))));
303+
assert_eq!(sm.state(), State::ProcessingCommand);
304+
305+
// Test rejection of command completion ACK
306+
sm.state = State::Idle(true);
307+
let cmd = Command::PpmCommand(ppm::Command::AckCcCi(ppm::ack_cc_ci::Args {
308+
ack: *ppm::ack_cc_ci::Ack::default().set_command_complete(true),
309+
}));
310+
let res = sm.consume(Input::Command(&cmd));
311+
assert_eq!(
312+
res,
313+
Err(InvalidTransition {
314+
state: State::Idle(true),
315+
input: Input::Command(&cmd)
316+
})
317+
);
318+
assert_eq!(sm.state(), State::Idle(true));
319+
320+
// Test acceptance of connector change ACK
321+
sm.state = State::Idle(true);
322+
let cmd = Command::PpmCommand(ppm::Command::AckCcCi(ppm::ack_cc_ci::Args {
323+
ack: *ppm::ack_cc_ci::Ack::default().set_connector_change(true),
324+
}));
325+
let res = sm.consume(Input::Command(&cmd));
326+
assert_eq!(res, Ok(Some(Output::ExecuteCommand(&cmd))));
327+
assert_eq!(sm.state(), State::ProcessingCommand);
328+
}
329+
330+
/// Test wait for command complete command transitions
331+
#[test]
332+
fn test_wait_for_command_complete_ack_commands() {
333+
let mut sm = GlobalStateMachine::new();
334+
sm.state = State::WaitForCommandCompleteAck;
335+
336+
// Command complete ACK should succeed
337+
let ack = *ppm::ack_cc_ci::Ack::default().set_command_complete(true);
338+
let cmd = Command::PpmCommand(ppm::Command::AckCcCi(ppm::ack_cc_ci::Args { ack }));
339+
let res = sm.consume(Input::Command(&cmd));
340+
assert_eq!(res, Ok(Some(Output::AckComplete(ack))));
341+
assert_eq!(sm.state(), State::Idle(true));
342+
343+
// Connector change ACK only should fail
344+
sm.state = State::WaitForCommandCompleteAck;
345+
let cmd = Command::PpmCommand(ppm::Command::AckCcCi(ppm::ack_cc_ci::Args {
346+
ack: *ppm::ack_cc_ci::Ack::default().set_connector_change(true),
347+
}));
348+
let res = sm.consume(Input::Command(&cmd));
349+
assert_eq!(
350+
res,
351+
Err(InvalidTransition {
352+
state: State::WaitForCommandCompleteAck,
353+
input: Input::Command(&cmd)
354+
})
355+
);
356+
assert_eq!(sm.state(), State::WaitForCommandCompleteAck);
357+
358+
// All other commands should fail as well
359+
sm.state = State::WaitForCommandCompleteAck;
360+
let cmd = Command::LpmCommand(lpm::Command::new(
361+
GlobalPortId(0),
362+
lpm::CommandData::GetPdos(lpm::get_pdos::Args::default()),
363+
));
364+
let res = sm.consume(Input::Command(&cmd));
365+
assert_eq!(
366+
res,
367+
Err(InvalidTransition {
368+
state: State::WaitForCommandCompleteAck,
369+
input: Input::Command(&cmd)
370+
})
371+
);
372+
assert_eq!(sm.state(), State::WaitForCommandCompleteAck);
373+
}
374+
}

0 commit comments

Comments
 (0)