Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tacd: error propagation instead of unwraping part 1 #43

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

tacd: error propagation instead of unwraping part 1 #43

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions src/adc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ impl Adc {

let adc = Self {
usb_host_curr: AdcChannel {
fast: stm32_thread.clone().get_channel("usb-host-curr").unwrap(),
fast: stm32_thread.clone().get_channel("usb-host-curr")?,
topic: bb.topic(
"/v1/usb/host/total/feedback/current",
true,
Expand All @@ -94,7 +94,7 @@ impl Adc {
),
},
usb_host1_curr: AdcChannel {
fast: stm32_thread.clone().get_channel("usb-host1-curr").unwrap(),
fast: stm32_thread.clone().get_channel("usb-host1-curr")?,
topic: bb.topic(
"/v1/usb/host/port1/feedback/current",
true,
Expand All @@ -105,7 +105,7 @@ impl Adc {
),
},
usb_host2_curr: AdcChannel {
fast: stm32_thread.clone().get_channel("usb-host2-curr").unwrap(),
fast: stm32_thread.clone().get_channel("usb-host2-curr")?,
topic: bb.topic(
"/v1/usb/host/port2/feedback/current",
true,
Expand All @@ -116,7 +116,7 @@ impl Adc {
),
},
usb_host3_curr: AdcChannel {
fast: stm32_thread.clone().get_channel("usb-host3-curr").unwrap(),
fast: stm32_thread.clone().get_channel("usb-host3-curr")?,
topic: bb.topic(
"/v1/usb/host/port3/feedback/current",
true,
Expand All @@ -127,7 +127,7 @@ impl Adc {
),
},
out0_volt: AdcChannel {
fast: stm32_thread.clone().get_channel("out0-volt").unwrap(),
fast: stm32_thread.clone().get_channel("out0-volt")?,
topic: bb.topic(
"/v1/output/out_0/feedback/voltage",
true,
Expand All @@ -138,7 +138,7 @@ impl Adc {
),
},
out1_volt: AdcChannel {
fast: stm32_thread.clone().get_channel("out1-volt").unwrap(),
fast: stm32_thread.clone().get_channel("out1-volt")?,
topic: bb.topic(
"/v1/output/out_1/feedback/voltage",
true,
Expand All @@ -149,7 +149,7 @@ impl Adc {
),
},
iobus_curr: AdcChannel {
fast: stm32_thread.clone().get_channel("iobus-curr").unwrap(),
fast: stm32_thread.clone().get_channel("iobus-curr")?,
topic: bb.topic(
"/v1/iobus/feedback/current",
true,
Expand All @@ -160,7 +160,7 @@ impl Adc {
),
},
iobus_volt: AdcChannel {
fast: stm32_thread.clone().get_channel("iobus-volt").unwrap(),
fast: stm32_thread.clone().get_channel("iobus-volt")?,
topic: bb.topic(
"/v1/iobus/feedback/voltage",
true,
Expand All @@ -171,7 +171,7 @@ impl Adc {
),
},
pwr_volt: AdcChannel {
fast: powerboard_thread.clone().get_channel("pwr-volt").unwrap(),
fast: powerboard_thread.clone().get_channel("pwr-volt")?,
topic: bb.topic(
"/v1/dut/feedback/voltage",
true,
Expand All @@ -182,7 +182,7 @@ impl Adc {
),
},
pwr_curr: AdcChannel {
fast: powerboard_thread.get_channel("pwr-curr").unwrap(),
fast: powerboard_thread.get_channel("pwr-curr")?,
topic: bb.topic(
"/v1/dut/feedback/current",
true,
Expand Down
18 changes: 8 additions & 10 deletions src/dbus.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,15 @@
// with this program; if not, write to the Free Software Foundation, Inc.,
// 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.

use anyhow::Result;
use async_std::sync::Arc;

use crate::broker::{BrokerBuilder, Topic};
use crate::led::BlinkPattern;

#[cfg(feature = "demo_mode")]
mod zb {
pub type Result<T> = std::result::Result<T, ()>;
use anyhow::Result;

pub struct Connection;
pub struct ConnectionBuilder;
Expand Down Expand Up @@ -51,7 +52,7 @@ mod zb {
pub use zbus::*;
}

use zb::{Connection, ConnectionBuilder, Result};
use zb::{Connection, ConnectionBuilder};

pub mod networkmanager;
pub mod rauc;
Expand All @@ -76,20 +77,17 @@ impl DbusSession {
bb: &mut BrokerBuilder,
led_dut: Arc<Topic<BlinkPattern>>,
led_uplink: Arc<Topic<BlinkPattern>>,
) -> Self {
) -> Result<Self> {
let tacd = Tacd::new();

let conn_builder = ConnectionBuilder::system()
.unwrap()
.name("de.pengutronix.tacd")
.unwrap();
let conn_builder = ConnectionBuilder::system()?.name("de.pengutronix.tacd")?;

let conn = Arc::new(tacd.serve(conn_builder).build().await.unwrap());
let conn = Arc::new(tacd.serve(conn_builder).build().await?);

Self {
Ok(Self {
network: Network::new(bb, &conn, led_dut, led_uplink),
rauc: Rauc::new(bb, &conn),
systemd: Systemd::new(bb, &conn).await,
}
})
}
}
28 changes: 14 additions & 14 deletions src/digital_io.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
// with this program; if not, write to the Free Software Foundation, Inc.,
// 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.

use anyhow::{Context, Result};
use async_std::prelude::*;
use async_std::sync::Arc;
use async_std::task::spawn;
Expand Down Expand Up @@ -58,43 +59,42 @@ fn handle_line_wo(
initial: bool,
inverted: bool,
led_topic: Option<Arc<Topic<BlinkPattern>>>,
) -> Arc<Topic<bool>> {
) -> Result<Arc<Topic<bool>>> {
let topic = bb.topic_rw(path, Some(initial));
let line = find_line(line_name).unwrap();
let dst = line
.request(LineRequestFlags::OUTPUT, (initial ^ inverted) as _, "tacd")
.unwrap();
let line = find_line(line_name).with_context(|| format!("couldn't find line {line_name}"))?;
let dst = line.request(LineRequestFlags::OUTPUT, (initial ^ inverted) as _, "tacd")?;

let (mut src, _) = topic.clone().subscribe_unbounded();

spawn(async move {
while let Some(ev) = src.next().await {
dst.set_value((ev ^ inverted) as _).unwrap();
dst.set_value((ev ^ inverted) as _)?;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An error from the spawned task propagates nowhere and thus silently ignored.
In my opinion that's worse than crashing the tacd.

We could build a wrapped async_std::task::spawn() that works something like BrokerBuilder in that you pass it around everywhere you want to spawn a task during init and it adds the spawned task to an internal list.
The JoinHandles could then be watched for returned errors in main(), so we can break out of it if something happens.

The usage would be something like:

src/digital_io.rs

…
fn handle_line_wo(
    bb: &mut BrokerBuilder,
    watched: &mut WatchedTaskBuilder,
    …
) -> Result<Arc<Topic<bool>>> {
    …
    watched.spawn(async move {
        …
    });
}
…

src/main.rs

…
#[async_std::main]
async fn main() -> Result<()> {
    let mut watched = WatchedTaskBuilder::new();
    …
    let dig_io = DigitalIo::new(&mut bb, &mut watched, led.out_0.clone(), led.out_1.clone())?;
    …
    watched.run().await
}

That would also give us the opportunity to define tasks that should complete before marking the initialization as successful (i.e. when to notify systemd that we are ready, so that the RAUC slot can be marked good by rauc-mark-good.service) by e.g. spawning them via a special WatchedTaskBuilder::spawn_init() or the likes.

I quite like the idea. Let me mock something up real quick …


if let Some(led) = &led_topic {
let pattern = BlinkPattern::solid(if ev { 1.0 } else { 0.0 });
led.set(pattern);
}
}
anyhow::Ok(())
});

topic
Ok(topic)
}

impl DigitalIo {
pub fn new(
bb: &mut BrokerBuilder,
led_0: Arc<Topic<BlinkPattern>>,
led_1: Arc<Topic<BlinkPattern>>,
) -> Self {
) -> Result<Self> {
let out_0 = handle_line_wo(
bb,
"/v1/output/out_0/asserted",
"OUT_0",
false,
false,
Some(led_0),
);
)?;

let out_1 = handle_line_wo(
bb,
Expand All @@ -103,16 +103,16 @@ impl DigitalIo {
false,
false,
Some(led_1),
);
)?;

let uart_rx_en = handle_line_wo(bb, "/v1/uart/rx/enabled", "UART_RX_EN", true, true, None);
let uart_tx_en = handle_line_wo(bb, "/v1/uart/tx/enabled", "UART_TX_EN", true, true, None);
let uart_rx_en = handle_line_wo(bb, "/v1/uart/rx/enabled", "UART_RX_EN", true, true, None)?;
let uart_tx_en = handle_line_wo(bb, "/v1/uart/tx/enabled", "UART_TX_EN", true, true, None)?;

Self {
Ok(Self {
out_0,
out_1,
uart_rx_en,
uart_tx_en,
}
})
}
}
24 changes: 7 additions & 17 deletions src/digital_io/gpio/demo_mode.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,33 +27,23 @@ pub struct LineHandle {
}

impl LineHandle {
pub fn set_value(&self, val: u8) -> Result<(), ()> {
pub fn set_value(&self, val: u8) -> Result<()> {
// This does not actually set up any IIO things.
// It is just a hack to let adc/iio/demo_mode.rs
// communicate with this function so that toggling an output
// has an effect on the measured values.
let iio_thread_stm32 = block_on(IioThread::new_stm32()).unwrap();
let iio_thread_pwr = block_on(IioThread::new_powerboard()).unwrap();
let iio_thread_stm32 = block_on(IioThread::new_stm32())?;
let iio_thread_pwr = block_on(IioThread::new_powerboard())?;

match self.name.as_str() {
"OUT_0" => iio_thread_stm32
.get_channel("out0-volt")
.unwrap()
.set(val != 0),
"OUT_1" => iio_thread_stm32
.get_channel("out1-volt")
.unwrap()
.set(val != 0),
"OUT_0" => iio_thread_stm32.get_channel("out0-volt")?.set(val != 0),
"OUT_1" => iio_thread_stm32.get_channel("out1-volt")?.set(val != 0),
"DUT_PWR_EN" => {
iio_thread_pwr
.clone()
.get_channel("pwr-curr")
.unwrap()
.set(val == 0);
iio_thread_pwr
.get_channel("pwr-volt")
.unwrap()
.get_channel("pwr-curr")?
.set(val == 0);
iio_thread_pwr.get_channel("pwr-volt")?.set(val == 0);
}
_ => {}
}
Expand Down
2 changes: 1 addition & 1 deletion src/digital_io/gpio/test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ pub struct LineHandle {
}

impl LineHandle {
pub fn set_value(&self, val: u8) -> Result<(), ()> {
pub fn set_value(&self, val: u8) -> Result<()> {
println!("GPIO simulation set {} to {}", self.name, val);
self.val.store(val, Ordering::Relaxed);
Ok(())
Expand Down
Loading