From cf7bd771d5d9d6b2f68ecaaf700e8416c811b7d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Leonard=20G=C3=B6hrs?= Date: Mon, 2 Oct 2023 14:19:38 +0200 Subject: [PATCH 1/8] watched_tasks: spawn long running tasks via special task builder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The WatchedTasksBuilder makes sure that the program ends if any of the tasks fails and also handles error propagation from the tasks to the main() function. Signed-off-by: Leonard Göhrs --- src/adc.rs | 7 +- src/backlight.rs | 8 +- src/broker.rs | 6 +- src/broker/persistence.rs | 7 +- src/dbus.rs | 10 +- src/dbus/hostname.rs | 17 ++- src/dbus/networkmanager.rs | 21 ++-- src/dbus/rauc.rs | 70 ++++++++---- src/dbus/systemd.rs | 63 ++++++++--- src/digital_io.rs | 30 +++++- src/dut_power.rs | 28 +++-- src/http_server.rs | 9 +- src/iobus.rs | 6 +- src/led.rs | 30 +++--- src/main.rs | 66 ++++++------ src/regulators.rs | 13 ++- src/setup_mode.rs | 16 ++- src/ui.rs | 29 +++-- src/ui/screens.rs | 25 +++-- src/ui/screens/help.rs | 12 ++- src/ui/screens/iobus_health.rs | 12 ++- src/ui/screens/locator.rs | 12 ++- src/ui/screens/overtemperature.rs | 12 ++- src/ui/screens/power_fail.rs | 12 ++- src/ui/screens/reboot.rs | 7 +- src/ui/screens/screensaver.rs | 12 ++- src/ui/screens/setup.rs | 11 +- src/ui/screens/update_available.rs | 12 ++- src/ui/screens/update_installation.rs | 11 +- src/ui/screens/usb_overload.rs | 7 +- src/usb_hub.rs | 28 +++-- src/watchdog.rs | 26 ++--- src/watched_tasks.rs | 149 ++++++++++++++++++++++++++ 33 files changed, 577 insertions(+), 207 deletions(-) create mode 100644 src/watched_tasks.rs diff --git a/src/adc.rs b/src/adc.rs index f2c79247..1b31b397 100644 --- a/src/adc.rs +++ b/src/adc.rs @@ -19,10 +19,11 @@ use std::time::Duration; use anyhow::Result; use async_std::sync::Arc; -use async_std::task::{sleep, spawn}; +use async_std::task::sleep; use crate::broker::{BrokerBuilder, Topic}; use crate::measurement::{Measurement, Timestamp}; +use crate::watched_tasks::WatchedTasksBuilder; const HISTORY_LENGTH: usize = 200; const SLOW_INTERVAL: Duration = Duration::from_millis(100); @@ -77,7 +78,7 @@ pub struct Adc { } impl Adc { - pub async fn new(bb: &mut BrokerBuilder) -> Result { + pub async fn new(bb: &mut BrokerBuilder, wtb: &mut WatchedTasksBuilder) -> Result { let stm32_thread = IioThread::new_stm32().await?; let powerboard_thread = IioThread::new_powerboard().await?; @@ -212,7 +213,7 @@ impl Adc { // Spawn an async task to transfer values from the Atomic value based // "fast" interface to the broker based "slow" interface. - spawn(async move { + wtb.spawn_task("adc-update", async move { loop { sleep(SLOW_INTERVAL).await; diff --git a/src/backlight.rs b/src/backlight.rs index 50595166..57c0ff29 100644 --- a/src/backlight.rs +++ b/src/backlight.rs @@ -18,7 +18,6 @@ use anyhow::Result; use async_std::prelude::*; use async_std::sync::Arc; -use async_std::task::spawn; use log::warn; mod demo_mode; @@ -30,13 +29,14 @@ use demo_mode::{Backlight as SysBacklight, Brightness, SysClass}; use sysfs_class::{Backlight as SysBacklight, Brightness, SysClass}; use crate::broker::{BrokerBuilder, Topic}; +use crate::watched_tasks::WatchedTasksBuilder; pub struct Backlight { pub brightness: Arc>, } impl Backlight { - pub fn new(bb: &mut BrokerBuilder) -> Result { + pub fn new(bb: &mut BrokerBuilder, wtb: &mut WatchedTasksBuilder) -> Result { let brightness = bb.topic_rw("/v1/tac/display/backlight/brightness", Some(1.0)); let (mut rx, _) = brightness.clone().subscribe_unbounded(); @@ -44,7 +44,7 @@ impl Backlight { let backlight = SysBacklight::new("backlight")?; let max_brightness = backlight.max_brightness()?; - spawn(async move { + wtb.spawn_task("backlight-dimmer", async move { while let Some(fraction) = rx.next().await { let brightness = (max_brightness as f32) * fraction; let mut brightness = brightness.clamp(0.0, max_brightness as f32) as u64; @@ -60,6 +60,8 @@ impl Backlight { warn!("Failed to set LED pattern: {}", e); } } + + Ok(()) }); Ok(Self { brightness }) diff --git a/src/broker.rs b/src/broker.rs index f51ced22..b68ca90a 100644 --- a/src/broker.rs +++ b/src/broker.rs @@ -18,6 +18,8 @@ use async_std::sync::Arc; use serde::{de::DeserializeOwned, Serialize}; +use crate::watched_tasks::WatchedTasksBuilder; + mod mqtt_conn; mod persistence; mod rest; @@ -113,10 +115,10 @@ impl BrokerBuilder { /// Finish building the broker /// /// This consumes the builder so that no new topics can be registered - pub fn build(self, server: &mut tide::Server<()>) { + pub fn build(self, wtb: &mut WatchedTasksBuilder, server: &mut tide::Server<()>) { let topics = Arc::new(self.topics); - persistence::register(topics.clone()); + persistence::register(wtb, topics.clone()); rest::register(server, topics.clone()); mqtt_conn::register(server, topics); } diff --git a/src/broker/persistence.rs b/src/broker/persistence.rs index efcd52ca..3f1543b1 100644 --- a/src/broker/persistence.rs +++ b/src/broker/persistence.rs @@ -22,13 +22,14 @@ use anyhow::{bail, Result}; use async_std::channel::{unbounded, Receiver}; use async_std::prelude::*; use async_std::sync::Arc; -use async_std::task::spawn; use log::{error, info}; use serde::{Deserialize, Serialize}; use serde_json::{from_reader, to_writer_pretty, Map, Value}; use super::{AnyTopic, TopicName}; +use crate::watched_tasks::WatchedTasksBuilder; + #[cfg(feature = "demo_mode")] const PERSISTENCE_PATH: &str = "demo_files/srv/tacd/state.json"; @@ -147,7 +148,7 @@ async fn save_on_change( Ok(()) } -pub fn register(topics: Arc>>) { +pub fn register(wtb: &mut WatchedTasksBuilder, topics: Arc>>) { load(&topics).unwrap(); let (tx, rx) = unbounded(); @@ -156,5 +157,5 @@ pub fn register(topics: Arc>>) { topic.subscribe_as_bytes(tx.clone(), false); } - spawn(async move { save_on_change(topics, rx).await.unwrap() }); + wtb.spawn_task("persistence-save", save_on_change(topics, rx)); } diff --git a/src/dbus.rs b/src/dbus.rs index a8d34e30..393a74ec 100644 --- a/src/dbus.rs +++ b/src/dbus.rs @@ -19,6 +19,7 @@ use async_std::sync::Arc; use crate::broker::{BrokerBuilder, Topic}; use crate::led::BlinkPattern; +use crate::watched_tasks::WatchedTasksBuilder; #[cfg(feature = "demo_mode")] mod zb { @@ -78,6 +79,7 @@ pub struct DbusSession { impl DbusSession { pub async fn new( bb: &mut BrokerBuilder, + wtb: &mut WatchedTasksBuilder, led_dut: Arc>, led_uplink: Arc>, ) -> Self { @@ -91,10 +93,10 @@ impl DbusSession { let conn = Arc::new(tacd.serve(conn_builder).build().await.unwrap()); Self { - hostname: Hostname::new(bb, &conn), - network: Network::new(bb, &conn, led_dut, led_uplink), - rauc: Rauc::new(bb, &conn), - systemd: Systemd::new(bb, &conn).await, + hostname: Hostname::new(bb, wtb, &conn), + network: Network::new(bb, wtb, &conn, led_dut, led_uplink), + rauc: Rauc::new(bb, wtb, &conn), + systemd: Systemd::new(bb, wtb, &conn).await, } } } diff --git a/src/dbus/hostname.rs b/src/dbus/hostname.rs index aba866d0..e51e257d 100644 --- a/src/dbus/hostname.rs +++ b/src/dbus/hostname.rs @@ -21,9 +21,11 @@ use async_std::stream::StreamExt; #[cfg(not(feature = "demo_mode"))] use zbus::Connection; -use crate::broker::{BrokerBuilder, Topic}; use async_std::sync::Arc; +use crate::broker::{BrokerBuilder, Topic}; +use crate::watched_tasks::WatchedTasksBuilder; + mod hostnamed; pub struct Hostname { @@ -32,19 +34,24 @@ pub struct Hostname { impl Hostname { #[cfg(feature = "demo_mode")] - pub fn new(bb: &mut BrokerBuilder, _conn: C) -> Self { + pub fn new(bb: &mut BrokerBuilder, _wtb: &mut WatchedTasksBuilder, _conn: C) -> Self { Self { hostname: bb.topic_ro("/v1/tac/network/hostname", Some("lxatac".into())), } } #[cfg(not(feature = "demo_mode"))] - pub fn new(bb: &mut BrokerBuilder, conn: &Arc) -> Self { + pub fn new( + bb: &mut BrokerBuilder, + wtb: &mut WatchedTasksBuilder, + conn: &Arc, + ) -> Self { let hostname = bb.topic_ro("/v1/tac/network/hostname", None); let conn = conn.clone(); let hostname_topic = hostname.clone(); - async_std::task::spawn(async move { + + wtb.spawn_task("hostname-update", async move { let proxy = hostnamed::HostnameProxy::new(&conn).await.unwrap(); let mut stream = proxy.receive_hostname_changed().await; @@ -58,6 +65,8 @@ impl Hostname { hostname_topic.set(h); } } + + Ok(()) }); Self { hostname } diff --git a/src/dbus/networkmanager.rs b/src/dbus/networkmanager.rs index e13849b3..ee97a933 100644 --- a/src/dbus/networkmanager.rs +++ b/src/dbus/networkmanager.rs @@ -22,6 +22,7 @@ use serde::{Deserialize, Serialize}; use crate::broker::{BrokerBuilder, Topic}; use crate::led::BlinkPattern; +use crate::watched_tasks::WatchedTasksBuilder; // Macro use makes these modules quite heavy, so we keep them commented // out until they are actually used @@ -244,6 +245,7 @@ impl Network { #[cfg(feature = "demo_mode")] pub fn new( bb: &mut BrokerBuilder, + _wtb: &mut WatchedTasksBuilder, _conn: C, _led_dut: Arc>, _led_uplink: Arc>, @@ -266,6 +268,7 @@ impl Network { #[cfg(not(feature = "demo_mode"))] pub fn new( bb: &mut BrokerBuilder, + wtb: &mut WatchedTasksBuilder, conn: &Arc, led_dut: Arc>, led_uplink: Arc>, @@ -274,26 +277,20 @@ impl Network { let conn_task = conn.clone(); let dut_interface = this.dut_interface.clone(); - async_std::task::spawn(async move { - handle_link_updates(&conn_task, dut_interface, "dut", led_dut) - .await - .unwrap(); + wtb.spawn_task("link-dut-update", async move { + handle_link_updates(&conn_task, dut_interface, "dut", led_dut).await }); let conn_task = conn.clone(); let uplink_interface = this.uplink_interface.clone(); - async_std::task::spawn(async move { - handle_link_updates(&conn_task, uplink_interface, "uplink", led_uplink) - .await - .unwrap(); + wtb.spawn_task("link-uplink-update", async move { + handle_link_updates(&conn_task, uplink_interface, "uplink", led_uplink).await }); let conn_task = conn.clone(); let bridge_interface = this.bridge_interface.clone(); - async_std::task::spawn(async move { - handle_ipv4_updates(&conn_task, bridge_interface, "tac-bridge") - .await - .unwrap(); + wtb.spawn_task("ip-tac-bridge-update", async move { + handle_ipv4_updates(&conn_task, bridge_interface, "tac-bridge").await }); this diff --git a/src/dbus/rauc.rs b/src/dbus/rauc.rs index b5ea19f4..8bd11622 100644 --- a/src/dbus/rauc.rs +++ b/src/dbus/rauc.rs @@ -19,6 +19,7 @@ use std::cmp::Ordering; use std::collections::HashMap; use std::time::{Duration, Instant}; +use anyhow::Result; use async_std::channel::Receiver; use async_std::stream::StreamExt; use async_std::sync::Arc; @@ -28,6 +29,7 @@ use serde::{Deserialize, Serialize}; use super::Connection; use crate::broker::{BrokerBuilder, Topic}; +use crate::watched_tasks::WatchedTasksBuilder; mod update_channels; pub use update_channels::Channel; @@ -65,7 +67,7 @@ mod imports { #[cfg(not(feature = "demo_mode"))] mod imports { - pub(super) use anyhow::{bail, Result}; + pub(super) use anyhow::bail; pub(super) use log::error; pub(super) const CHANNELS_DIR: &str = "/usr/share/tacd/update_channels"; @@ -233,7 +235,7 @@ async fn channel_list_update_task( enable_polling: Arc>, channels: Arc>>, slot_status: Arc>>, -) { +) -> Result<()> { let mut previous: Option = None; let mut polling_tasks: Vec> = Vec::new(); @@ -284,6 +286,8 @@ async fn channel_list_update_task( previous = Some(Instant::now()); } + + Ok(()) } impl Rauc { @@ -310,7 +314,11 @@ impl Rauc { } #[cfg(feature = "demo_mode")] - pub fn new(bb: &mut BrokerBuilder, _conn: &Arc) -> Self { + pub fn new( + bb: &mut BrokerBuilder, + wtb: &mut WatchedTasksBuilder, + _conn: &Arc, + ) -> Self { let inst = Self::setup_topics(bb); inst.operation.set("idle".to_string()); @@ -319,19 +327,26 @@ impl Rauc { // Reload the channel list on request let (reload_stream, _) = inst.reload.clone().subscribe_unbounded(); - spawn(channel_list_update_task( - Arc::new(Connection), - reload_stream, - inst.enable_polling.clone(), - inst.channels.clone(), - inst.slot_status.clone(), - )); + wtb.spawn_task( + "rauc-channel-list-update", + channel_list_update_task( + Arc::new(Connection), + reload_stream, + inst.enable_polling.clone(), + inst.channels.clone(), + inst.slot_status.clone(), + ), + ); inst } #[cfg(not(feature = "demo_mode"))] - pub fn new(bb: &mut BrokerBuilder, conn: &Arc) -> Self { + pub fn new( + bb: &mut BrokerBuilder, + wtb: &mut WatchedTasksBuilder, + conn: &Arc, + ) -> Self { let inst = Self::setup_topics(bb); let conn_task = conn.clone(); @@ -341,7 +356,7 @@ impl Rauc { let channels = inst.channels.clone(); let should_reboot = inst.should_reboot.clone(); - spawn(async move { + wtb.spawn_task("rauc-slot-status-update", async move { let proxy = InstallerProxy::new(&conn_task).await.unwrap(); let mut stream = proxy.receive_operation_changed().await; @@ -437,7 +452,7 @@ impl Rauc { operation.set(v); } } else { - break; + break Ok(()); } } }); @@ -446,7 +461,7 @@ impl Rauc { let progress = inst.progress.clone(); // Forward the "progress" property to the broker framework - spawn(async move { + wtb.spawn_task("rauc-progress-update", async move { let proxy = InstallerProxy::new(&conn_task).await.unwrap(); let mut stream = proxy.receive_progress_changed().await; @@ -460,13 +475,15 @@ impl Rauc { progress.set(p.into()); } } + + Ok(()) }); let conn_task = conn.clone(); let last_error = inst.last_error.clone(); // Forward the "last_error" property to the broker framework - spawn(async move { + wtb.spawn_task("rauc-forward-error", async move { let proxy = InstallerProxy::new(&conn_task).await.unwrap(); let mut stream = proxy.receive_last_error_changed().await; @@ -480,13 +497,15 @@ impl Rauc { last_error.set(e); } } + + Ok(()) }); let conn_task = conn.clone(); let (mut install_stream, _) = inst.install.clone().subscribe_unbounded(); // Forward the "install" topic from the broker framework to RAUC - spawn(async move { + wtb.spawn_task("rauc-forward-install", async move { let proxy = InstallerProxy::new(&conn_task).await.unwrap(); while let Some(url) = install_stream.next().await { @@ -498,17 +517,22 @@ impl Rauc { } } } + + Ok(()) }); // Reload the channel list on request let (reload_stream, _) = inst.reload.clone().subscribe_unbounded(); - spawn(channel_list_update_task( - conn.clone(), - reload_stream, - inst.enable_polling.clone(), - inst.channels.clone(), - inst.slot_status.clone(), - )); + wtb.spawn_task( + "rauc-channel-list-update", + channel_list_update_task( + conn.clone(), + reload_stream, + inst.enable_polling.clone(), + inst.channels.clone(), + inst.slot_status.clone(), + ), + ); inst } diff --git a/src/dbus/systemd.rs b/src/dbus/systemd.rs index f7e19d30..056094e7 100644 --- a/src/dbus/systemd.rs +++ b/src/dbus/systemd.rs @@ -17,8 +17,6 @@ use async_std::prelude::*; use async_std::sync::Arc; -use async_std::task::spawn; -use futures::join; use serde::{Deserialize, Serialize}; #[cfg(not(feature = "demo_mode"))] @@ -29,6 +27,7 @@ pub use log::warn; use super::{Connection, Result}; use crate::broker::{BrokerBuilder, Topic}; +use crate::watched_tasks::WatchedTasksBuilder; #[cfg(not(feature = "demo_mode"))] mod manager; @@ -96,12 +95,22 @@ impl Service { } #[cfg(feature = "demo_mode")] - async fn connect(&self, _conn: Arc, _unit_name: &'static str) { + async fn connect( + &self, + _wtb: &mut WatchedTasksBuilder, + _conn: Arc, + _unit_name: &str, + ) { self.status.set(ServiceStatus::get().await.unwrap()); } #[cfg(not(feature = "demo_mode"))] - async fn connect(&self, conn: Arc, unit_name: &'static str) { + async fn connect( + &self, + wtb: &mut WatchedTasksBuilder, + conn: Arc, + unit_name: &'static str, + ) { let unit_path = { let manager = manager::ManagerProxy::new(&conn).await.unwrap(); manager.get_unit(unit_name).await.unwrap() @@ -117,7 +126,7 @@ impl Service { let unit_task = unit.clone(); let status_topic = self.status.clone(); - spawn(async move { + wtb.spawn_task(format!("systemd-{unit_name}-state"), async move { let mut active_state_stream = unit_task.receive_active_state_changed().await.map(|_| ()); let mut sub_state_stream = unit_task.receive_sub_state_changed().await.map(|_| ()); @@ -145,7 +154,7 @@ impl Service { let (mut action_reqs, _) = self.action.clone().subscribe_unbounded(); - spawn(async move { + wtb.spawn_task(format!("systemd-{unit_name}-actions"), async move { while let Some(action) = action_reqs.next().await { let res = match action { ServiceAction::Start => unit.start("replace").await, @@ -160,29 +169,41 @@ impl Service { ); } } + + Ok(()) }); } } impl Systemd { #[cfg(feature = "demo_mode")] - pub fn handle_reboot(reboot: Arc>, _conn: Arc) { + pub fn handle_reboot( + wtb: &mut WatchedTasksBuilder, + reboot: Arc>, + _conn: Arc, + ) { let (mut reboot_reqs, _) = reboot.subscribe_unbounded(); - spawn(async move { + wtb.spawn_task("systemd-reboot", async move { while let Some(req) = reboot_reqs.next().await { if req { println!("Asked to reboot but don't feel like it"); } } + + Ok(()) }); } #[cfg(not(feature = "demo_mode"))] - pub fn handle_reboot(reboot: Arc>, conn: Arc) { + pub fn handle_reboot( + wtb: &mut WatchedTasksBuilder, + reboot: Arc>, + conn: Arc, + ) { let (mut reboot_reqs, _) = reboot.subscribe_unbounded(); - spawn(async move { + wtb.spawn_task("systemd-reboot", async move { let manager = manager::ManagerProxy::new(&conn).await.unwrap(); while let Some(req) = reboot_reqs.next().await { @@ -192,23 +213,31 @@ impl Systemd { } } } + + Ok(()) }); } - pub async fn new(bb: &mut BrokerBuilder, conn: &Arc) -> Self { + pub async fn new( + bb: &mut BrokerBuilder, + wtb: &mut WatchedTasksBuilder, + conn: &Arc, + ) -> Self { let reboot = bb.topic_rw("/v1/tac/reboot", Some(false)); - Self::handle_reboot(reboot.clone(), conn.clone()); + Self::handle_reboot(wtb, reboot.clone(), conn.clone()); let networkmanager = Service::new(bb, "network-manager"); let labgrid = Service::new(bb, "labgrid-exporter"); let iobus = Service::new(bb, "lxa-iobus"); - join!( - networkmanager.connect(conn.clone(), "NetworkManager.service"), - labgrid.connect(conn.clone(), "labgrid-exporter.service"), - iobus.connect(conn.clone(), "lxa-iobus.service"), - ); + networkmanager + .connect(wtb, conn.clone(), "NetworkManager.service") + .await; + labgrid + .connect(wtb, conn.clone(), "labgrid-exporter.service") + .await; + iobus.connect(wtb, conn.clone(), "lxa-iobus.service").await; Self { reboot, diff --git a/src/digital_io.rs b/src/digital_io.rs index f241011a..45086387 100644 --- a/src/digital_io.rs +++ b/src/digital_io.rs @@ -17,10 +17,10 @@ use async_std::prelude::*; use async_std::sync::Arc; -use async_std::task::spawn; use crate::broker::{BrokerBuilder, Topic}; use crate::led::BlinkPattern; +use crate::watched_tasks::WatchedTasksBuilder; #[allow(clippy::items_after_test_module)] #[cfg(test)] @@ -54,6 +54,7 @@ pub struct DigitalIo { /// writing to it. (e.g. whatever it is set to _is_ the line status). fn handle_line_wo( bb: &mut BrokerBuilder, + wtb: &mut WatchedTasksBuilder, path: &str, line_name: &str, initial: bool, @@ -68,7 +69,7 @@ fn handle_line_wo( let (mut src, _) = topic.clone().subscribe_unbounded(); - spawn(async move { + wtb.spawn_task(format!("digital-io-{line_name}-set"), async move { while let Some(ev) = src.next().await { dst.set_value((ev ^ inverted) as _).unwrap(); @@ -77,6 +78,8 @@ fn handle_line_wo( led.set(pattern); } } + + Ok(()) }); topic @@ -85,11 +88,13 @@ fn handle_line_wo( impl DigitalIo { pub fn new( bb: &mut BrokerBuilder, + wtb: &mut WatchedTasksBuilder, led_0: Arc>, led_1: Arc>, ) -> Self { let out_0 = handle_line_wo( bb, + wtb, "/v1/output/out_0/asserted", "OUT_0", false, @@ -99,6 +104,7 @@ impl DigitalIo { let out_1 = handle_line_wo( bb, + wtb, "/v1/output/out_1/asserted", "OUT_1", false, @@ -106,8 +112,24 @@ impl DigitalIo { 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, + wtb, + "/v1/uart/rx/enabled", + "UART_RX_EN", + true, + true, + None, + ); + let uart_tx_en = handle_line_wo( + bb, + wtb, + "/v1/uart/tx/enabled", + "UART_TX_EN", + true, + true, + None, + ); Self { out_0, diff --git a/src/dut_power.rs b/src/dut_power.rs index c9376886..c67d8ca4 100644 --- a/src/dut_power.rs +++ b/src/dut_power.rs @@ -30,6 +30,7 @@ use crate::adc::AdcChannel; use crate::broker::{BrokerBuilder, Topic}; use crate::digital_io::{find_line, LineHandle, LineRequestFlags}; use crate::led::{BlinkPattern, BlinkPatternBuilder}; +use crate::watched_tasks::WatchedTasksBuilder; #[cfg(any(test, feature = "demo_mode"))] mod prio { @@ -249,6 +250,7 @@ fn turn_off_with_reason( /// main interface used by e.g. the web UI pretty. fn setup_labgrid_compat( bb: &mut BrokerBuilder, + wtb: &mut WatchedTasksBuilder, request: Arc>, state: Arc>, ) { @@ -258,7 +260,7 @@ fn setup_labgrid_compat( let (mut state_stream, _) = state.subscribe_unbounded(); let (mut compat_request_stream, _) = compat_request.subscribe_unbounded(); - task::spawn(async move { + wtb.spawn_task("power-compat-from-labgrid", async move { while let Some(req) = compat_request_stream.next().await { match req { 0 => request.set(OutputRequest::Off), @@ -266,9 +268,11 @@ fn setup_labgrid_compat( _ => {} } } + + Ok(()) }); - task::spawn(async move { + wtb.spawn_task("power-compat-to-labgrid", async move { while let Some(state) = state_stream.next().await { match state { OutputState::On => compat_response.set(1), @@ -276,12 +280,15 @@ fn setup_labgrid_compat( _ => compat_response.set(0), } } + + Ok(()) }); } impl DutPwrThread { pub async fn new( bb: &mut BrokerBuilder, + wtb: &mut WatchedTasksBuilder, pwr_volt: AdcChannel, pwr_curr: AdcChannel, pwr_led: Arc>, @@ -487,23 +494,25 @@ impl DutPwrThread { let request_topic = bb.topic_wo::("/v1/dut/powered", None); let state_topic = bb.topic_ro::("/v1/dut/powered", None); - setup_labgrid_compat(bb, request_topic.clone(), state_topic.clone()); + setup_labgrid_compat(bb, wtb, request_topic.clone(), state_topic.clone()); // Requests come from the broker framework and are placed into an atomic // request variable read by the thread. let state_topic_task = state_topic.clone(); let (mut request_stream, _) = request_topic.clone().subscribe_unbounded(); - task::spawn(async move { + wtb.spawn_task("power-from-broker", async move { while let Some(req) = request_stream.next().await { state_topic_task.set(OutputState::Changing); request.store(req as u8, Ordering::Relaxed); } + + Ok(()) }); // State information comes from the thread in the form of an atomic // variable and is forwarded to the broker framework. let state_topic_task = state_topic.clone(); - task::spawn(async move { + wtb.spawn_task("power-to-broker", async move { loop { task::sleep(TASK_INTERVAL).await; @@ -514,7 +523,7 @@ impl DutPwrThread { // Forward the state information to the DUT Power LED let (mut state_stream, _) = state_topic.clone().subscribe_unbounded(); - task::spawn(async move { + wtb.spawn_task("power-to-led", async move { let pattern_on = BlinkPattern::solid(1.0); let pattern_off = BlinkPattern::solid(0.0); let pattern_error = { @@ -541,6 +550,8 @@ impl DutPwrThread { _ => pwr_led.set(pattern_error.clone()), } } + + Ok(()) }); Ok(Self { @@ -564,6 +575,7 @@ mod tests { use crate::adc::Adc; use crate::broker::{BrokerBuilder, Topic}; use crate::digital_io::find_line; + use crate::watched_tasks::WatchedTasksBuilder; use super::{ DutPwrThread, OutputRequest, OutputState, DISCHARGE_LINE_ASSERTED, MAX_CURRENT, @@ -572,16 +584,18 @@ mod tests { #[test] fn failsafe() { + let mut wtb = WatchedTasksBuilder::new(); let pwr_line = find_line("DUT_PWR_EN").unwrap(); let discharge_line = find_line("DUT_PWR_DISCH").unwrap(); let (adc, dut_pwr, led) = { let mut bb = BrokerBuilder::new(); - let adc = block_on(Adc::new(&mut bb)).unwrap(); + let adc = block_on(Adc::new(&mut bb, &mut wtb)).unwrap(); let led = Topic::anonymous(None); let dut_pwr = block_on(DutPwrThread::new( &mut bb, + &mut wtb, adc.pwr_volt.clone(), adc.pwr_curr.clone(), led.clone(), diff --git a/src/http_server.rs b/src/http_server.rs index 94d1a7be..f6d2abce 100644 --- a/src/http_server.rs +++ b/src/http_server.rs @@ -20,6 +20,8 @@ use std::net::TcpListener; use tide::{Body, Response, Server}; +use crate::watched_tasks::WatchedTasksBuilder; + mod serve_dir; use serve_dir::serve_dir; @@ -126,7 +128,10 @@ impl HttpServer { }); } - pub async fn serve(self) -> Result<(), std::io::Error> { - self.server.listen(self.listeners).await + pub fn serve(self, wtb: &mut WatchedTasksBuilder) { + wtb.spawn_task("http-server", async move { + self.server.listen(self.listeners).await?; + Ok(()) + }); } } diff --git a/src/iobus.rs b/src/iobus.rs index d58e1463..e6955a51 100644 --- a/src/iobus.rs +++ b/src/iobus.rs @@ -18,12 +18,13 @@ use std::time::Duration; use async_std::sync::Arc; -use async_std::task::{sleep, spawn}; +use async_std::task::sleep; use serde::{Deserialize, Serialize}; use crate::adc::CalibratedChannel; use crate::broker::{BrokerBuilder, Topic}; +use crate::watched_tasks::WatchedTasksBuilder; const CURRENT_MAX: f32 = 0.2; const VOLTAGE_MIN: f32 = 10.0; @@ -109,6 +110,7 @@ pub struct IoBus { impl IoBus { pub fn new( bb: &mut BrokerBuilder, + wtb: &mut WatchedTasksBuilder, iobus_pwr_en: Arc>, iobus_curr: CalibratedChannel, iobus_volt: CalibratedChannel, @@ -121,7 +123,7 @@ impl IoBus { let server_info_task = server_info.clone(); let nodes_task = nodes.clone(); - spawn(async move { + wtb.spawn_task("iobus-update", async move { loop { if let Ok(si) = http::get("http://127.0.0.1:8080/server-info/") .recv_json::() diff --git a/src/led.rs b/src/led.rs index 1d76dd46..d33c5031 100644 --- a/src/led.rs +++ b/src/led.rs @@ -19,10 +19,10 @@ use std::io::ErrorKind; use async_std::prelude::*; use async_std::sync::Arc; -use async_std::task::spawn; use log::{error, info, warn}; use crate::broker::{BrokerBuilder, Topic}; +use crate::watched_tasks::WatchedTasksBuilder; mod demo_mode; mod extras; @@ -67,6 +67,7 @@ fn get_led_checked(hardware_name: &'static str) -> Option { fn handle_pattern( bb: &mut BrokerBuilder, + wtb: &mut WatchedTasksBuilder, hardware_name: &'static str, topic_name: &'static str, ) -> Arc> { @@ -75,12 +76,14 @@ fn handle_pattern( if let Some(led) = get_led_checked(hardware_name) { let (mut rx, _) = topic.clone().subscribe_unbounded(); - spawn(async move { + wtb.spawn_task("led-pattern-update", async move { while let Some(pattern) = rx.next().await { if let Err(e) = led.set_pattern(pattern) { warn!("Failed to set LED pattern: {}", e); } } + + Ok(()) }); } @@ -89,6 +92,7 @@ fn handle_pattern( fn handle_color( bb: &mut BrokerBuilder, + wtb: &mut WatchedTasksBuilder, hardware_name: &'static str, topic_name: &'static str, ) -> Arc> { @@ -97,9 +101,9 @@ fn handle_color( if let Some(led) = get_led_checked(hardware_name) { let (mut rx, _) = topic.clone().subscribe_unbounded(); - spawn(async move { + wtb.spawn_task("led-color-update", async move { while let Some((r, g, b)) = rx.next().await { - let max = led.max_brightness().unwrap(); + let max = led.max_brightness()?; // I've encountered LEDs staying off when set to the max value, // but setting them to (max - 1) turned them on. @@ -109,6 +113,8 @@ fn handle_color( warn!("Failed to set LED color: {}", e); } } + + Ok(()) }); } @@ -116,15 +122,15 @@ fn handle_color( } impl Led { - pub fn new(bb: &mut BrokerBuilder) -> Self { + pub fn new(bb: &mut BrokerBuilder, wtb: &mut WatchedTasksBuilder) -> Self { Self { - out_0: handle_pattern(bb, "tac:green:out0", "out_0"), - out_1: handle_pattern(bb, "tac:green:out1", "out_1"), - dut_pwr: handle_pattern(bb, "tac:green:dutpwr", "dut_pwr"), - eth_dut: handle_pattern(bb, "tac:green:statusdut", "eth_dut"), - eth_lab: handle_pattern(bb, "tac:green:statuslab", "eth_lab"), - status: handle_pattern(bb, "rgb:status", "status"), - status_color: handle_color(bb, "rgb:status", "status"), + out_0: handle_pattern(bb, wtb, "tac:green:out0", "out_0"), + out_1: handle_pattern(bb, wtb, "tac:green:out1", "out_1"), + dut_pwr: handle_pattern(bb, wtb, "tac:green:dutpwr", "dut_pwr"), + eth_dut: handle_pattern(bb, wtb, "tac:green:statusdut", "eth_dut"), + eth_lab: handle_pattern(bb, wtb, "tac:green:statuslab", "eth_lab"), + status: handle_pattern(bb, wtb, "rgb:status", "status"), + status_color: handle_color(bb, wtb, "rgb:status", "status"), } } } diff --git a/src/main.rs b/src/main.rs index 8580c879..6a9078bc 100644 --- a/src/main.rs +++ b/src/main.rs @@ -17,7 +17,6 @@ use anyhow::Result; use async_std::future::pending; -use futures::{select, FutureExt}; use log::{error, info}; mod adc; @@ -38,6 +37,7 @@ mod temperatures; mod ui; mod usb_hub; mod watchdog; +mod watched_tasks; use adc::Adc; use backlight::Backlight; @@ -55,29 +55,37 @@ use temperatures::Temperatures; use ui::{message, setup_display, Display, Ui, UiResources}; use usb_hub::UsbHub; use watchdog::Watchdog; +use watched_tasks::WatchedTasksBuilder; + +async fn init() -> Result<(Ui, HttpServer, WatchedTasksBuilder)> { + // The tacd spawns a couple of async tasks that should run as long as + // the tacd runs and if any one fails the tacd should stop. + // These tasks are spawned via the watched task builder. + let mut wtb = WatchedTasksBuilder::new(); -async fn init() -> Result<(Ui, HttpServer, Option)> { // The BrokerBuilder collects topics that should be exported via the // MQTT/REST APIs. // The topics are also used to pass around data inside the tacd. let mut bb = BrokerBuilder::new(); // Expose hardware on the TAC via the broker framework. - let backlight = Backlight::new(&mut bb)?; - let led = Led::new(&mut bb); - let adc = Adc::new(&mut bb).await?; + let backlight = Backlight::new(&mut bb, &mut wtb)?; + let led = Led::new(&mut bb, &mut wtb); + let adc = Adc::new(&mut bb, &mut wtb).await?; let dut_pwr = DutPwrThread::new( &mut bb, + &mut wtb, adc.pwr_volt.clone(), adc.pwr_curr.clone(), led.dut_pwr.clone(), ) .await?; - let dig_io = DigitalIo::new(&mut bb, led.out_0.clone(), led.out_1.clone()); - let regulators = Regulators::new(&mut bb); + let dig_io = DigitalIo::new(&mut bb, &mut wtb, led.out_0.clone(), led.out_1.clone()); + let regulators = Regulators::new(&mut bb, &mut wtb); let temperatures = Temperatures::new(&mut bb); let usb_hub = UsbHub::new( &mut bb, + &mut wtb, adc.usb_host_curr.fast.clone(), adc.usb_host1_curr.fast.clone(), adc.usb_host2_curr.fast.clone(), @@ -88,12 +96,14 @@ async fn init() -> Result<(Ui, HttpServer, Option)> { // to them via HTTP / DBus APIs. let iobus = IoBus::new( &mut bb, + &mut wtb, regulators.iobus_pwr_en.clone(), adc.iobus_curr.fast.clone(), adc.iobus_volt.fast.clone(), ); let (hostname, network, rauc, systemd) = { - let dbus = DbusSession::new(&mut bb, led.eth_dut.clone(), led.eth_lab.clone()).await; + let dbus = + DbusSession::new(&mut bb, &mut wtb, led.eth_dut.clone(), led.eth_lab.clone()).await; (dbus.hostname, dbus.network, dbus.rauc, dbus.systemd) }; @@ -112,7 +122,7 @@ async fn init() -> Result<(Ui, HttpServer, Option)> { let mut http_server = HttpServer::new(); // Allow editing some aspects of the TAC configuration when in "setup mode". - let setup_mode = SetupMode::new(&mut bb, &mut http_server.server); + let setup_mode = SetupMode::new(&mut bb, &mut wtb, &mut http_server.server); // Expose a live log of the TAC's systemd journal so it can be viewed // in the web interface. @@ -140,43 +150,39 @@ async fn init() -> Result<(Ui, HttpServer, Option)> { usb_hub, }; - Ui::new(&mut bb, resources) + Ui::new(&mut bb, &mut wtb, resources)? }; // Consume the BrokerBuilder (no further topics can be added or removed) // and expose the topics via HTTP and MQTT-over-websocket. - bb.build(&mut http_server.server); + bb.build(&mut wtb, &mut http_server.server); - Ok((ui, http_server, watchdog)) + // If a watchdog was requested by systemd we can now start feeding it + if let Some(watchdog) = watchdog { + watchdog.keep_fed(&mut wtb)?; + } + + Ok((ui, http_server, wtb)) } async fn run( ui: Ui, mut http_server: HttpServer, - watchdog: Option, display: Display, + mut wtb: WatchedTasksBuilder, ) -> Result<()> { // Expose the display as a .png on the web server ui::serve_display(&mut http_server.server, display.screenshooter()); - info!("Setup complete. Handling requests"); + // Start serving files and the API + http_server.serve(&mut wtb); - // Run until the user interface, http server or (if selected) the watchdog - // exits (with an error). - if let Some(watchdog) = watchdog { - select! { - ui_err = ui.run(display).fuse() => ui_err, - wi_err = http_server.serve().fuse() => wi_err, - wd_err = watchdog.keep_fed().fuse() => wd_err, - }? - } else { - select! { - ui_err = ui.run(display).fuse() => ui_err, - wi_err = http_server.serve().fuse() => wi_err, - }? - } + // Start drawing the UI + ui.run(&mut wtb, display); + + info!("Setup complete. Handling requests"); - Ok(()) + wtb.watch().await } #[async_std::main] @@ -187,7 +193,7 @@ async fn main() -> Result<()> { let display = setup_display(); match init().await { - Ok((ui, http_server, watchdog)) => run(ui, http_server, watchdog, display).await, + Ok((ui, http_server, wtb)) => run(ui, http_server, display, wtb).await, Err(e) => { // Display a detailed error message on stderr (and thus in the journal) ... error!("Failed to initialize tacd: {e}"); diff --git a/src/regulators.rs b/src/regulators.rs index 6ef82c28..6f47ae66 100644 --- a/src/regulators.rs +++ b/src/regulators.rs @@ -17,9 +17,9 @@ use async_std::prelude::*; use async_std::sync::Arc; -use async_std::task::spawn; use crate::broker::{BrokerBuilder, Topic}; +use crate::watched_tasks::WatchedTasksBuilder; #[cfg(feature = "demo_mode")] mod reg { @@ -71,6 +71,7 @@ pub struct Regulators { fn handle_regulator( bb: &mut BrokerBuilder, + wtb: &mut WatchedTasksBuilder, path: &str, regulator_name: &'static str, initial: bool, @@ -78,20 +79,22 @@ fn handle_regulator( let topic = bb.topic_rw(path, Some(initial)); let (mut src, _) = topic.clone().subscribe_unbounded(); - spawn(async move { + wtb.spawn_task(format!("regulator-{regulator_name}-action"), async move { while let Some(ev) = src.next().await { regulator_set(regulator_name, ev).unwrap(); } + + Ok(()) }); topic } impl Regulators { - pub fn new(bb: &mut BrokerBuilder) -> Self { + pub fn new(bb: &mut BrokerBuilder, wtb: &mut WatchedTasksBuilder) -> Self { Self { - iobus_pwr_en: handle_regulator(bb, "/v1/iobus/powered", "output-iobus-12v", true), - uart_pwr_en: handle_regulator(bb, "/v1/uart/powered", "output-vuart", true), + iobus_pwr_en: handle_regulator(bb, wtb, "/v1/iobus/powered", "output-iobus-12v", true), + uart_pwr_en: handle_regulator(bb, wtb, "/v1/uart/powered", "output-vuart", true), } } } diff --git a/src/setup_mode.rs b/src/setup_mode.rs index d735cfef..3ef41261 100644 --- a/src/setup_mode.rs +++ b/src/setup_mode.rs @@ -21,10 +21,10 @@ use std::path::Path; use async_std::prelude::*; use async_std::sync::Arc; -use async_std::task::spawn; use tide::{http::mime, Request, Response, Server}; use crate::broker::{BrokerBuilder, Topic}; +use crate::watched_tasks::WatchedTasksBuilder; #[cfg(feature = "demo_mode")] const AUTHORIZED_KEYS_PATH: &str = "demo_files/home/root/ssh/authorized_keys"; @@ -106,7 +106,7 @@ impl SetupMode { }); } - fn handle_leave_requests(&self, bb: &mut BrokerBuilder) { + fn handle_leave_requests(&self, bb: &mut BrokerBuilder, wtb: &mut WatchedTasksBuilder) { // Use the "register a read-only and a write-only topic with the same name // to perform validation" trick that is also used with the DUT power endpoint. // We must make sure that a client from the web can only ever trigger _leaving_ @@ -116,17 +116,23 @@ impl SetupMode { .subscribe_unbounded(); let setup_mode = self.setup_mode.clone(); - spawn(async move { + wtb.spawn_task("setup-mode-leave-request", async move { while let Some(lr) = leave_requests.next().await { if !lr { // Only ever set the setup mode to false in here setup_mode.set(false) } } + + Ok(()) }); } - pub fn new(bb: &mut BrokerBuilder, server: &mut Server<()>) -> Self { + pub fn new( + bb: &mut BrokerBuilder, + wtb: &mut WatchedTasksBuilder, + server: &mut Server<()>, + ) -> Self { let this = Self { setup_mode: bb.topic("/v1/tac/setup_mode", true, false, true, Some(true), 1), show_help: bb.topic( @@ -139,7 +145,7 @@ impl SetupMode { ), }; - this.handle_leave_requests(bb); + this.handle_leave_requests(bb, wtb); this.expose_file_conditionally(server, AUTHORIZED_KEYS_PATH, "/v1/tac/ssh/authorized_keys"); this diff --git a/src/ui.rs b/src/ui.rs index cf9cc292..f68e18c1 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -17,14 +17,15 @@ use std::time::Duration; +use anyhow::Result; use async_std::prelude::*; use async_std::sync::Arc; -use async_std::task::spawn; use futures::{select, FutureExt}; use tide::{Response, Server}; use crate::broker::{BrokerBuilder, Topic}; use crate::led::{BlinkPattern, BlinkPatternBuilder}; +use crate::watched_tasks::WatchedTasksBuilder; mod alerts; mod buttons; @@ -123,7 +124,11 @@ pub fn serve_display(server: &mut Server<()>, screenshooter: ScreenShooter) { } impl Ui { - pub fn new(bb: &mut BrokerBuilder, res: UiResources) -> Self { + pub fn new( + bb: &mut BrokerBuilder, + wtb: &mut WatchedTasksBuilder, + res: UiResources, + ) -> Result { let screen = bb.topic_rw("/v1/tac/display/screen", Some(NormalScreen::first())); let locator = bb.topic_rw("/v1/tac/display/locator", Some(false)); let buttons = bb.topic("/v1/tac/display/buttons", true, true, false, None, 0); @@ -133,7 +138,7 @@ impl Ui { alerts.assert(AlertScreen::ScreenSaver); // Initialize all the screens now so they can be activated later - let screens = screens::init(&res, &alerts, &buttons, &reboot_message, &locator); + let screens = screens::init(wtb, &res, &alerts, &buttons, &reboot_message, &locator); handle_buttons( "/dev/input/by-path/platform-gpio-keys-event", @@ -144,7 +149,7 @@ impl Ui { let led_status_pattern = res.led.status.clone(); let led_status_color = res.led.status_color.clone(); let (mut locator_stream, _) = locator.clone().subscribe_unbounded(); - spawn(async move { + wtb.spawn_task("locator-led-updater", async move { let pattern_locator_on = BlinkPatternBuilder::new(0.0) .fade_to(1.0, Duration::from_millis(100)) .stay_for(Duration::from_millis(300)) @@ -165,9 +170,11 @@ impl Ui { led_status_pattern.set(pattern_locator_off.clone()); } } + + Ok(()) }); - Self { + Ok(Self { screen, alerts, locator, @@ -175,10 +182,10 @@ impl Ui { screens, reboot_message, res, - } + }) } - pub async fn run(mut self, display: Display) -> Result<(), std::io::Error> { + pub async fn render_loop(mut self, display: Display) -> Result<(), std::io::Error> { let (mut screen_rx, _) = self.screen.clone().subscribe_unbounded(); let (mut alerts_rx, _) = self.alerts.clone().subscribe_unbounded(); let (mut button_events, _) = self.buttons.clone().subscribe_unbounded(); @@ -282,4 +289,12 @@ impl Ui { Ok(()) } + + pub fn run(self, wtb: &mut WatchedTasksBuilder, display: Display) { + wtb.spawn_task("screen-render-loop", async move { + self.render_loop(display).await?; + + Ok(()) + }); + } } diff --git a/src/ui/screens.rs b/src/ui/screens.rs index 876a231f..bd269abe 100644 --- a/src/ui/screens.rs +++ b/src/ui/screens.rs @@ -65,8 +65,8 @@ use usb_overload::UsbOverloadScreen; use super::buttons; use super::widgets; use super::{AlertList, Alerter, InputEvent, Ui, UiResources}; -use crate::broker::Topic; use crate::ui::display::{Display, DisplayExclusive}; +use crate::{broker::Topic, watched_tasks::WatchedTasksBuilder}; use buttons::ButtonEvent; use widgets::UI_TEXT_FONT; @@ -120,7 +120,7 @@ impl NormalScreen { } #[async_trait] -pub(super) trait ActiveScreen { +pub(super) trait ActiveScreen: Send { fn my_type(&self) -> Screen; async fn deactivate(self: Box) -> Display; fn input(&mut self, ev: InputEvent); @@ -185,6 +185,7 @@ pub fn splash(target: &mut DisplayExclusive) -> Rectangle { } pub(super) fn init( + wtb: &mut WatchedTasksBuilder, res: &UiResources, alerts: &Arc>, buttons: &Arc>, @@ -198,24 +199,26 @@ pub(super) fn init( Box::new(SystemScreen::new()), Box::new(UartScreen::new()), Box::new(UsbScreen::new()), - Box::new(HelpScreen::new(alerts, &res.setup_mode.show_help)), - Box::new(IoBusHealthScreen::new(alerts, &res.iobus.supply_fault)), + Box::new(HelpScreen::new(wtb, alerts, &res.setup_mode.show_help)), + Box::new(IoBusHealthScreen::new(wtb, alerts, &res.iobus.supply_fault)), Box::new(UpdateInstallationScreen::new( + wtb, alerts, &res.rauc.operation, reboot_message, &res.rauc.should_reboot, )), - Box::new(UpdateAvailableScreen::new(alerts, &res.rauc.channels)), - Box::new(RebootConfirmScreen::new(alerts, reboot_message)), - Box::new(ScreenSaverScreen::new(buttons, alerts)), - Box::new(SetupScreen::new(alerts, &res.setup_mode.setup_mode)), + Box::new(UpdateAvailableScreen::new(wtb, alerts, &res.rauc.channels)), + Box::new(RebootConfirmScreen::new(wtb, alerts, reboot_message)), + Box::new(ScreenSaverScreen::new(wtb, buttons, alerts)), + Box::new(SetupScreen::new(wtb, alerts, &res.setup_mode.setup_mode)), Box::new(OverTemperatureScreen::new( + wtb, alerts, &res.temperatures.warning, )), - Box::new(LocatorScreen::new(alerts, locator)), - Box::new(UsbOverloadScreen::new(alerts, &res.usb_hub.overload)), - Box::new(PowerFailScreen::new(alerts, &res.dut_pwr.state)), + Box::new(LocatorScreen::new(wtb, alerts, locator)), + Box::new(UsbOverloadScreen::new(wtb, alerts, &res.usb_hub.overload)), + Box::new(PowerFailScreen::new(wtb, alerts, &res.dut_pwr.state)), ] } diff --git a/src/ui/screens/help.rs b/src/ui/screens/help.rs index ac3ac210..4fa308f3 100644 --- a/src/ui/screens/help.rs +++ b/src/ui/screens/help.rs @@ -17,7 +17,6 @@ use async_std::prelude::*; use async_std::sync::Arc; -use async_std::task::spawn; use async_trait::async_trait; use embedded_graphics::prelude::Point; @@ -27,6 +26,7 @@ use super::{ ActivatableScreen, ActiveScreen, AlertList, AlertScreen, Alerter, InputEvent, Screen, Ui, }; use crate::broker::Topic; +use crate::watched_tasks::WatchedTasksBuilder; const SCREEN_TYPE: AlertScreen = AlertScreen::Help; const PAGES: &[&str] = &[ @@ -65,11 +65,15 @@ struct Active { } impl HelpScreen { - pub fn new(alerts: &Arc>, show_help: &Arc>) -> Self { + pub fn new( + wtb: &mut WatchedTasksBuilder, + alerts: &Arc>, + show_help: &Arc>, + ) -> Self { let (mut show_help_events, _) = show_help.clone().subscribe_unbounded(); let alerts = alerts.clone(); - spawn(async move { + wtb.spawn_task("screen-help-activator", async move { while let Some(show_help) = show_help_events.next().await { if show_help { alerts.assert(AlertScreen::Help); @@ -77,6 +81,8 @@ impl HelpScreen { alerts.deassert(AlertScreen::Help); } } + + Ok(()) }); Self diff --git a/src/ui/screens/iobus_health.rs b/src/ui/screens/iobus_health.rs index 2eddbb7e..fa5b4ffc 100644 --- a/src/ui/screens/iobus_health.rs +++ b/src/ui/screens/iobus_health.rs @@ -17,7 +17,6 @@ use async_std::prelude::*; use async_std::sync::Arc; -use async_std::task::spawn; use async_trait::async_trait; use embedded_graphics::{ mono_font::MonoTextStyle, pixelcolor::BinaryColor, prelude::*, text::Text, @@ -30,6 +29,7 @@ use super::{ }; use crate::broker::Topic; use crate::measurement::Measurement; +use crate::watched_tasks::WatchedTasksBuilder; const SCREEN_TYPE: AlertScreen = AlertScreen::IoBusHealth; @@ -41,11 +41,15 @@ struct Active { } impl IoBusHealthScreen { - pub fn new(alerts: &Arc>, supply_fault: &Arc>) -> Self { + pub fn new( + wtb: &mut WatchedTasksBuilder, + alerts: &Arc>, + supply_fault: &Arc>, + ) -> Self { let (mut supply_fault_events, _) = supply_fault.clone().subscribe_unbounded(); let alerts = alerts.clone(); - spawn(async move { + wtb.spawn_task("screen-iobus-health-activator", async move { while let Some(fault) = supply_fault_events.next().await { if fault { alerts.assert(SCREEN_TYPE); @@ -53,6 +57,8 @@ impl IoBusHealthScreen { alerts.deassert(SCREEN_TYPE); } } + + Ok(()) }); Self diff --git a/src/ui/screens/locator.rs b/src/ui/screens/locator.rs index f8145b37..e9025716 100644 --- a/src/ui/screens/locator.rs +++ b/src/ui/screens/locator.rs @@ -19,7 +19,6 @@ use std::time::Instant; use async_std::prelude::*; use async_std::sync::Arc; -use async_std::task::spawn; use async_trait::async_trait; use embedded_graphics::{ mono_font::MonoTextStyle, @@ -35,6 +34,7 @@ use super::{ Ui, }; use crate::broker::Topic; +use crate::watched_tasks::WatchedTasksBuilder; const SCREEN_TYPE: AlertScreen = AlertScreen::Locator; @@ -46,11 +46,15 @@ struct Active { } impl LocatorScreen { - pub fn new(alerts: &Arc>, locator: &Arc>) -> Self { + pub fn new( + wtb: &mut WatchedTasksBuilder, + alerts: &Arc>, + locator: &Arc>, + ) -> Self { let (mut locator_events, _) = locator.clone().subscribe_unbounded(); let alerts = alerts.clone(); - spawn(async move { + wtb.spawn_task("screen-locator-activator", async move { while let Some(locator) = locator_events.next().await { if locator { alerts.assert(SCREEN_TYPE); @@ -58,6 +62,8 @@ impl LocatorScreen { alerts.deassert(SCREEN_TYPE); } } + + Ok(()) }); Self diff --git a/src/ui/screens/overtemperature.rs b/src/ui/screens/overtemperature.rs index 48b2289c..182c5392 100644 --- a/src/ui/screens/overtemperature.rs +++ b/src/ui/screens/overtemperature.rs @@ -17,7 +17,6 @@ use async_std::prelude::*; use async_std::sync::Arc; -use async_std::task::spawn; use async_trait::async_trait; use embedded_graphics::{ mono_font::MonoTextStyle, pixelcolor::BinaryColor, prelude::*, text::Text, @@ -31,6 +30,7 @@ use super::{ use crate::broker::Topic; use crate::measurement::Measurement; use crate::temperatures::Warning; +use crate::watched_tasks::WatchedTasksBuilder; const SCREEN_TYPE: AlertScreen = AlertScreen::OverTemperature; @@ -41,17 +41,23 @@ struct Active { } impl OverTemperatureScreen { - pub fn new(alerts: &Arc>, warning: &Arc>) -> Self { + pub fn new( + wtb: &mut WatchedTasksBuilder, + alerts: &Arc>, + warning: &Arc>, + ) -> Self { let (mut warning_events, _) = warning.clone().subscribe_unbounded(); let alerts = alerts.clone(); - spawn(async move { + wtb.spawn_task("screen-overtemperature-activator", async move { while let Some(warning) = warning_events.next().await { match warning { Warning::Okay => alerts.deassert(SCREEN_TYPE), Warning::SocHigh | Warning::SocCritical => alerts.assert(SCREEN_TYPE), } } + + Ok(()) }); Self diff --git a/src/ui/screens/power_fail.rs b/src/ui/screens/power_fail.rs index 77aa020a..6ff11c52 100644 --- a/src/ui/screens/power_fail.rs +++ b/src/ui/screens/power_fail.rs @@ -17,7 +17,6 @@ use async_std::prelude::*; use async_std::sync::Arc; -use async_std::task::spawn; use async_trait::async_trait; use embedded_graphics::{ mono_font::MonoTextStyle, pixelcolor::BinaryColor, prelude::*, text::Text, @@ -31,6 +30,7 @@ use super::{ }; use crate::broker::Topic; use crate::dut_power::{OutputRequest, OutputState}; +use crate::watched_tasks::WatchedTasksBuilder; const SCREEN_TYPE: AlertScreen = AlertScreen::PowerFail; @@ -58,12 +58,16 @@ struct Active { } impl PowerFailScreen { - pub fn new(alerts: &Arc>, out_state: &Arc>) -> Self { + pub fn new( + wtb: &mut WatchedTasksBuilder, + alerts: &Arc>, + out_state: &Arc>, + ) -> Self { let (mut out_state_events, _) = out_state.clone().subscribe_unbounded(); let alerts = alerts.clone(); - spawn(async move { + wtb.spawn_task("screen-power-fail-activator", async move { while let Some(state) = out_state_events.next().await { match state { OutputState::On | OutputState::Off | OutputState::OffFloating => { @@ -76,6 +80,8 @@ impl PowerFailScreen { OutputState::Changing => {} } } + + Ok(()) }); Self diff --git a/src/ui/screens/reboot.rs b/src/ui/screens/reboot.rs index 563bfa0e..eb391511 100644 --- a/src/ui/screens/reboot.rs +++ b/src/ui/screens/reboot.rs @@ -17,7 +17,6 @@ use async_std::prelude::*; use async_std::sync::Arc; -use async_std::task::spawn; use async_trait::async_trait; use embedded_graphics::{ mono_font::MonoTextStyle, @@ -32,6 +31,7 @@ use super::{ Ui, }; use crate::broker::Topic; +use crate::watched_tasks::WatchedTasksBuilder; const SCREEN_TYPE: AlertScreen = AlertScreen::RebootConfirm; @@ -41,6 +41,7 @@ pub struct RebootConfirmScreen { impl RebootConfirmScreen { pub fn new( + wtb: &mut WatchedTasksBuilder, alerts: &Arc>, reboot_message: &Arc>>, ) -> Self { @@ -49,7 +50,7 @@ impl RebootConfirmScreen { let reboot_message = reboot_message.clone(); let alerts = alerts.clone(); - spawn(async move { + wtb.spawn_task("screen-reboot-activator", async move { while let Some(reboot_message) = reboot_message_events.next().await { if reboot_message.is_some() { alerts.assert(SCREEN_TYPE); @@ -57,6 +58,8 @@ impl RebootConfirmScreen { alerts.deassert(SCREEN_TYPE); } } + + Ok(()) }); Self { reboot_message } diff --git a/src/ui/screens/screensaver.rs b/src/ui/screens/screensaver.rs index e352c2d0..936d89e1 100644 --- a/src/ui/screens/screensaver.rs +++ b/src/ui/screens/screensaver.rs @@ -21,7 +21,6 @@ use std::time::{Duration, SystemTime}; use async_std::future::timeout; use async_std::prelude::*; use async_std::sync::Arc; -use async_std::task::spawn; use async_trait::async_trait; use embedded_graphics::{ mono_font::{ascii::FONT_10X20, MonoFont, MonoTextStyle}, @@ -38,6 +37,7 @@ use super::{ Screen, Ui, }; use crate::broker::Topic; +use crate::watched_tasks::WatchedTasksBuilder; const UI_TEXT_FONT: MonoFont = FONT_10X20; const SCREEN_TYPE: AlertScreen = AlertScreen::ScreenSaver; @@ -91,12 +91,16 @@ impl BounceAnimation { pub struct ScreenSaverScreen; impl ScreenSaverScreen { - pub fn new(buttons: &Arc>, alerts: &Arc>) -> Self { + pub fn new( + wtb: &mut WatchedTasksBuilder, + buttons: &Arc>, + alerts: &Arc>, + ) -> Self { // Activate screensaver if no button is pressed for some time let (mut buttons_events, _) = buttons.clone().subscribe_unbounded(); let alerts = alerts.clone(); - spawn(async move { + wtb.spawn_task("screen-screensaver-activator", async move { loop { let ev = timeout(SCREENSAVER_TIMEOUT, buttons_events.next()).await; let activate_screensaver = match ev { @@ -109,6 +113,8 @@ impl ScreenSaverScreen { alerts.assert(SCREEN_TYPE); } } + + Ok(()) }); Self diff --git a/src/ui/screens/setup.rs b/src/ui/screens/setup.rs index a91cb575..b556ba37 100644 --- a/src/ui/screens/setup.rs +++ b/src/ui/screens/setup.rs @@ -28,6 +28,7 @@ use super::{ Ui, }; use crate::broker::{Native, SubscriptionHandle, Topic}; +use crate::watched_tasks::WatchedTasksBuilder; const SCREEN_TYPE: AlertScreen = AlertScreen::Setup; @@ -48,11 +49,15 @@ struct Active { } impl SetupScreen { - pub fn new(alerts: &Arc>, setup_mode: &Arc>) -> Self { + pub fn new( + wtb: &mut WatchedTasksBuilder, + alerts: &Arc>, + setup_mode: &Arc>, + ) -> Self { let (mut setup_mode_events, _) = setup_mode.clone().subscribe_unbounded(); let alerts = alerts.clone(); - spawn(async move { + wtb.spawn_task("screen-setup-avtivator", async move { while let Some(setup_mode) = setup_mode_events.next().await { if setup_mode { alerts.assert(AlertScreen::Setup); @@ -60,6 +65,8 @@ impl SetupScreen { alerts.deassert(AlertScreen::Setup); } } + + Ok(()) }); Self diff --git a/src/ui/screens/update_available.rs b/src/ui/screens/update_available.rs index 69e200e7..065661c5 100644 --- a/src/ui/screens/update_available.rs +++ b/src/ui/screens/update_available.rs @@ -17,7 +17,6 @@ use async_std::prelude::*; use async_std::sync::Arc; -use async_std::task::spawn; use async_trait::async_trait; use embedded_graphics::{ mono_font::MonoTextStyle, pixelcolor::BinaryColor, prelude::*, text::Text, @@ -31,6 +30,7 @@ use super::{ }; use crate::broker::Topic; use crate::dbus::rauc::Channel; +use crate::watched_tasks::WatchedTasksBuilder; const SCREEN_TYPE: AlertScreen = AlertScreen::UpdateAvailable; @@ -139,13 +139,17 @@ struct Active { } impl UpdateAvailableScreen { - pub fn new(alerts: &Arc>, channels: &Arc>>) -> Self { + pub fn new( + wtb: &mut WatchedTasksBuilder, + alerts: &Arc>, + channels: &Arc>>, + ) -> Self { let (mut channels_events, _) = channels.clone().subscribe_unbounded(); let alerts = alerts.clone(); let selection = Topic::anonymous(Some(Selection::new())); let selection_task = selection.clone(); - spawn(async move { + wtb.spawn_task("screen-update-available-activator", async move { while let Some(channels) = channels_events.next().await { selection_task.modify(|sel| sel.unwrap().update_channels(channels)); @@ -155,6 +159,8 @@ impl UpdateAvailableScreen { alerts.deassert(SCREEN_TYPE); } } + + Ok(()) }); Self { selection } diff --git a/src/ui/screens/update_installation.rs b/src/ui/screens/update_installation.rs index 83e7a0e3..15bdcfec 100644 --- a/src/ui/screens/update_installation.rs +++ b/src/ui/screens/update_installation.rs @@ -17,7 +17,6 @@ use async_std::prelude::*; use async_std::sync::Arc; -use async_std::task::spawn; use async_trait::async_trait; use embedded_graphics::prelude::*; @@ -28,6 +27,7 @@ use super::{ }; use crate::broker::Topic; use crate::dbus::rauc::Progress; +use crate::watched_tasks::WatchedTasksBuilder; const SCREEN_TYPE: AlertScreen = AlertScreen::UpdateInstallation; const REBOOT_MESSAGE: &str = "There is a newer @@ -46,6 +46,7 @@ struct Active { impl UpdateInstallationScreen { pub fn new( + wtb: &mut WatchedTasksBuilder, alerts: &Arc>, operation: &Arc>, reboot_message: &Arc>>, @@ -54,24 +55,28 @@ impl UpdateInstallationScreen { let (mut operation_events, _) = operation.clone().subscribe_unbounded(); let alerts = alerts.clone(); - spawn(async move { + wtb.spawn_task("screen-update-activator", async move { while let Some(ev) = operation_events.next().await { match ev.as_str() { "installing" => alerts.assert(SCREEN_TYPE), _ => alerts.deassert(SCREEN_TYPE), }; } + + Ok(()) }); let (mut should_reboot_events, _) = should_reboot.clone().subscribe_unbounded(); let reboot_message = reboot_message.clone(); - spawn(async move { + wtb.spawn_task("screen-update-should-reboot", async move { while let Some(should_reboot) = should_reboot_events.next().await { if should_reboot { reboot_message.set(Some(REBOOT_MESSAGE.to_string())) } } + + Ok(()) }); Self diff --git a/src/ui/screens/usb_overload.rs b/src/ui/screens/usb_overload.rs index 7962d0d7..86a2dd05 100644 --- a/src/ui/screens/usb_overload.rs +++ b/src/ui/screens/usb_overload.rs @@ -17,7 +17,6 @@ use async_std::prelude::*; use async_std::sync::Arc; -use async_std::task::spawn; use async_trait::async_trait; use embedded_graphics::{ mono_font::MonoTextStyle, pixelcolor::BinaryColor, prelude::*, text::Text, @@ -31,6 +30,7 @@ use super::{ use crate::broker::Topic; use crate::measurement::Measurement; use crate::usb_hub::{OverloadedPort, MAX_PORT_CURRENT, MAX_TOTAL_CURRENT}; +use crate::watched_tasks::WatchedTasksBuilder; const SCREEN_TYPE: AlertScreen = AlertScreen::UsbOverload; const OFFSET_BAR: Point = Point::new(75, -14); @@ -46,13 +46,14 @@ struct Active { impl UsbOverloadScreen { pub fn new( + wtb: &mut WatchedTasksBuilder, alerts: &Arc>, overload: &Arc>>, ) -> Self { let (mut overload_events, _) = overload.clone().subscribe_unbounded(); let alerts = alerts.clone(); - spawn(async move { + wtb.spawn_task("screen-usb-overload-activator", async move { while let Some(overload) = overload_events.next().await { if overload.is_some() { alerts.assert(SCREEN_TYPE) @@ -60,6 +61,8 @@ impl UsbOverloadScreen { alerts.deassert(SCREEN_TYPE) } } + + Ok(()) }); Self diff --git a/src/usb_hub.rs b/src/usb_hub.rs index 80b9bae8..fc6ef329 100644 --- a/src/usb_hub.rs +++ b/src/usb_hub.rs @@ -20,11 +20,12 @@ use std::time::Duration; use async_std::prelude::*; use async_std::sync::Arc; -use async_std::task::{sleep, spawn}; +use async_std::task::sleep; use serde::{Deserialize, Serialize}; use crate::adc::CalibratedChannel; use crate::broker::{BrokerBuilder, Topic}; +use crate::watched_tasks::WatchedTasksBuilder; #[cfg(feature = "demo_mode")] mod rw { @@ -197,7 +198,12 @@ pub struct UsbHub { pub port3: UsbPort, } -fn handle_port(bb: &mut BrokerBuilder, name: &'static str, base: &'static str) -> UsbPort { +fn handle_port( + bb: &mut BrokerBuilder, + wtb: &mut WatchedTasksBuilder, + name: &'static str, + base: &'static str, +) -> UsbPort { let port = UsbPort { request: bb.topic_wo(format!("/v1/usb/host/{name}/powered").as_str(), None), status: bb.topic_ro(format!("/v1/usb/host/{name}/powered").as_str(), None), @@ -212,11 +218,11 @@ fn handle_port(bb: &mut BrokerBuilder, name: &'static str, base: &'static str) - // Spawn a task that turns USB port power on or off upon request. // Also clears the device info upon power off so it does not contain stale // information until the next poll. - spawn(async move { + wtb.spawn_task(format!("usb-hub-{name}-actions"), async move { let (mut src, _) = request.subscribe_unbounded(); while let Some(ev) = src.next().await { - write(&disable_path, if ev { b"0" } else { b"1" }).unwrap(); + write(&disable_path, if ev { b"0" } else { b"1" })?; if !ev { device.set(None); @@ -224,6 +230,8 @@ fn handle_port(bb: &mut BrokerBuilder, name: &'static str, base: &'static str) - status.set(ev); } + + Ok(()) }); let status = port.status.clone(); @@ -241,7 +249,7 @@ fn handle_port(bb: &mut BrokerBuilder, name: &'static str, base: &'static str) - // Spawn a task that periodically polls the USB device info and disable state // and updates the corresponding topic on changes. - spawn(async move { + wtb.spawn_task(format!("usb-hub-{name}-state"), async move { loop { if let Ok(disable) = read_to_string(&disable_path) { let is_powered = match disable.trim() { @@ -279,6 +287,7 @@ fn handle_port(bb: &mut BrokerBuilder, name: &'static str, base: &'static str) - fn handle_overloads( bb: &mut BrokerBuilder, + wtb: &mut WatchedTasksBuilder, total: CalibratedChannel, port1: CalibratedChannel, port2: CalibratedChannel, @@ -288,7 +297,7 @@ fn handle_overloads( let overload_task = overload.clone(); - spawn(async move { + wtb.spawn_task("usb-hub-overload-state", async move { loop { let overloaded_port = OverloadedPort::from_currents( total.get().map(|m| m.value).unwrap_or(0.0), @@ -309,14 +318,17 @@ fn handle_overloads( impl UsbHub { pub fn new( bb: &mut BrokerBuilder, + wtb: &mut WatchedTasksBuilder, total: CalibratedChannel, port1: CalibratedChannel, port2: CalibratedChannel, port3: CalibratedChannel, ) -> Self { - let overload = handle_overloads(bb, total, port1, port2, port3); + let overload = handle_overloads(bb, wtb, total, port1, port2, port3); - let mut ports = PORTS.iter().map(|(name, base)| handle_port(bb, name, base)); + let mut ports = PORTS + .iter() + .map(|(name, base)| handle_port(bb, wtb, name, base)); Self { overload, diff --git a/src/watchdog.rs b/src/watchdog.rs index 01785be6..5ba06a98 100644 --- a/src/watchdog.rs +++ b/src/watchdog.rs @@ -15,12 +15,13 @@ // with this program; if not, write to the Free Software Foundation, Inc., // 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. -use std::io::{Error, ErrorKind, Result}; use std::time::Duration; +use anyhow::{bail, Result}; use async_std::task::sleep; use crate::dut_power::TickReader; +use crate::watched_tasks::WatchedTasksBuilder; #[cfg(any(test, feature = "demo_mode"))] mod sd { @@ -73,24 +74,23 @@ impl Watchdog { /// - dut_pwr thread - otherwise the tick would not be incremented /// - adc thread - if the adc values are too old dut_pwr_thread will /// not increment the tick. - pub async fn keep_fed(mut self) -> Result<()> { + pub fn keep_fed(mut self, wtb: &mut WatchedTasksBuilder) -> Result<()> { notify(false, [(STATE_READY, "1")].iter())?; - loop { - sleep(self.interval).await; + wtb.spawn_task("watchdog-feeder", async move { + loop { + sleep(self.interval).await; - if self.dut_power_tick.is_stale() { - eprintln!("Power Thread has stalled. Will trigger watchdog."); + if self.dut_power_tick.is_stale() { + notify(false, [(STATE_WATCHDOG, "trigger")].iter())?; - notify(false, [(STATE_WATCHDOG, "trigger")].iter())?; + bail!("Power Thread stalled for too long"); + } - break Err(Error::new( - ErrorKind::TimedOut, - "Power Thread stalled for too long", - )); + notify(false, [(STATE_WATCHDOG, "1")].iter())?; } + }); - notify(false, [(STATE_WATCHDOG, "1")].iter())?; - } + Ok(()) } } diff --git a/src/watched_tasks.rs b/src/watched_tasks.rs new file mode 100644 index 00000000..2c067a50 --- /dev/null +++ b/src/watched_tasks.rs @@ -0,0 +1,149 @@ +use std::future::Future; +use std::pin::Pin; +use std::task::{Context, Poll}; + +use anyhow::{Context as AnyhowContext, Result}; +use async_std::task; +use log::info; + +// This is a wrapper around async_std::task:spawn() that keeps track of the +// tasks it spawned. This solves the problem of error propagation from tasks +// for us. +// When using async_std::task:spawn() you get a handle back that can be used +// to check if and how the task has completed, but there is no common way in +// async_std to have a list of tasks to watch at the same time. +// We want to keep track of the various long running tasks we spawn in the +// tacd and want to propagate errors from back to main(). +// This is what WatchedTasks does. +// +// There are other solutions that do basically the same, but did not quite +// fit our needs: +// +// - async_nursery - Works for async_std but does not look that +// great code-wise. +// - tokio JoinSet - Does roughly the same as WatchedTasks, but +// for tokio (which we do not use). + +type TaskResult = Result<()>; +type TaskHandle = task::JoinHandle; + +pub struct WatchedTasksBuilder { + tasks: Vec, +} + +pub struct WatchedTasks { + tasks: Vec, +} + +impl WatchedTasksBuilder { + pub fn new() -> Self { + Self { tasks: Vec::new() } + } + + /// Spawn an async task that runs until the end of the program + /// + /// If any of the tasks spawned this way returns, the WatchedTasks + /// Future will return the Result of said task. + /// The WatchedTasks Future should be .awaited at the end of main() so + /// that the program ends if any of the watched tasks ends. + pub fn spawn_task(&mut self, name: S, future: F) + where + S: Into, + F: Future + Send + 'static, + { + let task = task::Builder::new() + .name(name.into()) + .spawn(future) + .expect("cannot spawn task"); + + self.tasks.push(task); + } + + /// Complete the task and thread creation and enter the steady state of the program + /// + /// The returned WatchedTasks should be .awaited at the end of `main()` to end the + /// program if any of the watched threads or tasks ends. + pub fn watch(self) -> WatchedTasks { + info!("Spawned {} tasks", self.tasks.len(),); + + WatchedTasks { tasks: self.tasks } + } +} + +impl Future for WatchedTasks { + type Output = TaskResult; + + fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { + for task in self.tasks.iter_mut() { + let name = task.task().name().unwrap_or("").to_owned(); + + if let Poll::Ready(res) = Pin::new(task).poll(cx) { + info!("Task {name} has completed"); + + let res = res.with_context(|| format!("Failed in task {name}")); + + // The first tasks task to finish determines when all + // other should finish as well. + return Poll::Ready(res); + } + } + + Poll::Pending + } +} + +#[cfg(test)] +mod tests { + use std::time::Duration; + + use anyhow::Result; + use async_std::channel::{unbounded, Sender}; + use async_std::future::timeout; + use async_std::task::block_on; + + use super::{TaskResult, WatchedTasks, WatchedTasksBuilder}; + + const TIMEOUT: Duration = Duration::from_millis(100); + + fn setup_tasks() -> (WatchedTasks, Vec>) { + let mut wtb = WatchedTasksBuilder::new(); + + // Spawn ten tasks that each wait for a message on a channel and + // complete if they receive it. + let senders_tasks: Vec<_> = (0..5) + .map(|i| { + let (tx, rx) = unbounded(); + + wtb.spawn_task(format!("task-{i}"), async move { + println!("Hi from task {i}!"); + let res = rx.recv().await?; + println!("Bye from task {i}!"); + res + }); + + tx + }) + .collect(); + + (wtb.watch(), senders_tasks) + } + + #[test] + fn tasks_end_execution() -> Result<()> { + let (mut wt, senders_tasks) = setup_tasks(); + + // At this point none of tasks have completed yet. + // Make sure wt reflects that. + let wt_early_res = block_on(timeout(TIMEOUT, async { (&mut wt).await })); + assert!(wt_early_res.is_err()); + + // Make one of the tasks complete. + senders_tasks[3].try_send(Ok(()))?; + + // Now wt should complete as well. + let wt_late_res = block_on(timeout(TIMEOUT, async { (&mut wt).await })); + assert!(matches!(wt_late_res, Ok(Ok(())))); + + Ok(()) + } +} From 6c7a2cd7fb57b626fb3f8abeac0bf367493eeb81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Leonard=20G=C3=B6hrs?= Date: Mon, 2 Oct 2023 14:25:05 +0200 Subject: [PATCH 2/8] watched_tasks: also handle threads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Execution should stop if any of the important threads ends, not just one of the tasks. Signed-off-by: Leonard Göhrs --- src/adc.rs | 4 +- src/adc/iio/demo_mode.rs | 4 +- src/adc/iio/hardware.rs | 45 ++++--- src/adc/iio/test.rs | 4 +- src/digital_io/gpio/demo_mode.rs | 4 +- src/dut_power.rs | 10 +- src/main.rs | 2 +- src/regulators.rs | 2 +- src/temperatures.rs | 32 ++--- src/ui.rs | 3 +- src/ui/buttons.rs | 16 ++- src/usb_hub.rs | 2 +- src/watched_tasks.rs | 219 ++++++++++++++++++++++++++++--- 13 files changed, 275 insertions(+), 72 deletions(-) diff --git a/src/adc.rs b/src/adc.rs index 1b31b397..f0bdc9be 100644 --- a/src/adc.rs +++ b/src/adc.rs @@ -79,8 +79,8 @@ pub struct Adc { impl Adc { pub async fn new(bb: &mut BrokerBuilder, wtb: &mut WatchedTasksBuilder) -> Result { - let stm32_thread = IioThread::new_stm32().await?; - let powerboard_thread = IioThread::new_powerboard().await?; + let stm32_thread = IioThread::new_stm32(wtb).await?; + let powerboard_thread = IioThread::new_powerboard(wtb).await?; let adc = Self { usb_host_curr: AdcChannel { diff --git a/src/adc/iio/demo_mode.rs b/src/adc/iio/demo_mode.rs index bc8fd4e1..f239ceb6 100644 --- a/src/adc/iio/demo_mode.rs +++ b/src/adc/iio/demo_mode.rs @@ -160,7 +160,7 @@ pub struct IioThread { } impl IioThread { - pub async fn new_stm32() -> Result> { + pub async fn new_stm32(_wtb: &W) -> Result> { let mut demo_magic = block_on(DEMO_MAGIC_STM32.lock()); // Only ever set up a single demo_mode "IioThread" per ADC @@ -195,7 +195,7 @@ impl IioThread { Ok(this) } - pub async fn new_powerboard() -> Result> { + pub async fn new_powerboard(_wtb: &W) -> Result> { let mut demo_magic = block_on(DEMO_MAGIC_POWERBOARD.lock()); // Only ever set up a single demo_mode "IioThread" per ADC diff --git a/src/adc/iio/hardware.rs b/src/adc/iio/hardware.rs index 0095c003..b07f52cf 100644 --- a/src/adc/iio/hardware.rs +++ b/src/adc/iio/hardware.rs @@ -20,9 +20,6 @@ use std::fs::create_dir; use std::io::Read; use std::path::Path; use std::sync::atomic::{AtomicU16, AtomicU64, Ordering}; -use std::sync::Mutex; -use std::thread; -use std::thread::JoinHandle; use std::time::{Duration, Instant}; use anyhow::{anyhow, Context, Error, Result}; @@ -35,6 +32,7 @@ use log::{debug, error, warn}; use thread_priority::*; use crate::measurement::{Measurement, Timestamp}; +use crate::watched_tasks::WatchedTasksBuilder; struct ChannelDesc { kernel_name: &'static str, @@ -255,7 +253,6 @@ pub struct IioThread { ref_instant: Instant, timestamp: AtomicU64, values: Vec, - join: Mutex>>, channel_descs: &'static [ChannelDesc], } @@ -325,7 +322,8 @@ impl IioThread { } async fn new( - thread_name: &str, + wtb: &mut WatchedTasksBuilder, + thread_name: &'static str, adc_name: &'static str, trigger_name: &'static str, sample_rate: i64, @@ -342,9 +340,8 @@ impl IioThread { let (thread_res_tx, thread_res_rx) = bounded(1); // Spawn a high priority thread that updates the atomic values in `thread`. - let join = thread::Builder::new() - .name(format!("tacd {thread_name} iio")) - .spawn(move || { + wtb.spawn_thread(thread_name, move || { + { let adc_setup_res = Self::adc_setup( adc_name, trigger_name, @@ -358,17 +355,15 @@ impl IioThread { ref_instant: Instant::now(), timestamp: AtomicU64::new(TIMESTAMP_ERROR), values: channels.iter().map(|_| AtomicU16::new(0)).collect(), - join: Mutex::new(None), channel_descs, }); - (thread, channels, buf) } Err(e) => { // Can not fail in practice as the queue is known to be empty // at this point. thread_res_tx.try_send(Err(e)).unwrap(); - return; + return Ok(()); } }; @@ -423,21 +418,20 @@ impl IioThread { tx.try_send(Ok(content)).unwrap(); } } - })?; + }; - let thread = thread_res_rx.recv().await??; + Ok(()) + })?; - // Locking the Mutex could only fail if the Mutex was poisoned by - // a thread that held the lock and panicked. - // At this point the Mutex has not yet been locked in another thread. - *thread.join.lock().unwrap() = Some(join); + let thread = thread_res_rx.recv().await??; Ok(thread) } - pub async fn new_stm32() -> Result> { + pub async fn new_stm32(wtb: &mut WatchedTasksBuilder) -> Result> { Self::new( - "stm32", + wtb, + "adc-stm32", "48003000.adc:adc@0", "tim4_trgo", 80, @@ -447,14 +441,23 @@ impl IioThread { .await } - pub async fn new_powerboard() -> Result> { + pub async fn new_powerboard(wtb: &mut WatchedTasksBuilder) -> Result> { let hr_trigger_path = Path::new(TRIGGER_HR_PWR_DIR); if !hr_trigger_path.is_dir() { create_dir(hr_trigger_path)?; } - Self::new("powerboard", "lmp92064", "tacd-pwr", 20, CHANNELS_PWR, 1).await + Self::new( + wtb, + "adc-powerboard", + "lmp92064", + "tacd-pwr", + 20, + CHANNELS_PWR, + 1, + ) + .await } /// Use the channel names defined at the top of the file to get a reference diff --git a/src/adc/iio/test.rs b/src/adc/iio/test.rs index 54371a49..3594083b 100644 --- a/src/adc/iio/test.rs +++ b/src/adc/iio/test.rs @@ -107,7 +107,7 @@ pub struct IioThread { } impl IioThread { - pub async fn new_stm32() -> Result> { + pub async fn new_stm32(_wtb: &W) -> Result> { let mut channels = Vec::new(); for name in CHANNELS_STM32 { @@ -117,7 +117,7 @@ impl IioThread { Ok(Arc::new(Self { channels })) } - pub async fn new_powerboard() -> Result> { + pub async fn new_powerboard(_wtb: &W) -> Result> { let mut channels = Vec::new(); for name in CHANNELS_PWR { diff --git a/src/digital_io/gpio/demo_mode.rs b/src/digital_io/gpio/demo_mode.rs index 1c438a3d..f3b81f57 100644 --- a/src/digital_io/gpio/demo_mode.rs +++ b/src/digital_io/gpio/demo_mode.rs @@ -32,8 +32,8 @@ impl LineHandle { // 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(&())).unwrap(); + let iio_thread_pwr = block_on(IioThread::new_powerboard(&())).unwrap(); match self.name.as_str() { "OUT_0" => iio_thread_stm32 diff --git a/src/dut_power.rs b/src/dut_power.rs index c67d8ca4..477b33ea 100644 --- a/src/dut_power.rs +++ b/src/dut_power.rs @@ -320,9 +320,8 @@ impl DutPwrThread { // Spawn a high priority thread that handles the power status // in a realtimey fashion. - thread::Builder::new() - .name("tacd power".into()) - .spawn(move || { + wtb.spawn_thread("power-thread", move || { + { let mut last_ts: Option = None; // There may be transients in the measured voltage/current, e.g. due to EMI or @@ -482,7 +481,10 @@ impl DutPwrThread { // Make sure to enter fail safe mode before leaving the thread turn_off_with_reason(OutputState::Off, &pwr_line, &discharge_line, &state); - })?; + }; + + Ok(()) + })?; let (tick, request, state) = thread_res_rx.next().await.unwrap()?; diff --git a/src/main.rs b/src/main.rs index 6a9078bc..197d2d72 100644 --- a/src/main.rs +++ b/src/main.rs @@ -82,7 +82,7 @@ async fn init() -> Result<(Ui, HttpServer, WatchedTasksBuilder)> { .await?; let dig_io = DigitalIo::new(&mut bb, &mut wtb, led.out_0.clone(), led.out_1.clone()); let regulators = Regulators::new(&mut bb, &mut wtb); - let temperatures = Temperatures::new(&mut bb); + let temperatures = Temperatures::new(&mut bb, &mut wtb)?; let usb_hub = UsbHub::new( &mut bb, &mut wtb, diff --git a/src/regulators.rs b/src/regulators.rs index 6f47ae66..971dcb39 100644 --- a/src/regulators.rs +++ b/src/regulators.rs @@ -31,7 +31,7 @@ mod reg { pub fn regulator_set(name: &str, state: bool) -> Result<()> { if name == "output_iobus_12v" { - let iio_thread = block_on(IioThread::new_stm32()).unwrap(); + let iio_thread = block_on(IioThread::new_stm32(&())).unwrap(); iio_thread .clone() diff --git a/src/temperatures.rs b/src/temperatures.rs index 500e1f2f..484200f1 100644 --- a/src/temperatures.rs +++ b/src/temperatures.rs @@ -19,34 +19,37 @@ use std::sync::atomic::{AtomicBool, Ordering}; use std::thread::sleep; use std::time::Duration; +use anyhow::Result; use async_std::sync::Arc; -use async_std::task::spawn_blocking; use serde::{Deserialize, Serialize}; use crate::broker::{BrokerBuilder, Topic}; use crate::measurement::Measurement; +use crate::watched_tasks::WatchedTasksBuilder; #[cfg(feature = "demo_mode")] mod hw { + use anyhow::Result; + pub(super) trait SysClass { - fn input(&self) -> Result; + fn input(&self) -> Result; } pub(super) struct HwMon; pub(super) struct TempDecoy; impl SysClass for TempDecoy { - fn input(&self) -> Result { + fn input(&self) -> Result { Ok(30_000) } } impl HwMon { - pub(super) fn new(_: &'static str) -> Result { + pub(super) fn new(_: &'static str) -> Result { Ok(Self) } - pub(super) fn temp(&self, _: u64) -> Result { + pub(super) fn temp(&self, _: u64) -> Result { Ok(TempDecoy) } } @@ -89,7 +92,7 @@ pub struct Temperatures { } impl Temperatures { - pub fn new(bb: &mut BrokerBuilder) -> Self { + pub fn new(bb: &mut BrokerBuilder, wtb: &mut WatchedTasksBuilder) -> Result { let run = Arc::new(AtomicBool::new(true)); let soc_temperature = bb.topic_ro("/v1/tac/temperatures/soc", None); let warning = bb.topic_ro("/v1/tac/temperatures/warning", None); @@ -98,14 +101,9 @@ impl Temperatures { let soc_temperature_thread = soc_temperature.clone(); let warning_thread = warning.clone(); - spawn_blocking(move || { + wtb.spawn_thread("temperature-update", move || { while run_thread.load(Ordering::Relaxed) { - let val = HwMon::new("hwmon0") - .unwrap() - .temp(1) - .unwrap() - .input() - .unwrap(); + let val = HwMon::new("hwmon0")?.temp(1)?.input()?; let val = val as f32 / 1000.0; @@ -120,13 +118,15 @@ impl Temperatures { sleep(UPDATE_INTERVAL); } - }); - Self { + Ok(()) + })?; + + Ok(Self { soc_temperature, warning, run: Some(run), - } + }) } } diff --git a/src/ui.rs b/src/ui.rs index f68e18c1..21366fcd 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -141,9 +141,10 @@ impl Ui { let screens = screens::init(wtb, &res, &alerts, &buttons, &reboot_message, &locator); handle_buttons( + wtb, "/dev/input/by-path/platform-gpio-keys-event", buttons.clone(), - ); + )?; // Blink the status LED when locator is active let led_status_pattern = res.led.status.clone(); diff --git a/src/ui/buttons.rs b/src/ui/buttons.rs index 74567698..1418d2b5 100644 --- a/src/ui/buttons.rs +++ b/src/ui/buttons.rs @@ -17,11 +17,13 @@ use std::time::Duration; +use anyhow::Result; use async_std::sync::Arc; -use async_std::task::{block_on, sleep, spawn, spawn_blocking, JoinHandle}; +use async_std::task::{block_on, sleep, spawn, JoinHandle}; use serde::{Deserialize, Serialize}; use crate::broker::Topic; +use crate::watched_tasks::WatchedTasksBuilder; pub const LONG_PRESS: Duration = Duration::from_millis(500); @@ -133,8 +135,12 @@ impl ButtonEvent { /// Spawn a thread that blockingly reads user input and pushes them into /// a broker framework topic. -pub fn handle_buttons(path: &'static str, topic: Arc>) { - spawn_blocking(move || { +pub fn handle_buttons( + wtb: &mut WatchedTasksBuilder, + path: &'static str, + topic: Arc>, +) -> Result<()> { + wtb.spawn_thread("button-input-thread", move || { let mut device = Device::open(path).unwrap(); let mut press_task: [Option>; 2] = [None, None]; let mut start_time = [None, None]; @@ -179,5 +185,7 @@ pub fn handle_buttons(path: &'static str, topic: Arc>) { } } } - }); + })?; + + Ok(()) } diff --git a/src/usb_hub.rs b/src/usb_hub.rs index fc6ef329..54c885ca 100644 --- a/src/usb_hub.rs +++ b/src/usb_hub.rs @@ -94,7 +94,7 @@ mod rw { for (path_tail, iio_channel) in DISABLE_CHANNELS { if path.ends_with(path_tail) { - let iio_thread = block_on(IioThread::new_stm32()).unwrap(); + let iio_thread = block_on(IioThread::new_stm32(&())).unwrap(); iio_thread .get_channel(iio_channel) diff --git a/src/watched_tasks.rs b/src/watched_tasks.rs index 2c067a50..d1168116 100644 --- a/src/watched_tasks.rs +++ b/src/watched_tasks.rs @@ -1,8 +1,10 @@ use std::future::Future; use std::pin::Pin; -use std::task::{Context, Poll}; +use std::sync::{Arc, Mutex}; +use std::task::{Context, Poll, Waker}; +use std::thread; -use anyhow::{Context as AnyhowContext, Result}; +use anyhow::{anyhow, Context as AnyhowContext, Result}; use async_std::task; use log::info; @@ -14,30 +16,139 @@ use log::info; // async_std to have a list of tasks to watch at the same time. // We want to keep track of the various long running tasks we spawn in the // tacd and want to propagate errors from back to main(). -// This is what WatchedTasks does. +// We also want the same, with a similar API for actual threads instead of +// async tasks. WatchedTasks does both. // // There are other solutions that do basically the same, but did not quite // fit our needs: // -// - async_nursery - Works for async_std but does not look that -// great code-wise. -// - tokio JoinSet - Does roughly the same as WatchedTasks, but -// for tokio (which we do not use). +// - async_nursery - Works for async_std but does not handle actual threads +// and does not look that great code-wise. +// - tokio JoinSet - Does roughly the same as WatchedTasks, but also without +// native thread support and - more importantly for tokio (which we do +// not use). type TaskResult = Result<()>; type TaskHandle = task::JoinHandle; +struct ThreadHandle { + handle: Option>, + wake_on_exit: Arc>>, +} + pub struct WatchedTasksBuilder { tasks: Vec, + threads: Vec, } pub struct WatchedTasks { tasks: Vec, + threads: Vec, +} + +impl ThreadHandle { + fn new(name: String, function: F) -> Result + where + F: FnOnce() -> TaskResult + Send + 'static, + { + let wake_on_exit = Arc::new(Mutex::new(None::)); + let wake_on_exit_thread = wake_on_exit.clone(); + + // We initially used async_std::task::spawn_blocking() here, + // but that does not seem to be intended for long-running threads but instead + // to run blocking operations and get the result of them as a Future. + // There is a maximum amount of threads that can be spawned via spawn_blocking() + // (configurable via an environment variable) and if more tasks are spawned they + // will not start exeuting until enough tasks exit (which in our case they won't). + // We also do configurations like setting realtime priorities for threads, + // which we should not to for threads that are recycled in a thread pool. + // Instead spawn a thread the normal way and handle completion-notifications + // manually. + + let handle = thread::Builder::new().name(name).spawn(move || { + // We could std::panic::catch_unwind() here in the future to handle + // panics inside of spawned threads. + let res = function(); + + // Keep the Mutex locked until exiting the thread to prevent the case + // following race condition: + // + // - Another thread checks if this one ended (which it did not) + // - This thread is about to end and checks wake_on_exit + // - The other thread sets wake_on_exit + let mut wake_on_exit = wake_on_exit_thread + .lock() + .map_err(|_| anyhow!("Tried to lock a tainted Mutex"))?; + + if let Some(waker) = wake_on_exit.take() { + waker.wake(); + } + + res + })?; + + Ok(Self { + handle: Some(handle), + wake_on_exit, + }) + } + + fn name(&self) -> Option<&str> { + self.handle + .as_ref() + .and_then(|handle| handle.thread().name()) + } +} + +impl Future for ThreadHandle { + type Output = TaskResult; + + fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { + { + // Lock the Mutex before checking for thread completion to prevent + // the thread from completing while we are setting up the waker. + let mut wake_on_exit = self + .wake_on_exit + .lock() + .expect("Tried to lock a tainted Mutex"); + + let ready = self + .handle + .as_ref() + .map(|handle| handle.is_finished()) + .unwrap_or(true); + + if !ready { + // The thread is not yet ready. Make sure the current task is polled again + // if the thread becomes ready. + *wake_on_exit = Some(cx.waker().clone()); + + return Poll::Pending; + } + } + + // Get the actual result of the thread via the JoinHandle. + // The handle.join() call is technically blocking, but we know that the + // task has completed from the notification channel, so it isn't in practice. + let res = self + .handle + .take() + .ok_or_else(|| anyhow!("ThreadHandle was already polled to completion")) + .and_then(|handle| match handle.join() { + Ok(r) => r, + Err(_) => Err(anyhow!("Failed to get thread join result")), + }); + + Poll::Ready(res) + } } impl WatchedTasksBuilder { pub fn new() -> Self { - Self { tasks: Vec::new() } + Self { + tasks: Vec::new(), + threads: Vec::new(), + } } /// Spawn an async task that runs until the end of the program @@ -59,14 +170,39 @@ impl WatchedTasksBuilder { self.tasks.push(task); } + /// Spawn a thread that runs until the end of the program + /// + /// If any of the threads spawned this way returns, the WatchedTasks + /// Future will return the Result of said thread. + /// The WatchedTasks Future should be .awaited at the end of main() so + /// that the program ends if any of the watched threads ends. + pub fn spawn_thread(&mut self, name: S, function: F) -> Result<()> + where + S: Into, + F: FnOnce() -> TaskResult + Send + 'static, + { + let thread = ThreadHandle::new(name.into(), function)?; + + self.threads.push(thread); + + Ok(()) + } + /// Complete the task and thread creation and enter the steady state of the program /// /// The returned WatchedTasks should be .awaited at the end of `main()` to end the /// program if any of the watched threads or tasks ends. pub fn watch(self) -> WatchedTasks { - info!("Spawned {} tasks", self.tasks.len(),); + info!( + "Spawned {} tasks and {} threads", + self.tasks.len(), + self.threads.len() + ); - WatchedTasks { tasks: self.tasks } + WatchedTasks { + tasks: self.tasks, + threads: self.threads, + } } } @@ -82,8 +218,20 @@ impl Future for WatchedTasks { let res = res.with_context(|| format!("Failed in task {name}")); - // The first tasks task to finish determines when all - // other should finish as well. + // The first task to finish determines when all other should finish as well. + return Poll::Ready(res); + } + } + + for thread in self.threads.iter_mut() { + let name = thread.name().unwrap_or("").to_owned(); + + if let Poll::Ready(res) = Pin::new(thread).poll(cx) { + info!("Thread {name} has completed"); + + let res = res.with_context(|| format!("Failed in thread {name}")); + + // The first thread to finish determines when all other should finish as well. return Poll::Ready(res); } } @@ -105,7 +253,11 @@ mod tests { const TIMEOUT: Duration = Duration::from_millis(100); - fn setup_tasks() -> (WatchedTasks, Vec>) { + fn setup_tasks_and_threads() -> ( + WatchedTasks, + Vec>, + Vec>, + ) { let mut wtb = WatchedTasksBuilder::new(); // Spawn ten tasks that each wait for a message on a channel and @@ -125,12 +277,30 @@ mod tests { }) .collect(); - (wtb.watch(), senders_tasks) + // Spawn ten tasks that each wait for a message on a channel and + // complete if they receive it. + let senders_threads: Vec<_> = (0..5) + .map(|i| { + let (tx, rx) = unbounded(); + + wtb.spawn_thread(format!("thread-{i}"), move || { + println!("Hi from thread {i}!"); + let res = block_on(rx.recv())?; + println!("Bye from thread {i}!"); + res + }) + .unwrap(); + + tx + }) + .collect(); + + (wtb.watch(), senders_tasks, senders_threads) } #[test] fn tasks_end_execution() -> Result<()> { - let (mut wt, senders_tasks) = setup_tasks(); + let (mut wt, senders_tasks, _senders_threads) = setup_tasks_and_threads(); // At this point none of tasks have completed yet. // Make sure wt reflects that. @@ -146,4 +316,23 @@ mod tests { Ok(()) } + + #[test] + fn threads_end_execution() -> Result<()> { + let (mut wt, _senders_tasks, senders_threads) = setup_tasks_and_threads(); + + // At this point none of threads have completed yet. + // Make sure wt reflects that. + let wt_early_res = block_on(timeout(TIMEOUT, async { (&mut wt).await })); + assert!(wt_early_res.is_err()); + + // Make one of the threads complete. + senders_threads[3].try_send(Ok(()))?; + + // Now wt should complete as well. + let wt_late_res = block_on(timeout(TIMEOUT, async { (&mut wt).await })); + assert!(matches!(wt_late_res, Ok(Ok(())))); + + Ok(()) + } } From 41a3b5735ee0f01c96d037a1f7aff4b531d9c667 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Leonard=20G=C3=B6hrs?= Date: Mon, 2 Oct 2023 14:26:17 +0200 Subject: [PATCH 3/8] dut_power: remove indentation hack and let cargo fmt do its thing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The large re-indentation would have made the commit that adds the wtb.spawn_thread() call rather unreadable, as it mixes formatting and code changes. Instead add this commit that _only_ changes formatting. Signed-off-by: Leonard Göhrs --- src/dut_power.rs | 284 +++++++++++++++++++++++------------------------ 1 file changed, 141 insertions(+), 143 deletions(-) diff --git a/src/dut_power.rs b/src/dut_power.rs index 477b33ea..b5fd6444 100644 --- a/src/dut_power.rs +++ b/src/dut_power.rs @@ -321,167 +321,165 @@ impl DutPwrThread { // Spawn a high priority thread that handles the power status // in a realtimey fashion. wtb.spawn_thread("power-thread", move || { - { - let mut last_ts: Option = None; - - // There may be transients in the measured voltage/current, e.g. due to EMI or - // inrush currents. - // Nothing will break if they are sufficiently short, so the DUT can stay powered. - // Filter out transients by taking the last four values, throwing away the largest - // and smallest and averaging the two remaining ones. - let mut volt_filter = MedianFilter::<4>::new(); - let mut curr_filter = MedianFilter::<4>::new(); - - let (tick_weak, request, state) = match realtime_priority() { - Ok(_) => { - let tick = Arc::new(AtomicU32::new(0)); - let tick_weak = Arc::downgrade(&tick); - - let request = Arc::new(AtomicU8::new(OutputRequest::Idle as u8)); - let state = Arc::new(AtomicU8::new(OutputState::Off as u8)); - - thread_res_tx - .try_send(Ok((tick, request.clone(), state.clone()))) - .unwrap(); + let mut last_ts: Option = None; - (tick_weak, request, state) - } - Err(e) => { - thread_res_tx.try_send(Err(e)).unwrap(); - panic!() - } - }; + // There may be transients in the measured voltage/current, e.g. due to EMI or + // inrush currents. + // Nothing will break if they are sufficiently short, so the DUT can stay powered. + // Filter out transients by taking the last four values, throwing away the largest + // and smallest and averaging the two remaining ones. + let mut volt_filter = MedianFilter::<4>::new(); + let mut curr_filter = MedianFilter::<4>::new(); - // Run as long as there is a strong reference to `tick`. - // As tick is a private member of the struct this is equivalent - // to running as long as the DutPwrThread was not dropped. - while let Some(tick) = tick_weak.upgrade() { - thread::sleep(THREAD_INTERVAL); - - // Get new voltage and current readings while making sure - // that they are not stale - let (volt, curr) = loop { - let feedback = pwr_volt - .fast - .try_get_multiple([&pwr_volt.fast, &pwr_curr.fast]); - - // We do not care too much about _why_ we could not get - // a new value from the ADC. - // If we get a new valid value before the timeout we - // are fine. - // If not we are not. - if let Ok(m) = feedback { - last_ts = Some(m[0].ts.as_instant()); - } - - let too_old = last_ts - .map(|ts| Instant::now().duration_since(ts) > MAX_AGE) - .unwrap_or(false); - - if too_old { - turn_off_with_reason( - OutputState::RealtimeViolation, - &pwr_line, - &discharge_line, - &state, - ); - } else { - // We have a fresh ADC value. Signal "everything is well" - // to the watchdog task. - tick.fetch_add(1, Ordering::Relaxed); - } - - if let Ok(m) = feedback { - break (m[0].value, m[1].value); - } - }; - - // The median filter needs some values in it's backlog before it - // starts outputting values. - let (volt, curr) = match (volt_filter.step(volt), curr_filter.step(curr)) { - (Some(volt), Some(curr)) => (volt, curr), - _ => continue, - }; - - // Take the next pending OutputRequest (if any) even if it - // may not be used due to a pending error condition, as it - // could be quite surprising for the output to turn on - // immediately when a fault is cleared after quite some time - // of the output being off. - let req = request - .swap(OutputRequest::Idle as u8, Ordering::Relaxed) - .into(); - - // Don't even look at the requests if there is an ongoing - // overvoltage condition. Instead turn the output off and - // go back to measuring. - if volt > MAX_VOLTAGE { - turn_off_with_reason( - OutputState::OverVoltage, - &pwr_line, - &discharge_line, - &state, - ); + let (tick_weak, request, state) = match realtime_priority() { + Ok(_) => { + let tick = Arc::new(AtomicU32::new(0)); + let tick_weak = Arc::downgrade(&tick); - continue; - } + let request = Arc::new(AtomicU8::new(OutputRequest::Idle as u8)); + let state = Arc::new(AtomicU8::new(OutputState::Off as u8)); - // Don't even look at the requests if there is an ongoin - // polarity inversion. Turn off, go back to start, do not - // collect $200. - if volt < MIN_VOLTAGE { - turn_off_with_reason( - OutputState::InvertedPolarity, - &pwr_line, - &discharge_line, - &state, - ); + thread_res_tx + .try_send(Ok((tick, request.clone(), state.clone()))) + .unwrap(); + + (tick_weak, request, state) + } + Err(e) => { + thread_res_tx.try_send(Err(e)).unwrap(); + panic!() + } + }; - continue; + // Run as long as there is a strong reference to `tick`. + // As tick is a private member of the struct this is equivalent + // to running as long as the DutPwrThread was not dropped. + while let Some(tick) = tick_weak.upgrade() { + thread::sleep(THREAD_INTERVAL); + + // Get new voltage and current readings while making sure + // that they are not stale + let (volt, curr) = loop { + let feedback = pwr_volt + .fast + .try_get_multiple([&pwr_volt.fast, &pwr_curr.fast]); + + // We do not care too much about _why_ we could not get + // a new value from the ADC. + // If we get a new valid value before the timeout we + // are fine. + // If not we are not. + if let Ok(m) = feedback { + last_ts = Some(m[0].ts.as_instant()); } - // Don't even look at the requests if there is an ongoin - // overcurrent condition. - if curr > MAX_CURRENT { + let too_old = last_ts + .map(|ts| Instant::now().duration_since(ts) > MAX_AGE) + .unwrap_or(false); + + if too_old { turn_off_with_reason( - OutputState::OverCurrent, + OutputState::RealtimeViolation, &pwr_line, &discharge_line, &state, ); + } else { + // We have a fresh ADC value. Signal "everything is well" + // to the watchdog task. + tick.fetch_add(1, Ordering::Relaxed); + } - continue; + if let Ok(m) = feedback { + break (m[0].value, m[1].value); } + }; + + // The median filter needs some values in it's backlog before it + // starts outputting values. + let (volt, curr) = match (volt_filter.step(volt), curr_filter.step(curr)) { + (Some(volt), Some(curr)) => (volt, curr), + _ => continue, + }; + + // Take the next pending OutputRequest (if any) even if it + // may not be used due to a pending error condition, as it + // could be quite surprising for the output to turn on + // immediately when a fault is cleared after quite some time + // of the output being off. + let req = request + .swap(OutputRequest::Idle as u8, Ordering::Relaxed) + .into(); + + // Don't even look at the requests if there is an ongoing + // overvoltage condition. Instead turn the output off and + // go back to measuring. + if volt > MAX_VOLTAGE { + turn_off_with_reason( + OutputState::OverVoltage, + &pwr_line, + &discharge_line, + &state, + ); + + continue; + } + + // Don't even look at the requests if there is an ongoin + // polarity inversion. Turn off, go back to start, do not + // collect $200. + if volt < MIN_VOLTAGE { + turn_off_with_reason( + OutputState::InvertedPolarity, + &pwr_line, + &discharge_line, + &state, + ); + + continue; + } + + // Don't even look at the requests if there is an ongoin + // overcurrent condition. + if curr > MAX_CURRENT { + turn_off_with_reason( + OutputState::OverCurrent, + &pwr_line, + &discharge_line, + &state, + ); + + continue; + } - // There is no ongoing fault condition, so we could e.g. turn - // the output on if requested. - match req { - OutputRequest::Idle => {} - OutputRequest::On => { - discharge_line - .set_value(1 - DISCHARGE_LINE_ASSERTED) - .unwrap(); - pwr_line.set_value(PWR_LINE_ASSERTED).unwrap(); - state.store(OutputState::On as u8, Ordering::Relaxed); - } - OutputRequest::Off => { - discharge_line.set_value(DISCHARGE_LINE_ASSERTED).unwrap(); - pwr_line.set_value(1 - PWR_LINE_ASSERTED).unwrap(); - state.store(OutputState::Off as u8, Ordering::Relaxed); - } - OutputRequest::OffFloating => { - discharge_line - .set_value(1 - DISCHARGE_LINE_ASSERTED) - .unwrap(); - pwr_line.set_value(1 - PWR_LINE_ASSERTED).unwrap(); - state.store(OutputState::OffFloating as u8, Ordering::Relaxed); - } + // There is no ongoing fault condition, so we could e.g. turn + // the output on if requested. + match req { + OutputRequest::Idle => {} + OutputRequest::On => { + discharge_line + .set_value(1 - DISCHARGE_LINE_ASSERTED) + .unwrap(); + pwr_line.set_value(PWR_LINE_ASSERTED).unwrap(); + state.store(OutputState::On as u8, Ordering::Relaxed); + } + OutputRequest::Off => { + discharge_line.set_value(DISCHARGE_LINE_ASSERTED).unwrap(); + pwr_line.set_value(1 - PWR_LINE_ASSERTED).unwrap(); + state.store(OutputState::Off as u8, Ordering::Relaxed); + } + OutputRequest::OffFloating => { + discharge_line + .set_value(1 - DISCHARGE_LINE_ASSERTED) + .unwrap(); + pwr_line.set_value(1 - PWR_LINE_ASSERTED).unwrap(); + state.store(OutputState::OffFloating as u8, Ordering::Relaxed); } } + } - // Make sure to enter fail safe mode before leaving the thread - turn_off_with_reason(OutputState::Off, &pwr_line, &discharge_line, &state); - }; + // Make sure to enter fail safe mode before leaving the thread + turn_off_with_reason(OutputState::Off, &pwr_line, &discharge_line, &state); Ok(()) })?; From 52de8ae53738de21275a11ffde630ce28534fc6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Leonard=20G=C3=B6hrs?= Date: Mon, 2 Oct 2023 14:28:09 +0200 Subject: [PATCH 4/8] adc: hardware: remove indentation hack and let cargo fmt do its thing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The large re-indentation would have made the commit that adds the wtb.spawn_thread() call rather unreadable, as it mixes formatting and code changes. Instead add this commit that _only_ changes formatting. Signed-off-by: Leonard Göhrs --- src/adc/iio/hardware.rs | 137 ++++++++++++++++++++-------------------- 1 file changed, 68 insertions(+), 69 deletions(-) diff --git a/src/adc/iio/hardware.rs b/src/adc/iio/hardware.rs index b07f52cf..d3bf8441 100644 --- a/src/adc/iio/hardware.rs +++ b/src/adc/iio/hardware.rs @@ -341,84 +341,83 @@ impl IioThread { // Spawn a high priority thread that updates the atomic values in `thread`. wtb.spawn_thread(thread_name, move || { - { - let adc_setup_res = Self::adc_setup( - adc_name, - trigger_name, - sample_rate, - channel_descs, - buffer_len, - ); - let (thread, channels, mut buf) = match adc_setup_res { - Ok((channels, buf)) => { - let thread = Arc::new(Self { - ref_instant: Instant::now(), - timestamp: AtomicU64::new(TIMESTAMP_ERROR), - values: channels.iter().map(|_| AtomicU16::new(0)).collect(), - channel_descs, - }); - (thread, channels, buf) - } - Err(e) => { - // Can not fail in practice as the queue is known to be empty - // at this point. - thread_res_tx.try_send(Err(e)).unwrap(); - return Ok(()); - } - }; - - let thread_weak = Arc::downgrade(&thread); - let mut signal_ready = Some((thread, thread_res_tx)); - - // Stop running as soon as the last reference to this Arc - // is dropped (e.g. the weak reference can no longer be upgraded). - while let Some(thread) = thread_weak.upgrade() { - if let Err(e) = buf.refill() { - thread.timestamp.store(TIMESTAMP_ERROR, Ordering::Relaxed); - - error!("Failed to refill {} ADC buffer: {}", adc_name, e); - - // If the ADC has not yet produced any values we still have the - // queue at hand that signals readiness to the main thread. - // This gives us a chance to return an Err from new(). - // If the queue was already used just print an error instead. - if let Some((_, tx)) = signal_ready.take() { - // Can not fail in practice as the queue is only .take()n - // once and thus known to be empty. - tx.try_send(Err(Error::new(e))).unwrap(); - } - - break; - } - - let values = channels.iter().map(|ch| { - let buf_sum: u32 = buf.channel_iter::(ch).map(|v| v as u32).sum(); - (buf_sum / (buf.capacity() as u32)) as u16 + let adc_setup_res = Self::adc_setup( + adc_name, + trigger_name, + sample_rate, + channel_descs, + buffer_len, + ); + let (thread, channels, mut buf) = match adc_setup_res { + Ok((channels, buf)) => { + let thread = Arc::new(Self { + ref_instant: Instant::now(), + timestamp: AtomicU64::new(TIMESTAMP_ERROR), + values: channels.iter().map(|_| AtomicU16::new(0)).collect(), + channel_descs, }); - for (d, s) in thread.values.iter().zip(values) { - d.store(s, Ordering::Relaxed) - } + (thread, channels, buf) + } + Err(e) => { + // Can not fail in practice as the queue is known to be empty + // at this point. + thread_res_tx.try_send(Err(e)).unwrap(); + return Ok(()); + } + }; + + let thread_weak = Arc::downgrade(&thread); + let mut signal_ready = Some((thread, thread_res_tx)); - // These should only fail if - // a) The monotonic time started running backward - // b) The tacd has been running for more than 2**64ns (584 years). - let ts: u64 = Instant::now() - .checked_duration_since(thread.ref_instant) - .and_then(|d| d.as_nanos().try_into().ok()) - .unwrap_or(TIMESTAMP_ERROR); + // Stop running as soon as the last reference to this Arc + // is dropped (e.g. the weak reference can no longer be upgraded). + while let Some(thread) = thread_weak.upgrade() { + if let Err(e) = buf.refill() { + thread.timestamp.store(TIMESTAMP_ERROR, Ordering::Relaxed); - thread.timestamp.store(ts, Ordering::Release); + error!("Failed to refill {} ADC buffer: {}", adc_name, e); - // Now that we know that the ADC actually works and we have - // initial values: return a handle to it. - if let Some((content, tx)) = signal_ready.take() { + // If the ADC has not yet produced any values we still have the + // queue at hand that signals readiness to the main thread. + // This gives us a chance to return an Err from new(). + // If the queue was already used just print an error instead. + if let Some((_, tx)) = signal_ready.take() { // Can not fail in practice as the queue is only .take()n // once and thus known to be empty. - tx.try_send(Ok(content)).unwrap(); + tx.try_send(Err(Error::new(e))).unwrap(); } + + break; } - }; + + let values = channels.iter().map(|ch| { + let buf_sum: u32 = buf.channel_iter::(ch).map(|v| v as u32).sum(); + (buf_sum / (buf.capacity() as u32)) as u16 + }); + + for (d, s) in thread.values.iter().zip(values) { + d.store(s, Ordering::Relaxed) + } + + // These should only fail if + // a) The monotonic time started running backward + // b) The tacd has been running for more than 2**64ns (584 years). + let ts: u64 = Instant::now() + .checked_duration_since(thread.ref_instant) + .and_then(|d| d.as_nanos().try_into().ok()) + .unwrap_or(TIMESTAMP_ERROR); + + thread.timestamp.store(ts, Ordering::Release); + + // Now that we know that the ADC actually works and we have + // initial values: return a handle to it. + if let Some((content, tx)) = signal_ready.take() { + // Can not fail in practice as the queue is only .take()n + // once and thus known to be empty. + tx.try_send(Ok(content)).unwrap(); + } + } Ok(()) })?; From dc802bd1d74de1d9012d51803aba626f1d50f19a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Leonard=20G=C3=B6hrs?= Date: Fri, 6 Oct 2023 10:25:25 +0200 Subject: [PATCH 5/8] watched_tasks: let spawn_task return Result instead of panicking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Leonard Göhrs --- src/adc.rs | 2 +- src/backlight.rs | 2 +- src/broker.rs | 7 ++++-- src/broker/persistence.rs | 4 +-- src/dbus.rs | 23 ++++++++--------- src/dbus/hostname.rs | 21 ++++++++++------ src/dbus/networkmanager.rs | 18 +++++++------- src/dbus/rauc.rs | 20 +++++++-------- src/dbus/systemd.rs | 36 ++++++++++++++++----------- src/digital_io.rs | 22 ++++++++-------- src/dut_power.rs | 16 ++++++------ src/http_server.rs | 5 ++-- src/iobus.rs | 9 ++++--- src/led.rs | 33 ++++++++++++------------ src/main.rs | 20 +++++++-------- src/regulators.rs | 17 +++++++------ src/setup_mode.rs | 15 +++++++---- src/ui.rs | 8 +++--- src/ui/screens.rs | 33 +++++++++++++----------- src/ui/screens/help.rs | 7 +++--- src/ui/screens/iobus_health.rs | 7 +++--- src/ui/screens/locator.rs | 7 +++--- src/ui/screens/overtemperature.rs | 7 +++--- src/ui/screens/power_fail.rs | 7 +++--- src/ui/screens/reboot.rs | 7 +++--- src/ui/screens/screensaver.rs | 7 +++--- src/ui/screens/setup.rs | 7 +++--- src/ui/screens/update_available.rs | 7 +++--- src/ui/screens/update_installation.rs | 9 ++++--- src/ui/screens/usb_overload.rs | 7 +++--- src/usb_hub.rs | 35 +++++++++++++++----------- src/watchdog.rs | 2 +- src/watched_tasks.rs | 12 ++++----- 33 files changed, 243 insertions(+), 196 deletions(-) diff --git a/src/adc.rs b/src/adc.rs index f0bdc9be..bb0f3734 100644 --- a/src/adc.rs +++ b/src/adc.rs @@ -227,7 +227,7 @@ impl Adc { time.set(Timestamp::now()); } - }); + })?; Ok(adc) } diff --git a/src/backlight.rs b/src/backlight.rs index 57c0ff29..9bec66f7 100644 --- a/src/backlight.rs +++ b/src/backlight.rs @@ -62,7 +62,7 @@ impl Backlight { } Ok(()) - }); + })?; Ok(Self { brightness }) } diff --git a/src/broker.rs b/src/broker.rs index b68ca90a..b023b541 100644 --- a/src/broker.rs +++ b/src/broker.rs @@ -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::Result; use async_std::sync::Arc; use serde::{de::DeserializeOwned, Serialize}; @@ -115,11 +116,13 @@ impl BrokerBuilder { /// Finish building the broker /// /// This consumes the builder so that no new topics can be registered - pub fn build(self, wtb: &mut WatchedTasksBuilder, server: &mut tide::Server<()>) { + pub fn build(self, wtb: &mut WatchedTasksBuilder, server: &mut tide::Server<()>) -> Result<()> { let topics = Arc::new(self.topics); - persistence::register(wtb, topics.clone()); + persistence::register(wtb, topics.clone())?; rest::register(server, topics.clone()); mqtt_conn::register(server, topics); + + Ok(()) } } diff --git a/src/broker/persistence.rs b/src/broker/persistence.rs index 3f1543b1..8367ba82 100644 --- a/src/broker/persistence.rs +++ b/src/broker/persistence.rs @@ -148,7 +148,7 @@ async fn save_on_change( Ok(()) } -pub fn register(wtb: &mut WatchedTasksBuilder, topics: Arc>>) { +pub fn register(wtb: &mut WatchedTasksBuilder, topics: Arc>>) -> Result<()> { load(&topics).unwrap(); let (tx, rx) = unbounded(); @@ -157,5 +157,5 @@ pub fn register(wtb: &mut WatchedTasksBuilder, topics: Arc topic.subscribe_as_bytes(tx.clone(), false); } - wtb.spawn_task("persistence-save", save_on_change(topics, rx)); + wtb.spawn_task("persistence-save", save_on_change(topics, rx)) } diff --git a/src/dbus.rs b/src/dbus.rs index 393a74ec..837752f0 100644 --- a/src/dbus.rs +++ b/src/dbus.rs @@ -23,7 +23,7 @@ use crate::watched_tasks::WatchedTasksBuilder; #[cfg(feature = "demo_mode")] mod zb { - pub(super) type Result = std::result::Result; + pub(super) use anyhow::Result; pub struct Connection; pub struct ConnectionBuilder; @@ -82,21 +82,18 @@ impl DbusSession { wtb: &mut WatchedTasksBuilder, led_dut: Arc>, led_uplink: Arc>, - ) -> Self { + ) -> anyhow::Result { 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 { - hostname: Hostname::new(bb, wtb, &conn), - network: Network::new(bb, wtb, &conn, led_dut, led_uplink), - rauc: Rauc::new(bb, wtb, &conn), - systemd: Systemd::new(bb, wtb, &conn).await, - } + Ok(Self { + hostname: Hostname::new(bb, wtb, &conn)?, + network: Network::new(bb, wtb, &conn, led_dut, led_uplink)?, + rauc: Rauc::new(bb, wtb, &conn)?, + systemd: Systemd::new(bb, wtb, &conn).await?, + }) } } diff --git a/src/dbus/hostname.rs b/src/dbus/hostname.rs index e51e257d..ae58bfb6 100644 --- a/src/dbus/hostname.rs +++ b/src/dbus/hostname.rs @@ -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; + #[cfg(not(feature = "demo_mode"))] use async_std::stream::StreamExt; #[cfg(not(feature = "demo_mode"))] use zbus::Connection; -use async_std::sync::Arc; - use crate::broker::{BrokerBuilder, Topic}; use crate::watched_tasks::WatchedTasksBuilder; @@ -34,10 +35,14 @@ pub struct Hostname { impl Hostname { #[cfg(feature = "demo_mode")] - pub fn new(bb: &mut BrokerBuilder, _wtb: &mut WatchedTasksBuilder, _conn: C) -> Self { - Self { + pub fn new( + bb: &mut BrokerBuilder, + _wtb: &mut WatchedTasksBuilder, + _conn: C, + ) -> Result { + Ok(Self { hostname: bb.topic_ro("/v1/tac/network/hostname", Some("lxatac".into())), - } + }) } #[cfg(not(feature = "demo_mode"))] @@ -45,7 +50,7 @@ impl Hostname { bb: &mut BrokerBuilder, wtb: &mut WatchedTasksBuilder, conn: &Arc, - ) -> Self { + ) -> Result { let hostname = bb.topic_ro("/v1/tac/network/hostname", None); let conn = conn.clone(); @@ -67,8 +72,8 @@ impl Hostname { } Ok(()) - }); + })?; - Self { hostname } + Ok(Self { hostname }) } } diff --git a/src/dbus/networkmanager.rs b/src/dbus/networkmanager.rs index ee97a933..99d424bd 100644 --- a/src/dbus/networkmanager.rs +++ b/src/dbus/networkmanager.rs @@ -15,9 +15,9 @@ // 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; use async_std::sync::Arc; - use serde::{Deserialize, Serialize}; use crate::broker::{BrokerBuilder, Topic}; @@ -39,7 +39,7 @@ mod manager; // Put them inside a mod so we do not have to decorate each one with #[cfg(not(feature = "demo_mode"))] mod optional_includes { - pub(super) use anyhow::{anyhow, Result}; + pub(super) use anyhow::anyhow; pub(super) use async_std::stream::StreamExt; pub(super) use async_std::task::sleep; pub(super) use futures::{future::FutureExt, select}; @@ -249,7 +249,7 @@ impl Network { _conn: C, _led_dut: Arc>, _led_uplink: Arc>, - ) -> Self { + ) -> Result { let this = Self::setup_topics(bb); this.bridge_interface.set(vec![String::from("192.168.1.1")]); @@ -262,7 +262,7 @@ impl Network { carrier: true, }); - this + Ok(this) } #[cfg(not(feature = "demo_mode"))] @@ -272,27 +272,27 @@ impl Network { conn: &Arc, led_dut: Arc>, led_uplink: Arc>, - ) -> Self { + ) -> Result { let this = Self::setup_topics(bb); let conn_task = conn.clone(); let dut_interface = this.dut_interface.clone(); wtb.spawn_task("link-dut-update", async move { handle_link_updates(&conn_task, dut_interface, "dut", led_dut).await - }); + })?; let conn_task = conn.clone(); let uplink_interface = this.uplink_interface.clone(); wtb.spawn_task("link-uplink-update", async move { handle_link_updates(&conn_task, uplink_interface, "uplink", led_uplink).await - }); + })?; let conn_task = conn.clone(); let bridge_interface = this.bridge_interface.clone(); wtb.spawn_task("ip-tac-bridge-update", async move { handle_ipv4_updates(&conn_task, bridge_interface, "tac-bridge").await - }); + })?; - this + Ok(this) } } diff --git a/src/dbus/rauc.rs b/src/dbus/rauc.rs index 8bd11622..f4844ae8 100644 --- a/src/dbus/rauc.rs +++ b/src/dbus/rauc.rs @@ -318,7 +318,7 @@ impl Rauc { bb: &mut BrokerBuilder, wtb: &mut WatchedTasksBuilder, _conn: &Arc, - ) -> Self { + ) -> Result { let inst = Self::setup_topics(bb); inst.operation.set("idle".to_string()); @@ -336,9 +336,9 @@ impl Rauc { inst.channels.clone(), inst.slot_status.clone(), ), - ); + )?; - inst + Ok(inst) } #[cfg(not(feature = "demo_mode"))] @@ -346,7 +346,7 @@ impl Rauc { bb: &mut BrokerBuilder, wtb: &mut WatchedTasksBuilder, conn: &Arc, - ) -> Self { + ) -> Result { let inst = Self::setup_topics(bb); let conn_task = conn.clone(); @@ -455,7 +455,7 @@ impl Rauc { break Ok(()); } } - }); + })?; let conn_task = conn.clone(); let progress = inst.progress.clone(); @@ -477,7 +477,7 @@ impl Rauc { } Ok(()) - }); + })?; let conn_task = conn.clone(); let last_error = inst.last_error.clone(); @@ -499,7 +499,7 @@ impl Rauc { } Ok(()) - }); + })?; let conn_task = conn.clone(); let (mut install_stream, _) = inst.install.clone().subscribe_unbounded(); @@ -519,7 +519,7 @@ impl Rauc { } Ok(()) - }); + })?; // Reload the channel list on request let (reload_stream, _) = inst.reload.clone().subscribe_unbounded(); @@ -532,9 +532,9 @@ impl Rauc { inst.channels.clone(), inst.slot_status.clone(), ), - ); + )?; - inst + Ok(inst) } } diff --git a/src/dbus/systemd.rs b/src/dbus/systemd.rs index 056094e7..85490a88 100644 --- a/src/dbus/systemd.rs +++ b/src/dbus/systemd.rs @@ -100,8 +100,10 @@ impl Service { _wtb: &mut WatchedTasksBuilder, _conn: Arc, _unit_name: &str, - ) { + ) -> anyhow::Result<()> { self.status.set(ServiceStatus::get().await.unwrap()); + + Ok(()) } #[cfg(not(feature = "demo_mode"))] @@ -110,7 +112,7 @@ impl Service { wtb: &mut WatchedTasksBuilder, conn: Arc, unit_name: &'static str, - ) { + ) -> anyhow::Result<()> { let unit_path = { let manager = manager::ManagerProxy::new(&conn).await.unwrap(); manager.get_unit(unit_name).await.unwrap() @@ -150,7 +152,7 @@ impl Service { .await .unwrap(); } - }); + })?; let (mut action_reqs, _) = self.action.clone().subscribe_unbounded(); @@ -171,7 +173,9 @@ impl Service { } Ok(()) - }); + })?; + + Ok(()) } } @@ -181,7 +185,7 @@ impl Systemd { wtb: &mut WatchedTasksBuilder, reboot: Arc>, _conn: Arc, - ) { + ) -> anyhow::Result<()> { let (mut reboot_reqs, _) = reboot.subscribe_unbounded(); wtb.spawn_task("systemd-reboot", async move { @@ -192,7 +196,7 @@ impl Systemd { } Ok(()) - }); + }) } #[cfg(not(feature = "demo_mode"))] @@ -200,7 +204,7 @@ impl Systemd { wtb: &mut WatchedTasksBuilder, reboot: Arc>, conn: Arc, - ) { + ) -> anyhow::Result<()> { let (mut reboot_reqs, _) = reboot.subscribe_unbounded(); wtb.spawn_task("systemd-reboot", async move { @@ -215,17 +219,17 @@ impl Systemd { } Ok(()) - }); + }) } pub async fn new( bb: &mut BrokerBuilder, wtb: &mut WatchedTasksBuilder, conn: &Arc, - ) -> Self { + ) -> anyhow::Result { let reboot = bb.topic_rw("/v1/tac/reboot", Some(false)); - Self::handle_reboot(wtb, reboot.clone(), conn.clone()); + Self::handle_reboot(wtb, reboot.clone(), conn.clone())?; let networkmanager = Service::new(bb, "network-manager"); let labgrid = Service::new(bb, "labgrid-exporter"); @@ -233,17 +237,19 @@ impl Systemd { networkmanager .connect(wtb, conn.clone(), "NetworkManager.service") - .await; + .await?; labgrid .connect(wtb, conn.clone(), "labgrid-exporter.service") - .await; - iobus.connect(wtb, conn.clone(), "lxa-iobus.service").await; + .await?; + iobus + .connect(wtb, conn.clone(), "lxa-iobus.service") + .await?; - Self { + Ok(Self { reboot, networkmanager, labgrid, iobus, - } + }) } } diff --git a/src/digital_io.rs b/src/digital_io.rs index 45086387..ff859832 100644 --- a/src/digital_io.rs +++ b/src/digital_io.rs @@ -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::Result; use async_std::prelude::*; use async_std::sync::Arc; @@ -60,7 +61,7 @@ fn handle_line_wo( initial: bool, inverted: bool, led_topic: Option>>, -) -> Arc> { +) -> Result>> { let topic = bb.topic_rw(path, Some(initial)); let line = find_line(line_name).unwrap(); let dst = line @@ -80,9 +81,9 @@ fn handle_line_wo( } Ok(()) - }); + })?; - topic + Ok(topic) } impl DigitalIo { @@ -91,7 +92,7 @@ impl DigitalIo { wtb: &mut WatchedTasksBuilder, led_0: Arc>, led_1: Arc>, - ) -> Self { + ) -> Result { let out_0 = handle_line_wo( bb, wtb, @@ -100,7 +101,7 @@ impl DigitalIo { false, false, Some(led_0), - ); + )?; let out_1 = handle_line_wo( bb, @@ -110,7 +111,7 @@ impl DigitalIo { false, false, Some(led_1), - ); + )?; let uart_rx_en = handle_line_wo( bb, @@ -120,7 +121,8 @@ impl DigitalIo { true, true, None, - ); + )?; + let uart_tx_en = handle_line_wo( bb, wtb, @@ -129,13 +131,13 @@ impl DigitalIo { true, true, None, - ); + )?; - Self { + Ok(Self { out_0, out_1, uart_rx_en, uart_tx_en, - } + }) } } diff --git a/src/dut_power.rs b/src/dut_power.rs index b5fd6444..3231436c 100644 --- a/src/dut_power.rs +++ b/src/dut_power.rs @@ -253,7 +253,7 @@ fn setup_labgrid_compat( wtb: &mut WatchedTasksBuilder, request: Arc>, state: Arc>, -) { +) -> Result<()> { let compat_request = bb.topic_wo::("/v1/dut/powered/compat", None); let compat_response = bb.topic_ro::("/v1/dut/powered/compat", None); @@ -270,7 +270,7 @@ fn setup_labgrid_compat( } Ok(()) - }); + })?; wtb.spawn_task("power-compat-to-labgrid", async move { while let Some(state) = state_stream.next().await { @@ -282,7 +282,9 @@ fn setup_labgrid_compat( } Ok(()) - }); + })?; + + Ok(()) } impl DutPwrThread { @@ -494,7 +496,7 @@ impl DutPwrThread { let request_topic = bb.topic_wo::("/v1/dut/powered", None); let state_topic = bb.topic_ro::("/v1/dut/powered", None); - setup_labgrid_compat(bb, wtb, request_topic.clone(), state_topic.clone()); + setup_labgrid_compat(bb, wtb, request_topic.clone(), state_topic.clone())?; // Requests come from the broker framework and are placed into an atomic // request variable read by the thread. @@ -507,7 +509,7 @@ impl DutPwrThread { } Ok(()) - }); + })?; // State information comes from the thread in the form of an atomic // variable and is forwarded to the broker framework. @@ -519,7 +521,7 @@ impl DutPwrThread { let curr_state = state.load(Ordering::Relaxed).into(); state_topic_task.set_if_changed(curr_state); } - }); + })?; // Forward the state information to the DUT Power LED let (mut state_stream, _) = state_topic.clone().subscribe_unbounded(); @@ -552,7 +554,7 @@ impl DutPwrThread { } Ok(()) - }); + })?; Ok(Self { request: request_topic, diff --git a/src/http_server.rs b/src/http_server.rs index f6d2abce..63536e23 100644 --- a/src/http_server.rs +++ b/src/http_server.rs @@ -18,6 +18,7 @@ use std::fs::write; use std::net::TcpListener; +use anyhow::Result; use tide::{Body, Response, Server}; use crate::watched_tasks::WatchedTasksBuilder; @@ -128,10 +129,10 @@ impl HttpServer { }); } - pub fn serve(self, wtb: &mut WatchedTasksBuilder) { + pub fn serve(self, wtb: &mut WatchedTasksBuilder) -> Result<()> { wtb.spawn_task("http-server", async move { self.server.listen(self.listeners).await?; Ok(()) - }); + }) } } diff --git a/src/iobus.rs b/src/iobus.rs index e6955a51..541eed51 100644 --- a/src/iobus.rs +++ b/src/iobus.rs @@ -17,6 +17,7 @@ use std::time::Duration; +use anyhow::Result; use async_std::sync::Arc; use async_std::task::sleep; @@ -114,7 +115,7 @@ impl IoBus { iobus_pwr_en: Arc>, iobus_curr: CalibratedChannel, iobus_volt: CalibratedChannel, - ) -> Self { + ) -> Result { let supply_fault = bb.topic_ro("/v1/iobus/feedback/fault", None); let server_info = bb.topic_ro("/v1/iobus/server/info", None); let nodes = bb.topic_ro("/v1/iobus/server/nodes", None); @@ -153,12 +154,12 @@ impl IoBus { sleep(Duration::from_secs(1)).await; } - }); + })?; - Self { + Ok(Self { supply_fault, server_info, nodes, - } + }) } } diff --git a/src/led.rs b/src/led.rs index d33c5031..f4a2373a 100644 --- a/src/led.rs +++ b/src/led.rs @@ -17,6 +17,7 @@ use std::io::ErrorKind; +use anyhow::Result; use async_std::prelude::*; use async_std::sync::Arc; use log::{error, info, warn}; @@ -70,7 +71,7 @@ fn handle_pattern( wtb: &mut WatchedTasksBuilder, hardware_name: &'static str, topic_name: &'static str, -) -> Arc> { +) -> Result>> { let topic = bb.topic_ro(&format!("/v1/tac/led/{topic_name}/pattern"), None); if let Some(led) = get_led_checked(hardware_name) { @@ -84,10 +85,10 @@ fn handle_pattern( } Ok(()) - }); + })?; } - topic + Ok(topic) } fn handle_color( @@ -95,7 +96,7 @@ fn handle_color( wtb: &mut WatchedTasksBuilder, hardware_name: &'static str, topic_name: &'static str, -) -> Arc> { +) -> Result>> { let topic = bb.topic_ro(&format!("/v1/tac/led/{topic_name}/color"), None); if let Some(led) = get_led_checked(hardware_name) { @@ -115,22 +116,22 @@ fn handle_color( } Ok(()) - }); + })?; } - topic + Ok(topic) } impl Led { - pub fn new(bb: &mut BrokerBuilder, wtb: &mut WatchedTasksBuilder) -> Self { - Self { - out_0: handle_pattern(bb, wtb, "tac:green:out0", "out_0"), - out_1: handle_pattern(bb, wtb, "tac:green:out1", "out_1"), - dut_pwr: handle_pattern(bb, wtb, "tac:green:dutpwr", "dut_pwr"), - eth_dut: handle_pattern(bb, wtb, "tac:green:statusdut", "eth_dut"), - eth_lab: handle_pattern(bb, wtb, "tac:green:statuslab", "eth_lab"), - status: handle_pattern(bb, wtb, "rgb:status", "status"), - status_color: handle_color(bb, wtb, "rgb:status", "status"), - } + pub fn new(bb: &mut BrokerBuilder, wtb: &mut WatchedTasksBuilder) -> Result { + Ok(Self { + out_0: handle_pattern(bb, wtb, "tac:green:out0", "out_0")?, + out_1: handle_pattern(bb, wtb, "tac:green:out1", "out_1")?, + dut_pwr: handle_pattern(bb, wtb, "tac:green:dutpwr", "dut_pwr")?, + eth_dut: handle_pattern(bb, wtb, "tac:green:statusdut", "eth_dut")?, + eth_lab: handle_pattern(bb, wtb, "tac:green:statuslab", "eth_lab")?, + status: handle_pattern(bb, wtb, "rgb:status", "status")?, + status_color: handle_color(bb, wtb, "rgb:status", "status")?, + }) } } diff --git a/src/main.rs b/src/main.rs index 197d2d72..091c27c7 100644 --- a/src/main.rs +++ b/src/main.rs @@ -70,7 +70,7 @@ async fn init() -> Result<(Ui, HttpServer, WatchedTasksBuilder)> { // Expose hardware on the TAC via the broker framework. let backlight = Backlight::new(&mut bb, &mut wtb)?; - let led = Led::new(&mut bb, &mut wtb); + let led = Led::new(&mut bb, &mut wtb)?; let adc = Adc::new(&mut bb, &mut wtb).await?; let dut_pwr = DutPwrThread::new( &mut bb, @@ -80,8 +80,8 @@ async fn init() -> Result<(Ui, HttpServer, WatchedTasksBuilder)> { led.dut_pwr.clone(), ) .await?; - let dig_io = DigitalIo::new(&mut bb, &mut wtb, led.out_0.clone(), led.out_1.clone()); - let regulators = Regulators::new(&mut bb, &mut wtb); + let dig_io = DigitalIo::new(&mut bb, &mut wtb, led.out_0.clone(), led.out_1.clone())?; + let regulators = Regulators::new(&mut bb, &mut wtb)?; let temperatures = Temperatures::new(&mut bb, &mut wtb)?; let usb_hub = UsbHub::new( &mut bb, @@ -90,7 +90,7 @@ async fn init() -> Result<(Ui, HttpServer, WatchedTasksBuilder)> { adc.usb_host1_curr.fast.clone(), adc.usb_host2_curr.fast.clone(), adc.usb_host3_curr.fast.clone(), - ); + )?; // Expose other software on the TAC via the broker framework by connecting // to them via HTTP / DBus APIs. @@ -100,10 +100,10 @@ async fn init() -> Result<(Ui, HttpServer, WatchedTasksBuilder)> { regulators.iobus_pwr_en.clone(), adc.iobus_curr.fast.clone(), adc.iobus_volt.fast.clone(), - ); + )?; let (hostname, network, rauc, systemd) = { let dbus = - DbusSession::new(&mut bb, &mut wtb, led.eth_dut.clone(), led.eth_lab.clone()).await; + DbusSession::new(&mut bb, &mut wtb, led.eth_dut.clone(), led.eth_lab.clone()).await?; (dbus.hostname, dbus.network, dbus.rauc, dbus.systemd) }; @@ -122,7 +122,7 @@ async fn init() -> Result<(Ui, HttpServer, WatchedTasksBuilder)> { let mut http_server = HttpServer::new(); // Allow editing some aspects of the TAC configuration when in "setup mode". - let setup_mode = SetupMode::new(&mut bb, &mut wtb, &mut http_server.server); + let setup_mode = SetupMode::new(&mut bb, &mut wtb, &mut http_server.server)?; // Expose a live log of the TAC's systemd journal so it can be viewed // in the web interface. @@ -155,7 +155,7 @@ async fn init() -> Result<(Ui, HttpServer, WatchedTasksBuilder)> { // Consume the BrokerBuilder (no further topics can be added or removed) // and expose the topics via HTTP and MQTT-over-websocket. - bb.build(&mut wtb, &mut http_server.server); + bb.build(&mut wtb, &mut http_server.server)?; // If a watchdog was requested by systemd we can now start feeding it if let Some(watchdog) = watchdog { @@ -175,10 +175,10 @@ async fn run( ui::serve_display(&mut http_server.server, display.screenshooter()); // Start serving files and the API - http_server.serve(&mut wtb); + http_server.serve(&mut wtb)?; // Start drawing the UI - ui.run(&mut wtb, display); + ui.run(&mut wtb, display)?; info!("Setup complete. Handling requests"); diff --git a/src/regulators.rs b/src/regulators.rs index 971dcb39..76ed1ee9 100644 --- a/src/regulators.rs +++ b/src/regulators.rs @@ -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::Result; use async_std::prelude::*; use async_std::sync::Arc; @@ -75,7 +76,7 @@ fn handle_regulator( path: &str, regulator_name: &'static str, initial: bool, -) -> Arc> { +) -> Result>> { let topic = bb.topic_rw(path, Some(initial)); let (mut src, _) = topic.clone().subscribe_unbounded(); @@ -85,16 +86,16 @@ fn handle_regulator( } Ok(()) - }); + })?; - topic + Ok(topic) } impl Regulators { - pub fn new(bb: &mut BrokerBuilder, wtb: &mut WatchedTasksBuilder) -> Self { - Self { - iobus_pwr_en: handle_regulator(bb, wtb, "/v1/iobus/powered", "output-iobus-12v", true), - uart_pwr_en: handle_regulator(bb, wtb, "/v1/uart/powered", "output-vuart", true), - } + pub fn new(bb: &mut BrokerBuilder, wtb: &mut WatchedTasksBuilder) -> Result { + Ok(Self { + iobus_pwr_en: handle_regulator(bb, wtb, "/v1/iobus/powered", "output-iobus-12v", true)?, + uart_pwr_en: handle_regulator(bb, wtb, "/v1/uart/powered", "output-vuart", true)?, + }) } } diff --git a/src/setup_mode.rs b/src/setup_mode.rs index 3ef41261..01e3a767 100644 --- a/src/setup_mode.rs +++ b/src/setup_mode.rs @@ -19,6 +19,7 @@ use std::fs::{create_dir_all, read, write}; use std::io::ErrorKind; use std::path::Path; +use anyhow::Result; use async_std::prelude::*; use async_std::sync::Arc; use tide::{http::mime, Request, Response, Server}; @@ -106,7 +107,11 @@ impl SetupMode { }); } - fn handle_leave_requests(&self, bb: &mut BrokerBuilder, wtb: &mut WatchedTasksBuilder) { + fn handle_leave_requests( + &self, + bb: &mut BrokerBuilder, + wtb: &mut WatchedTasksBuilder, + ) -> Result<()> { // Use the "register a read-only and a write-only topic with the same name // to perform validation" trick that is also used with the DUT power endpoint. // We must make sure that a client from the web can only ever trigger _leaving_ @@ -125,14 +130,14 @@ impl SetupMode { } Ok(()) - }); + }) } pub fn new( bb: &mut BrokerBuilder, wtb: &mut WatchedTasksBuilder, server: &mut Server<()>, - ) -> Self { + ) -> Result { let this = Self { setup_mode: bb.topic("/v1/tac/setup_mode", true, false, true, Some(true), 1), show_help: bb.topic( @@ -145,9 +150,9 @@ impl SetupMode { ), }; - this.handle_leave_requests(bb, wtb); + this.handle_leave_requests(bb, wtb)?; this.expose_file_conditionally(server, AUTHORIZED_KEYS_PATH, "/v1/tac/ssh/authorized_keys"); - this + Ok(this) } } diff --git a/src/ui.rs b/src/ui.rs index 21366fcd..aaf8a8b0 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -138,7 +138,7 @@ impl Ui { alerts.assert(AlertScreen::ScreenSaver); // Initialize all the screens now so they can be activated later - let screens = screens::init(wtb, &res, &alerts, &buttons, &reboot_message, &locator); + let screens = screens::init(wtb, &res, &alerts, &buttons, &reboot_message, &locator)?; handle_buttons( wtb, @@ -173,7 +173,7 @@ impl Ui { } Ok(()) - }); + })?; Ok(Self { screen, @@ -291,11 +291,11 @@ impl Ui { Ok(()) } - pub fn run(self, wtb: &mut WatchedTasksBuilder, display: Display) { + pub fn run(self, wtb: &mut WatchedTasksBuilder, display: Display) -> Result<()> { wtb.spawn_task("screen-render-loop", async move { self.render_loop(display).await?; Ok(()) - }); + }) } } diff --git a/src/ui/screens.rs b/src/ui/screens.rs index bd269abe..d076fb36 100644 --- a/src/ui/screens.rs +++ b/src/ui/screens.rs @@ -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::Result; use async_std::sync::Arc; use async_trait::async_trait; use embedded_graphics::{ @@ -191,34 +192,38 @@ pub(super) fn init( buttons: &Arc>, reboot_message: &Arc>>, locator: &Arc>, -) -> Vec> { - vec![ +) -> Result>> { + Ok(vec![ Box::new(DigOutScreen::new()), Box::new(IoBusScreen::new()), Box::new(PowerScreen::new()), Box::new(SystemScreen::new()), Box::new(UartScreen::new()), Box::new(UsbScreen::new()), - Box::new(HelpScreen::new(wtb, alerts, &res.setup_mode.show_help)), - Box::new(IoBusHealthScreen::new(wtb, alerts, &res.iobus.supply_fault)), + Box::new(HelpScreen::new(wtb, alerts, &res.setup_mode.show_help)?), + Box::new(IoBusHealthScreen::new( + wtb, + alerts, + &res.iobus.supply_fault, + )?), Box::new(UpdateInstallationScreen::new( wtb, alerts, &res.rauc.operation, reboot_message, &res.rauc.should_reboot, - )), - Box::new(UpdateAvailableScreen::new(wtb, alerts, &res.rauc.channels)), - Box::new(RebootConfirmScreen::new(wtb, alerts, reboot_message)), - Box::new(ScreenSaverScreen::new(wtb, buttons, alerts)), - Box::new(SetupScreen::new(wtb, alerts, &res.setup_mode.setup_mode)), + )?), + Box::new(UpdateAvailableScreen::new(wtb, alerts, &res.rauc.channels)?), + Box::new(RebootConfirmScreen::new(wtb, alerts, reboot_message)?), + Box::new(ScreenSaverScreen::new(wtb, buttons, alerts)?), + Box::new(SetupScreen::new(wtb, alerts, &res.setup_mode.setup_mode)?), Box::new(OverTemperatureScreen::new( wtb, alerts, &res.temperatures.warning, - )), - Box::new(LocatorScreen::new(wtb, alerts, locator)), - Box::new(UsbOverloadScreen::new(wtb, alerts, &res.usb_hub.overload)), - Box::new(PowerFailScreen::new(wtb, alerts, &res.dut_pwr.state)), - ] + )?), + Box::new(LocatorScreen::new(wtb, alerts, locator)?), + Box::new(UsbOverloadScreen::new(wtb, alerts, &res.usb_hub.overload)?), + Box::new(PowerFailScreen::new(wtb, alerts, &res.dut_pwr.state)?), + ]) } diff --git a/src/ui/screens/help.rs b/src/ui/screens/help.rs index 4fa308f3..ed193d9a 100644 --- a/src/ui/screens/help.rs +++ b/src/ui/screens/help.rs @@ -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::Result; use async_std::prelude::*; use async_std::sync::Arc; use async_trait::async_trait; @@ -69,7 +70,7 @@ impl HelpScreen { wtb: &mut WatchedTasksBuilder, alerts: &Arc>, show_help: &Arc>, - ) -> Self { + ) -> Result { let (mut show_help_events, _) = show_help.clone().subscribe_unbounded(); let alerts = alerts.clone(); @@ -83,9 +84,9 @@ impl HelpScreen { } Ok(()) - }); + })?; - Self + Ok(Self) } } diff --git a/src/ui/screens/iobus_health.rs b/src/ui/screens/iobus_health.rs index fa5b4ffc..ad59eeaa 100644 --- a/src/ui/screens/iobus_health.rs +++ b/src/ui/screens/iobus_health.rs @@ -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::Result; use async_std::prelude::*; use async_std::sync::Arc; use async_trait::async_trait; @@ -45,7 +46,7 @@ impl IoBusHealthScreen { wtb: &mut WatchedTasksBuilder, alerts: &Arc>, supply_fault: &Arc>, - ) -> Self { + ) -> Result { let (mut supply_fault_events, _) = supply_fault.clone().subscribe_unbounded(); let alerts = alerts.clone(); @@ -59,9 +60,9 @@ impl IoBusHealthScreen { } Ok(()) - }); + })?; - Self + Ok(Self) } } diff --git a/src/ui/screens/locator.rs b/src/ui/screens/locator.rs index e9025716..70321fb6 100644 --- a/src/ui/screens/locator.rs +++ b/src/ui/screens/locator.rs @@ -17,6 +17,7 @@ use std::time::Instant; +use anyhow::Result; use async_std::prelude::*; use async_std::sync::Arc; use async_trait::async_trait; @@ -50,7 +51,7 @@ impl LocatorScreen { wtb: &mut WatchedTasksBuilder, alerts: &Arc>, locator: &Arc>, - ) -> Self { + ) -> Result { let (mut locator_events, _) = locator.clone().subscribe_unbounded(); let alerts = alerts.clone(); @@ -64,9 +65,9 @@ impl LocatorScreen { } Ok(()) - }); + })?; - Self + Ok(Self) } } diff --git a/src/ui/screens/overtemperature.rs b/src/ui/screens/overtemperature.rs index 182c5392..9167a5b3 100644 --- a/src/ui/screens/overtemperature.rs +++ b/src/ui/screens/overtemperature.rs @@ -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::Result; use async_std::prelude::*; use async_std::sync::Arc; use async_trait::async_trait; @@ -45,7 +46,7 @@ impl OverTemperatureScreen { wtb: &mut WatchedTasksBuilder, alerts: &Arc>, warning: &Arc>, - ) -> Self { + ) -> Result { let (mut warning_events, _) = warning.clone().subscribe_unbounded(); let alerts = alerts.clone(); @@ -58,9 +59,9 @@ impl OverTemperatureScreen { } Ok(()) - }); + })?; - Self + Ok(Self) } } diff --git a/src/ui/screens/power_fail.rs b/src/ui/screens/power_fail.rs index 6ff11c52..e68f3091 100644 --- a/src/ui/screens/power_fail.rs +++ b/src/ui/screens/power_fail.rs @@ -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::Result; use async_std::prelude::*; use async_std::sync::Arc; use async_trait::async_trait; @@ -62,7 +63,7 @@ impl PowerFailScreen { wtb: &mut WatchedTasksBuilder, alerts: &Arc>, out_state: &Arc>, - ) -> Self { + ) -> Result { let (mut out_state_events, _) = out_state.clone().subscribe_unbounded(); let alerts = alerts.clone(); @@ -82,9 +83,9 @@ impl PowerFailScreen { } Ok(()) - }); + })?; - Self + Ok(Self) } } diff --git a/src/ui/screens/reboot.rs b/src/ui/screens/reboot.rs index eb391511..6b1fcd56 100644 --- a/src/ui/screens/reboot.rs +++ b/src/ui/screens/reboot.rs @@ -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::Result; use async_std::prelude::*; use async_std::sync::Arc; use async_trait::async_trait; @@ -44,7 +45,7 @@ impl RebootConfirmScreen { wtb: &mut WatchedTasksBuilder, alerts: &Arc>, reboot_message: &Arc>>, - ) -> Self { + ) -> Result { // Receive questions like Some("Do you want to reboot?") and activate this screen let (mut reboot_message_events, _) = reboot_message.clone().subscribe_unbounded(); let reboot_message = reboot_message.clone(); @@ -60,9 +61,9 @@ impl RebootConfirmScreen { } Ok(()) - }); + })?; - Self { reboot_message } + Ok(Self { reboot_message }) } } diff --git a/src/ui/screens/screensaver.rs b/src/ui/screens/screensaver.rs index 936d89e1..b8245acd 100644 --- a/src/ui/screens/screensaver.rs +++ b/src/ui/screens/screensaver.rs @@ -18,6 +18,7 @@ use std::convert::TryInto; use std::time::{Duration, SystemTime}; +use anyhow::Result; use async_std::future::timeout; use async_std::prelude::*; use async_std::sync::Arc; @@ -95,7 +96,7 @@ impl ScreenSaverScreen { wtb: &mut WatchedTasksBuilder, buttons: &Arc>, alerts: &Arc>, - ) -> Self { + ) -> Result { // Activate screensaver if no button is pressed for some time let (mut buttons_events, _) = buttons.clone().subscribe_unbounded(); let alerts = alerts.clone(); @@ -115,9 +116,9 @@ impl ScreenSaverScreen { } Ok(()) - }); + })?; - Self + Ok(Self) } } diff --git a/src/ui/screens/setup.rs b/src/ui/screens/setup.rs index b556ba37..09c318a0 100644 --- a/src/ui/screens/setup.rs +++ b/src/ui/screens/setup.rs @@ -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::Result; use async_std::prelude::*; use async_std::sync::Arc; use async_std::task::spawn; @@ -53,7 +54,7 @@ impl SetupScreen { wtb: &mut WatchedTasksBuilder, alerts: &Arc>, setup_mode: &Arc>, - ) -> Self { + ) -> Result { let (mut setup_mode_events, _) = setup_mode.clone().subscribe_unbounded(); let alerts = alerts.clone(); @@ -67,9 +68,9 @@ impl SetupScreen { } Ok(()) - }); + })?; - Self + Ok(Self) } } diff --git a/src/ui/screens/update_available.rs b/src/ui/screens/update_available.rs index 065661c5..6fa3bd60 100644 --- a/src/ui/screens/update_available.rs +++ b/src/ui/screens/update_available.rs @@ -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::Result; use async_std::prelude::*; use async_std::sync::Arc; use async_trait::async_trait; @@ -143,7 +144,7 @@ impl UpdateAvailableScreen { wtb: &mut WatchedTasksBuilder, alerts: &Arc>, channels: &Arc>>, - ) -> Self { + ) -> Result { let (mut channels_events, _) = channels.clone().subscribe_unbounded(); let alerts = alerts.clone(); let selection = Topic::anonymous(Some(Selection::new())); @@ -161,9 +162,9 @@ impl UpdateAvailableScreen { } Ok(()) - }); + })?; - Self { selection } + Ok(Self { selection }) } } diff --git a/src/ui/screens/update_installation.rs b/src/ui/screens/update_installation.rs index 15bdcfec..e3ce6867 100644 --- a/src/ui/screens/update_installation.rs +++ b/src/ui/screens/update_installation.rs @@ -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::Result; use async_std::prelude::*; use async_std::sync::Arc; use async_trait::async_trait; @@ -51,7 +52,7 @@ impl UpdateInstallationScreen { operation: &Arc>, reboot_message: &Arc>>, should_reboot: &Arc>, - ) -> Self { + ) -> Result { let (mut operation_events, _) = operation.clone().subscribe_unbounded(); let alerts = alerts.clone(); @@ -64,7 +65,7 @@ impl UpdateInstallationScreen { } Ok(()) - }); + })?; let (mut should_reboot_events, _) = should_reboot.clone().subscribe_unbounded(); let reboot_message = reboot_message.clone(); @@ -77,9 +78,9 @@ impl UpdateInstallationScreen { } Ok(()) - }); + })?; - Self + Ok(Self) } } diff --git a/src/ui/screens/usb_overload.rs b/src/ui/screens/usb_overload.rs index 86a2dd05..a491c588 100644 --- a/src/ui/screens/usb_overload.rs +++ b/src/ui/screens/usb_overload.rs @@ -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::Result; use async_std::prelude::*; use async_std::sync::Arc; use async_trait::async_trait; @@ -49,7 +50,7 @@ impl UsbOverloadScreen { wtb: &mut WatchedTasksBuilder, alerts: &Arc>, overload: &Arc>>, - ) -> Self { + ) -> Result { let (mut overload_events, _) = overload.clone().subscribe_unbounded(); let alerts = alerts.clone(); @@ -63,9 +64,9 @@ impl UsbOverloadScreen { } Ok(()) - }); + })?; - Self + Ok(Self) } } diff --git a/src/usb_hub.rs b/src/usb_hub.rs index 54c885ca..6c38711c 100644 --- a/src/usb_hub.rs +++ b/src/usb_hub.rs @@ -18,6 +18,7 @@ use std::path::Path; use std::time::Duration; +use anyhow::{anyhow, Result}; use async_std::prelude::*; use async_std::sync::Arc; use async_std::task::sleep; @@ -203,7 +204,7 @@ fn handle_port( wtb: &mut WatchedTasksBuilder, name: &'static str, base: &'static str, -) -> UsbPort { +) -> Result { let port = UsbPort { request: bb.topic_wo(format!("/v1/usb/host/{name}/powered").as_str(), None), status: bb.topic_ro(format!("/v1/usb/host/{name}/powered").as_str(), None), @@ -232,7 +233,7 @@ fn handle_port( } Ok(()) - }); + })?; let status = port.status.clone(); let device = port.device.clone(); @@ -280,9 +281,9 @@ fn handle_port( sleep(POLL_INTERVAL).await; } - }); + })?; - port + Ok(port) } fn handle_overloads( @@ -292,7 +293,7 @@ fn handle_overloads( port1: CalibratedChannel, port2: CalibratedChannel, port3: CalibratedChannel, -) -> Arc>> { +) -> Result>>> { let overload = bb.topic_ro("/v1/usb/host/overload", None); let overload_task = overload.clone(); @@ -310,9 +311,9 @@ fn handle_overloads( sleep(POLL_INTERVAL).await; } - }); + })?; - overload + Ok(overload) } impl UsbHub { @@ -323,18 +324,24 @@ impl UsbHub { port1: CalibratedChannel, port2: CalibratedChannel, port3: CalibratedChannel, - ) -> Self { - let overload = handle_overloads(bb, wtb, total, port1, port2, port3); + ) -> Result { + let overload = handle_overloads(bb, wtb, total, port1, port2, port3)?; let mut ports = PORTS .iter() .map(|(name, base)| handle_port(bb, wtb, name, base)); - Self { + Ok(Self { overload, - port1: ports.next().unwrap(), - port2: ports.next().unwrap(), - port3: ports.next().unwrap(), - } + port1: ports + .next() + .ok_or_else(|| anyhow!("Failed to find USB port 1"))??, + port2: ports + .next() + .ok_or_else(|| anyhow!("Failed to find USB port 2"))??, + port3: ports + .next() + .ok_or_else(|| anyhow!("Failed to find USB port 3"))??, + }) } } diff --git a/src/watchdog.rs b/src/watchdog.rs index 5ba06a98..1f4b60b8 100644 --- a/src/watchdog.rs +++ b/src/watchdog.rs @@ -89,7 +89,7 @@ impl Watchdog { notify(false, [(STATE_WATCHDOG, "1")].iter())?; } - }); + })?; Ok(()) } diff --git a/src/watched_tasks.rs b/src/watched_tasks.rs index d1168116..bb05edd5 100644 --- a/src/watched_tasks.rs +++ b/src/watched_tasks.rs @@ -157,17 +157,16 @@ impl WatchedTasksBuilder { /// Future will return the Result of said task. /// The WatchedTasks Future should be .awaited at the end of main() so /// that the program ends if any of the watched tasks ends. - pub fn spawn_task(&mut self, name: S, future: F) + pub fn spawn_task(&mut self, name: S, future: F) -> Result<()> where S: Into, F: Future + Send + 'static, { - let task = task::Builder::new() - .name(name.into()) - .spawn(future) - .expect("cannot spawn task"); + let task = task::Builder::new().name(name.into()).spawn(future)?; self.tasks.push(task); + + Ok(()) } /// Spawn a thread that runs until the end of the program @@ -271,7 +270,8 @@ mod tests { let res = rx.recv().await?; println!("Bye from task {i}!"); res - }); + }) + .unwrap(); tx }) From 75e027e51c81948ababf3ef84266c3ad4da6af6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Leonard=20G=C3=B6hrs?= Date: Fri, 6 Oct 2023 10:39:58 +0200 Subject: [PATCH 6/8] adc: hardware: replace .unwraps() around setup done queue with .expect() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While .expect() still means panicking it is considerd a better kind of panicking than .unwrap(). Signed-off-by: Leonard Göhrs --- src/adc/iio/hardware.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/adc/iio/hardware.rs b/src/adc/iio/hardware.rs index d3bf8441..8c2b37c4 100644 --- a/src/adc/iio/hardware.rs +++ b/src/adc/iio/hardware.rs @@ -362,7 +362,9 @@ impl IioThread { Err(e) => { // Can not fail in practice as the queue is known to be empty // at this point. - thread_res_tx.try_send(Err(e)).unwrap(); + thread_res_tx + .try_send(Err(e)) + .expect("Failed to signal ADC setup error due to full queue"); return Ok(()); } }; @@ -385,7 +387,8 @@ impl IioThread { if let Some((_, tx)) = signal_ready.take() { // Can not fail in practice as the queue is only .take()n // once and thus known to be empty. - tx.try_send(Err(Error::new(e))).unwrap(); + tx.try_send(Err(Error::new(e))) + .expect("Failed to signal ADC setup error due to full queue"); } break; @@ -415,7 +418,8 @@ impl IioThread { if let Some((content, tx)) = signal_ready.take() { // Can not fail in practice as the queue is only .take()n // once and thus known to be empty. - tx.try_send(Ok(content)).unwrap(); + tx.try_send(Ok(content)) + .expect("Failed to signal ADC setup completion due to full queue"); } } From c8235151a2c5eb1e7273310cd49ef7c63fd92471 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Leonard=20G=C3=B6hrs?= Date: Mon, 6 Nov 2023 14:24:48 +0100 Subject: [PATCH 7/8] tacd: finish the http server setup in init() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This allows us to display an error message if the server setup fails. Signed-off-by: Leonard Göhrs --- src/main.rs | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/src/main.rs b/src/main.rs index 091c27c7..861b237f 100644 --- a/src/main.rs +++ b/src/main.rs @@ -52,12 +52,12 @@ use regulators::Regulators; use setup_mode::SetupMode; use system::System; use temperatures::Temperatures; -use ui::{message, setup_display, Display, Ui, UiResources}; +use ui::{message, setup_display, Display, ScreenShooter, Ui, UiResources}; use usb_hub::UsbHub; use watchdog::Watchdog; use watched_tasks::WatchedTasksBuilder; -async fn init() -> Result<(Ui, HttpServer, WatchedTasksBuilder)> { +async fn init(screenshooter: ScreenShooter) -> Result<(Ui, WatchedTasksBuilder)> { // The tacd spawns a couple of async tasks that should run as long as // the tacd runs and if any one fails the tacd should stop. // These tasks are spawned via the watched task builder. @@ -157,26 +157,21 @@ async fn init() -> Result<(Ui, HttpServer, WatchedTasksBuilder)> { // and expose the topics via HTTP and MQTT-over-websocket. bb.build(&mut wtb, &mut http_server.server)?; + // Expose the display as a .png on the web server + ui::serve_display(&mut http_server.server, screenshooter); + + // Start serving files and the API + http_server.serve(&mut wtb)?; + // If a watchdog was requested by systemd we can now start feeding it if let Some(watchdog) = watchdog { watchdog.keep_fed(&mut wtb)?; } - Ok((ui, http_server, wtb)) + Ok((ui, wtb)) } -async fn run( - ui: Ui, - mut http_server: HttpServer, - display: Display, - mut wtb: WatchedTasksBuilder, -) -> Result<()> { - // Expose the display as a .png on the web server - ui::serve_display(&mut http_server.server, display.screenshooter()); - - // Start serving files and the API - http_server.serve(&mut wtb)?; - +async fn run(ui: Ui, display: Display, mut wtb: WatchedTasksBuilder) -> Result<()> { // Start drawing the UI ui.run(&mut wtb, display)?; @@ -191,9 +186,10 @@ async fn main() -> Result<()> { // Show a splash screen very early on let display = setup_display(); + let screenshooter = display.screenshooter(); - match init().await { - Ok((ui, http_server, wtb)) => run(ui, http_server, display, wtb).await, + match init(screenshooter).await { + Ok((ui, wtb)) => run(ui, display, wtb).await, Err(e) => { // Display a detailed error message on stderr (and thus in the journal) ... error!("Failed to initialize tacd: {e}"); From 396136e4afbbd3675a9f92170c9159e5a6e73923 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Leonard=20G=C3=B6hrs?= Date: Mon, 6 Nov 2023 14:27:27 +0100 Subject: [PATCH 8/8] tacd: inline run() into main() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Many small functions is a nice thing in general, but a three line function that is only used once seems a bit over the top. Signed-off-by: Leonard Göhrs --- src/main.rs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/main.rs b/src/main.rs index 861b237f..48478121 100644 --- a/src/main.rs +++ b/src/main.rs @@ -52,7 +52,7 @@ use regulators::Regulators; use setup_mode::SetupMode; use system::System; use temperatures::Temperatures; -use ui::{message, setup_display, Display, ScreenShooter, Ui, UiResources}; +use ui::{message, setup_display, ScreenShooter, Ui, UiResources}; use usb_hub::UsbHub; use watchdog::Watchdog; use watched_tasks::WatchedTasksBuilder; @@ -171,25 +171,25 @@ async fn init(screenshooter: ScreenShooter) -> Result<(Ui, WatchedTasksBuilder)> Ok((ui, wtb)) } -async fn run(ui: Ui, display: Display, mut wtb: WatchedTasksBuilder) -> Result<()> { - // Start drawing the UI - ui.run(&mut wtb, display)?; - - info!("Setup complete. Handling requests"); - - wtb.watch().await -} - #[async_std::main] async fn main() -> Result<()> { env_logger::init(); // Show a splash screen very early on let display = setup_display(); + + // This allows us to expose screenshoots of the LCD screen via HTTP let screenshooter = display.screenshooter(); match init(screenshooter).await { - Ok((ui, wtb)) => run(ui, display, wtb).await, + Ok((ui, mut wtb)) => { + // Start drawing the UI + ui.run(&mut wtb, display)?; + + info!("Setup complete. Handling requests"); + + wtb.watch().await + } Err(e) => { // Display a detailed error message on stderr (and thus in the journal) ... error!("Failed to initialize tacd: {e}");