From a68ec3206c96b90bb209af4184ef737754890b13 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Leonard=20G=C3=B6hrs?= <l.goehrs@pengutronix.de>
Date: Tue, 16 May 2023 11:10:57 +0200
Subject: [PATCH] usb_hub: split powered state and request to prevent glitches
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Previously a change request could come in while the polling task was reading
the status resulting in the state switching back and forth.

Signed-off-by: Leonard Göhrs <l.goehrs@pengutronix.de>
---
 src/ui/screens/usb.rs | 30 ++++++++++++++++++++----------
 src/usb_hub.rs        | 17 +++++++++++------
 2 files changed, 31 insertions(+), 16 deletions(-)

diff --git a/src/ui/screens/usb.rs b/src/ui/screens/usb.rs
index aaaa979b..2e3e416a 100644
--- a/src/ui/screens/usb.rs
+++ b/src/ui/screens/usb.rs
@@ -51,7 +51,8 @@ impl UsbScreen {
 
 struct Active {
     widgets: WidgetContainer,
-    port_enables: [Arc<Topic<bool>>; 3],
+    port_requests: [Arc<Topic<bool>>; 3],
+    port_states: [Arc<Topic<bool>>; 3],
     highlighted: Arc<Topic<usize>>,
 }
 
@@ -78,19 +79,19 @@ impl ActivatableScreen for UsbScreen {
             (
                 0,
                 "Port 1",
-                &ui.res.usb_hub.port1.powered,
+                &ui.res.usb_hub.port1.status,
                 &ui.res.adc.usb_host1_curr.topic,
             ),
             (
                 1,
                 "Port 2",
-                &ui.res.usb_hub.port2.powered,
+                &ui.res.usb_hub.port2.status,
                 &ui.res.adc.usb_host2_curr.topic,
             ),
             (
                 2,
                 "Port 3",
-                &ui.res.usb_hub.port3.powered,
+                &ui.res.usb_hub.port3.status,
                 &ui.res.adc.usb_host3_curr.topic,
             ),
         ];
@@ -147,16 +148,22 @@ impl ActivatableScreen for UsbScreen {
             });
         }
 
-        let port_enables = [
-            ui.res.usb_hub.port1.powered.clone(),
-            ui.res.usb_hub.port2.powered.clone(),
-            ui.res.usb_hub.port3.powered.clone(),
+        let port_requests = [
+            ui.res.usb_hub.port1.request.clone(),
+            ui.res.usb_hub.port2.request.clone(),
+            ui.res.usb_hub.port3.request.clone(),
+        ];
+        let port_states = [
+            ui.res.usb_hub.port1.status.clone(),
+            ui.res.usb_hub.port2.status.clone(),
+            ui.res.usb_hub.port3.status.clone(),
         ];
         let highlighted = self.highlighted.clone();
 
         let active = Active {
             widgets,
-            port_enables,
+            port_requests,
+            port_states,
             highlighted,
         };
 
@@ -182,7 +189,10 @@ impl ActiveScreen for Active {
             InputEvent::ToggleAction(_) => {
                 self.highlighted.set((highlighted + 1) % 3);
             }
-            InputEvent::PerformAction(_) => self.port_enables[highlighted].toggle(false),
+            InputEvent::PerformAction(_) => {
+                let status = self.port_states[highlighted].try_get().unwrap_or(false);
+                self.port_requests[highlighted].set(!status);
+            }
         }
     }
 }
diff --git a/src/usb_hub.rs b/src/usb_hub.rs
index 1aad7aef..c5fc5e9c 100644
--- a/src/usb_hub.rs
+++ b/src/usb_hub.rs
@@ -145,7 +145,8 @@ pub struct UsbDevice {
 
 #[derive(Clone)]
 pub struct UsbPort {
-    pub powered: Arc<Topic<bool>>,
+    pub request: Arc<Topic<bool>>,
+    pub status: Arc<Topic<bool>>,
     pub device: Arc<Topic<Option<UsbDevice>>>,
 }
 
@@ -157,11 +158,13 @@ pub struct UsbHub {
 
 fn handle_port(bb: &mut BrokerBuilder, name: &'static str, base: &'static str) -> UsbPort {
     let port = UsbPort {
-        powered: bb.topic_rw(format!("/v1/usb/host/{name}/powered").as_str(), None),
+        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),
         device: bb.topic_ro(format!("/v1/usb/host/{name}/device").as_str(), Some(None)),
     };
 
-    let powered = port.powered.clone();
+    let request = port.request.clone();
+    let status = port.status.clone();
     let device = port.device.clone();
     let disable_path = Path::new(base).join("disable");
 
@@ -169,7 +172,7 @@ fn handle_port(bb: &mut BrokerBuilder, name: &'static str, base: &'static str) -
     // Also clears the device info upon power off so it does not contain stale
     // information until the next poll.
     spawn(async move {
-        let (mut src, _) = powered.subscribe_unbounded();
+        let (mut src, _) = request.subscribe_unbounded();
 
         while let Some(ev) = src.next().await {
             write(&disable_path, if ev { b"0" } else { b"1" }).unwrap();
@@ -177,10 +180,12 @@ fn handle_port(bb: &mut BrokerBuilder, name: &'static str, base: &'static str) -
             if !ev {
                 device.set(None);
             }
+
+            status.set(ev);
         }
     });
 
-    let powered = port.powered.clone();
+    let status = port.status.clone();
     let device = port.device.clone();
     let disable_path = Path::new(base).join("disable");
     let (id_product_path, id_vendor_path, manufacturer_path, product_path) = {
@@ -204,7 +209,7 @@ fn handle_port(bb: &mut BrokerBuilder, name: &'static str, base: &'static str) -
                     _ => panic!("Read unexpected value for USB port disable state"),
                 };
 
-                powered.set_if_changed(is_powered);
+                status.set_if_changed(is_powered);
             }
 
             let id_product = read_to_string(&id_product_path).ok();