Skip to content

Commit 95f5a15

Browse files
committed
dut_power: improve/simplify error handling
Now that errors from threads are properly propagated we can simplify the error handling. Signed-off-by: Leonard Göhrs <l.goehrs@pengutronix.de>
1 parent 3114a63 commit 95f5a15

File tree

3 files changed

+35
-38
lines changed

3 files changed

+35
-38
lines changed

src/digital_io/gpio/demo_mode.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ pub struct LineHandle {
2727
}
2828

2929
impl LineHandle {
30-
pub fn set_value(&self, val: u8) -> Result<(), ()> {
30+
pub fn set_value(&self, val: u8) -> Result<()> {
3131
// This does not actually set up any IIO things.
3232
// It is just a hack to let adc/iio/demo_mode.rs
3333
// communicate with this function so that toggling an output

src/digital_io/gpio/test.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ pub struct LineHandle {
3333
}
3434

3535
impl LineHandle {
36-
pub fn set_value(&self, val: u8) -> Result<(), ()> {
36+
pub fn set_value(&self, val: u8) -> Result<()> {
3737
println!("GPIO simulation set {} to {}", self.name, val);
3838
self.val.store(val, Ordering::Relaxed);
3939
Ok(())

src/dut_power.rs

Lines changed: 33 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,12 @@ mod prio {
4949
use thread_priority::*;
5050

5151
pub fn realtime_priority() -> Result<()> {
52+
let prio = ThreadPriorityValue::try_from(10)
53+
.map_err(|e| anyhow!("Failed to choose realtime priority level 10: {e:?}"))?;
54+
5255
set_thread_priority_and_policy(
5356
thread_native_id(),
54-
ThreadPriority::Crossplatform(ThreadPriorityValue::try_from(10).unwrap()),
57+
ThreadPriority::Crossplatform(prio),
5558
ThreadSchedulePolicy::Realtime(RealtimeThreadSchedulePolicy::Fifo),
5659
)
5760
.map_err(|e| anyhow!("Failed to set up realtime priority {e:?}"))
@@ -237,10 +240,12 @@ fn turn_off_with_reason(
237240
pwr_line: &LineHandle,
238241
discharge_line: &LineHandle,
239242
fail_state: &AtomicU8,
240-
) {
241-
pwr_line.set_value(1 - PWR_LINE_ASSERTED).unwrap();
242-
discharge_line.set_value(DISCHARGE_LINE_ASSERTED).unwrap();
243+
) -> Result<()> {
244+
pwr_line.set_value(1 - PWR_LINE_ASSERTED)?;
245+
discharge_line.set_value(DISCHARGE_LINE_ASSERTED)?;
243246
fail_state.store(reason as u8, Ordering::Relaxed);
247+
248+
Ok(())
244249
}
245250

246251
/// Labgrid has a fixed assumption of how a REST based power port should work.
@@ -318,7 +323,7 @@ impl DutPwrThread {
318323
// as well.
319324
// Use a queue to notify the calling thread if the priority setup
320325
// succeeded.
321-
let (thread_res_tx, mut thread_res_rx) = bounded(1);
326+
let (thread_tx, thread_rx) = bounded(1);
322327

323328
// Spawn a high priority thread that handles the power status
324329
// in a realtimey fashion.
@@ -333,24 +338,20 @@ impl DutPwrThread {
333338
let mut volt_filter = MedianFilter::<4>::new();
334339
let mut curr_filter = MedianFilter::<4>::new();
335340

336-
let (tick_weak, request, state) = match realtime_priority() {
337-
Ok(_) => {
338-
let tick = Arc::new(AtomicU32::new(0));
339-
let tick_weak = Arc::downgrade(&tick);
341+
realtime_priority()?;
340342

341-
let request = Arc::new(AtomicU8::new(OutputRequest::Idle as u8));
342-
let state = Arc::new(AtomicU8::new(OutputState::Off as u8));
343+
let (tick_weak, request, state) = {
344+
let tick = Arc::new(AtomicU32::new(0));
345+
let tick_weak = Arc::downgrade(&tick);
343346

344-
thread_res_tx
345-
.try_send(Ok((tick, request.clone(), state.clone())))
346-
.unwrap();
347+
let request = Arc::new(AtomicU8::new(OutputRequest::Idle as u8));
348+
let state = Arc::new(AtomicU8::new(OutputState::Off as u8));
347349

348-
(tick_weak, request, state)
349-
}
350-
Err(e) => {
351-
thread_res_tx.try_send(Err(e)).unwrap();
352-
panic!()
353-
}
350+
thread_tx
351+
.try_send((tick, request.clone(), state.clone()))
352+
.expect("Queue that should be empty wasn't");
353+
354+
(tick_weak, request, state)
354355
};
355356

356357
// Run as long as there is a strong reference to `tick`.
@@ -385,7 +386,7 @@ impl DutPwrThread {
385386
&pwr_line,
386387
&discharge_line,
387388
&state,
388-
);
389+
)?;
389390
} else {
390391
// We have a fresh ADC value. Signal "everything is well"
391392
// to the watchdog task.
@@ -422,7 +423,7 @@ impl DutPwrThread {
422423
&pwr_line,
423424
&discharge_line,
424425
&state,
425-
);
426+
)?;
426427

427428
continue;
428429
}
@@ -436,7 +437,7 @@ impl DutPwrThread {
436437
&pwr_line,
437438
&discharge_line,
438439
&state,
439-
);
440+
)?;
440441

441442
continue;
442443
}
@@ -449,7 +450,7 @@ impl DutPwrThread {
449450
&pwr_line,
450451
&discharge_line,
451452
&state,
452-
);
453+
)?;
453454

454455
continue;
455456
}
@@ -459,34 +460,30 @@ impl DutPwrThread {
459460
match req {
460461
OutputRequest::Idle => {}
461462
OutputRequest::On => {
462-
discharge_line
463-
.set_value(1 - DISCHARGE_LINE_ASSERTED)
464-
.unwrap();
465-
pwr_line.set_value(PWR_LINE_ASSERTED).unwrap();
463+
discharge_line.set_value(1 - DISCHARGE_LINE_ASSERTED)?;
464+
pwr_line.set_value(PWR_LINE_ASSERTED)?;
466465
state.store(OutputState::On as u8, Ordering::Relaxed);
467466
}
468467
OutputRequest::Off => {
469-
discharge_line.set_value(DISCHARGE_LINE_ASSERTED).unwrap();
470-
pwr_line.set_value(1 - PWR_LINE_ASSERTED).unwrap();
468+
discharge_line.set_value(DISCHARGE_LINE_ASSERTED)?;
469+
pwr_line.set_value(1 - PWR_LINE_ASSERTED)?;
471470
state.store(OutputState::Off as u8, Ordering::Relaxed);
472471
}
473472
OutputRequest::OffFloating => {
474-
discharge_line
475-
.set_value(1 - DISCHARGE_LINE_ASSERTED)
476-
.unwrap();
477-
pwr_line.set_value(1 - PWR_LINE_ASSERTED).unwrap();
473+
discharge_line.set_value(1 - DISCHARGE_LINE_ASSERTED)?;
474+
pwr_line.set_value(1 - PWR_LINE_ASSERTED)?;
478475
state.store(OutputState::OffFloating as u8, Ordering::Relaxed);
479476
}
480477
}
481478
}
482479

483480
// Make sure to enter fail safe mode before leaving the thread
484-
turn_off_with_reason(OutputState::Off, &pwr_line, &discharge_line, &state);
481+
turn_off_with_reason(OutputState::Off, &pwr_line, &discharge_line, &state)?;
485482

486483
Ok(())
487484
})?;
488485

489-
let (tick, request, state) = thread_res_rx.next().await.unwrap()?;
486+
let (tick, request, state) = thread_rx.recv().await?;
490487

491488
// The request and state topic use the same external path, this way one
492489
// can e.g. publish "On" to the topic and be sure that the output is

0 commit comments

Comments
 (0)