Skip to content

Commit 0e16a4e

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 8b0afc5 commit 0e16a4e

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.
@@ -316,7 +321,7 @@ impl DutPwrThread {
316321
// as well.
317322
// Use a queue to notify the calling thread if the priority setup
318323
// succeeded.
319-
let (thread_res_tx, mut thread_res_rx) = bounded(1);
324+
let (thread_tx, thread_rx) = bounded(1);
320325

321326
// Spawn a high priority thread that handles the power status
322327
// in a realtimey fashion.
@@ -331,24 +336,20 @@ impl DutPwrThread {
331336
let mut volt_filter = MedianFilter::<4>::new();
332337
let mut curr_filter = MedianFilter::<4>::new();
333338

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

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

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

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

354355
// Run as long as there is a strong reference to `tick`.
@@ -383,7 +384,7 @@ impl DutPwrThread {
383384
&pwr_line,
384385
&discharge_line,
385386
&state,
386-
);
387+
)?;
387388
} else {
388389
// We have a fresh ADC value. Signal "everything is well"
389390
// to the watchdog task.
@@ -420,7 +421,7 @@ impl DutPwrThread {
420421
&pwr_line,
421422
&discharge_line,
422423
&state,
423-
);
424+
)?;
424425

425426
continue;
426427
}
@@ -434,7 +435,7 @@ impl DutPwrThread {
434435
&pwr_line,
435436
&discharge_line,
436437
&state,
437-
);
438+
)?;
438439

439440
continue;
440441
}
@@ -447,7 +448,7 @@ impl DutPwrThread {
447448
&pwr_line,
448449
&discharge_line,
449450
&state,
450-
);
451+
)?;
451452

452453
continue;
453454
}
@@ -457,34 +458,30 @@ impl DutPwrThread {
457458
match req {
458459
OutputRequest::Idle => {}
459460
OutputRequest::On => {
460-
discharge_line
461-
.set_value(1 - DISCHARGE_LINE_ASSERTED)
462-
.unwrap();
463-
pwr_line.set_value(PWR_LINE_ASSERTED).unwrap();
461+
discharge_line.set_value(1 - DISCHARGE_LINE_ASSERTED)?;
462+
pwr_line.set_value(PWR_LINE_ASSERTED)?;
464463
state.store(OutputState::On as u8, Ordering::Relaxed);
465464
}
466465
OutputRequest::Off => {
467-
discharge_line.set_value(DISCHARGE_LINE_ASSERTED).unwrap();
468-
pwr_line.set_value(1 - PWR_LINE_ASSERTED).unwrap();
466+
discharge_line.set_value(DISCHARGE_LINE_ASSERTED)?;
467+
pwr_line.set_value(1 - PWR_LINE_ASSERTED)?;
469468
state.store(OutputState::Off as u8, Ordering::Relaxed);
470469
}
471470
OutputRequest::OffFloating => {
472-
discharge_line
473-
.set_value(1 - DISCHARGE_LINE_ASSERTED)
474-
.unwrap();
475-
pwr_line.set_value(1 - PWR_LINE_ASSERTED).unwrap();
471+
discharge_line.set_value(1 - DISCHARGE_LINE_ASSERTED)?;
472+
pwr_line.set_value(1 - PWR_LINE_ASSERTED)?;
476473
state.store(OutputState::OffFloating as u8, Ordering::Relaxed);
477474
}
478475
}
479476
}
480477

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

484481
Ok(())
485482
})?;
486483

487-
let (tick, request, state) = thread_res_rx.next().await.unwrap()?;
484+
let (tick, request, state) = thread_rx.recv().await?;
488485

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

0 commit comments

Comments
 (0)