display: make DisplayControls cloneable in a multi-thread context

src/animation.rs

@@ -1,9 +1,10 @@
 use embassy_time::{Duration, Timer};
 use figments::surface::Surface;
+use figments_render::output::Brightness;
 use core::{fmt::Debug, ops::{Deref, DerefMut}};
 use log::*;
 
-use crate::events::DisplayControls;
+use crate::display::DisplayControls;
 
 #[derive(Default, Debug, Clone, Copy)]
 pub struct Animation {
@@ -28,7 +29,7 @@ impl<S: Surface> AnimationActor for S {
 }
 
 #[derive(Debug)]
-pub struct AnimDisplay<'a>(pub &'a DisplayControls);
+pub struct AnimDisplay<'a>(pub &'a mut DisplayControls);
 
 impl<'a> AnimationActor for AnimDisplay<'a> {
     fn get_opacity(&self) -> u8 {

src/bin/main.rs

@@ -83,7 +83,7 @@ async fn main(spawner: Spawner) {
         hi_spawn.must_spawn(renderbug_embassy::tasks::render::render(peripherals.RMT, peripherals.GPIO5.degrade(), surfaces, garage.display.clone(), wdt));
     }
     #[cfg(feature="headless")]
-    garage.display.render_is_running.signal(true);
+    garage.display.notify_render_is_running(true);
 
     #[cfg(feature="motion")]
     {

src/display.rs

@@ -1,5 +1,6 @@
+use embassy_sync::{blocking_mutex::{raw::CriticalSectionRawMutex, Mutex}, signal::Signal, watch::{Receiver, Watch}};
 use figments::prelude::*;
-use core::fmt::Debug;
+use core::{fmt::Debug, sync::atomic::{AtomicBool, AtomicU8}};
 use alloc::sync::Arc;
 
 //use super::{Output};
@@ -7,14 +8,11 @@ use figments_render::{
     gamma::{GammaCurve, WithGamma}, output::{Brightness, GammaCorrected, OutputAsync}, power::AsMilliwatts, smart_leds::PowerManagedWriterAsync
 };
 use smart_leds::SmartLedsWriteAsync;
-
-use crate::events::DisplayControls;
-
 // FIXME: We need a way to specify a different buffer format from the 'native' hardware output, due to sometimes testing with GRB strips instead of RGB
 pub struct BikeOutput<T: SmartLedsWriteAsync> {
     pixbuf: [T::Color; 178],
     writer: PowerManagedWriterAsync<T>,
-    controls: Arc<DisplayControls>
+    controls: DisplayControls
 }
 
 impl<T:SmartLedsWriteAsync> GammaCorrected for BikeOutput<T> where T::Color: PixelBlend<Rgb<u8>> + PixelFormat + Debug + WithGamma, T::Error: Debug {
@@ -24,7 +22,7 @@ impl<T:SmartLedsWriteAsync> GammaCorrected for BikeOutput<T> where T::Color: Pix
 }
 
 impl<T: SmartLedsWriteAsync> BikeOutput<T> where T::Color: PixelBlend<Rgb<u8>> + PixelFormat + WithGamma + 'static,  T::Error: core::fmt::Debug {
-    pub fn new(target: T, max_mw: u32, controls: Arc<DisplayControls>) -> Self {
+    pub fn new(target: T, max_mw: u32, controls: DisplayControls) -> Self {
         Self {
             pixbuf: [Default::default(); 178],
             writer: PowerManagedWriterAsync::new(target, max_mw),
@@ -39,15 +37,19 @@ impl<T: SmartLedsWriteAsync> BikeOutput<T> where T::Color: PixelBlend<Rgb<u8>> +
 
 impl<'a, T: SmartLedsWriteAsync + 'a> OutputAsync<'a, SegmentSpace> for BikeOutput<T> where T::Color: PixelBlend<Rgb<u8>> + Debug + 'static + AsMilliwatts + PixelFormat + WithGamma, [T::Color; 178]: AsMilliwatts + WithGamma + Copy,  T::Error: core::fmt::Debug {
     async fn commit_async(&mut self) -> Result<(), T::Error> {
-        let c = self.controls.as_ref();
+        self.writer.controls().set_brightness(self.controls.brightness());
-        self.writer.controls().set_brightness(c.brightness());
+        self.writer.controls().set_on(self.controls.is_on());
-        self.writer.controls().set_on(c.is_on());
         // TODO: We should grab the power used here and somehow feed it back into the telemetry layer, probably just via another atomic u32
         self.writer.write(&self.pixbuf).await
     }
     
     type HardwarePixel = T::Color;
     type Error = T::Error;
+    type Controls = DisplayControls;
+    
+    fn controls(&self) -> Option<&Self::Controls> {
+        Some(&self.controls)
+    }
 }
 
 impl<'a, T: SmartLedsWriteAsync> Sample<'a, SegmentSpace> for BikeOutput<T> where T::Color: Debug + PixelFormat + 'a, [T::Color; 178]: Sample<'a, SegmentSpace, Output = T::Color> {
@@ -139,3 +141,99 @@ pub struct Uniforms {
     pub frame: usize,
     pub primary_color: Hsv
 }
+
+struct ControlData {
+    on: AtomicBool,
+    brightness: AtomicU8
+}
+
+impl Default for ControlData {
+    fn default() -> Self {
+        Self {
+            on: AtomicBool::new(true),
+            brightness: AtomicU8::new(255)
+        }
+    }
+}
+
+// A watch that indicates whether or not the rendering engine is running. If the display is off or sleeping, this will be false.
+static RENDER_IS_RUNNING: Watch<CriticalSectionRawMutex, bool, 5> = Watch::new();
+
+// TODO: Implement something similar for a system-wide sleep mechanism
+pub struct DisplayControls {
+    data: Arc<ControlData>,
+    render_pause: Arc<Signal<CriticalSectionRawMutex, bool>>,
+    render_run_receiver: Receiver<'static, CriticalSectionRawMutex, bool, 5>
+}
+
+impl Clone for DisplayControls {
+    fn clone(&self) -> Self {
+        Self {
+            data: Arc::clone(&self.data),
+            render_pause: Arc::clone(&self.render_pause),
+            render_run_receiver: RENDER_IS_RUNNING.receiver().expect("Could not create enough render running receivers")
+        }
+    }
+}
+
+impl DisplayControls {
+    pub fn is_on(&self) -> bool {
+        self.data.on.load(core::sync::atomic::Ordering::Relaxed)
+    }
+
+    pub fn brightness(&self) -> u8 {
+        self.data.brightness.load(core::sync::atomic::Ordering::Relaxed)
+    }
+
+    // FIXME: its a bit weird we have a pub function for the renderer's privates to wait while hiding render_pause, but directly expose render_is_running for any task to wait on
+    pub async fn wait_until_display_is_on(&self) {
+        if let Some(true) = self.render_pause.try_take() {
+            while self.render_pause.wait().await {}
+        }
+    }
+
+    pub fn notify_render_is_running(&mut self, value: bool) {
+        RENDER_IS_RUNNING.sender().send(value);
+    }
+
+    pub async fn wait_until_render_is_running(&mut self) {
+        while !self.render_run_receiver.get().await {}
+    }
+}
+
+impl GammaCorrected for DisplayControls {
+    fn set_gamma(&mut self, gamma: GammaCurve) {
+        todo!()
+    }
+}
+
+impl Brightness for DisplayControls {
+    fn set_brightness(&mut self, brightness: u8) {
+        self.data.brightness.store(brightness, core::sync::atomic::Ordering::Relaxed);
+    }
+
+    fn set_on(&mut self, is_on: bool) {
+        self.data.on.store(is_on, core::sync::atomic::Ordering::Relaxed);
+        self.render_pause.signal(!is_on);
+    }
+}
+
+impl core::fmt::Debug for DisplayControls {
+    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> Result<(), core::fmt::Error> {
+        f.debug_struct("DisplayControls")
+            .field("on", &self.data.on)
+            .field("brightness", &self.data.brightness)
+            .field("render_pause", &self.render_pause.signaled())
+            .finish()
+    }
+}
+
+impl Default for DisplayControls {
+    fn default() -> Self {
+        Self {
+            data: Default::default(),
+            render_pause: Default::default(),
+            render_run_receiver: RENDER_IS_RUNNING.receiver().unwrap()
+        }
+    }
+}

src/events.rs

@@ -2,9 +2,8 @@
 use embassy_sync::{blocking_mutex::raw::{CriticalSectionRawMutex, NoopRawMutex}, channel::Channel, pubsub::PubSubChannel};
 use embassy_time::Duration;
 use nalgebra::{Vector2, Vector3};
-use alloc::sync::Arc;
 
-use crate::ego::engine::MotionState;
+use crate::{display::DisplayControls, ego::engine::MotionState};
 
 #[derive(Clone, Copy, Default, Debug)]
 pub enum Scene {
@@ -60,7 +59,13 @@ pub enum Notification {
     SetBrakelight(bool),
 
     // TODO: BPM detection via bluetooth
-    Beat
+    Beat,
+}
+
+#[derive(Clone, Copy, Debug)]
+pub enum Telemetry {
+    Notification(Notification),
+    Prediction(Prediction),
 }
 
 #[derive(Clone, Copy, Debug)]
@@ -69,71 +74,13 @@ pub enum SensorSource {
     GPS
 }
 
-// TODO: Make this clone() able, so multiple threads can point to the same underlying atomics for the hardware controls
-// FIXME: We only ever hold this behind an Arc and therefore end up storing a Signal inside of an Arc<>... which defeats the whole purpose and can introduce a deadlock
-pub struct DisplayControls {
-    on: AtomicBool,
-    brightness: AtomicU8,
-    // FIXME: This should get turned into a embassy_sync::Watch sender, so multiple tasks can wait on the renderer to be running.
-    pub render_is_running: Signal<CriticalSectionRawMutex, bool>,
-    render_pause: Signal<CriticalSectionRawMutex, bool>
-}
-
-impl DisplayControls {
-    pub fn is_on(&self) -> bool {
-        self.on.load(core::sync::atomic::Ordering::Relaxed)
-    }
-
-    pub fn brightness(&self) -> u8 {
-        self.brightness.load(core::sync::atomic::Ordering::Relaxed)
-    }
-
-    pub fn set_brightness(&self, brightness: u8) {
-        self.brightness.store(brightness, core::sync::atomic::Ordering::Relaxed);
-    }
-
-    // FIXME: its a bit weird we have a pub function for the renderer's privates to wait while hiding render_pause, but directly expose render_is_running for any task to wait on
-    pub async fn wait_for_on(&self) {
-        self.render_pause.wait().await;
-    }
-
-    pub fn set_on(&self, is_on: bool) {
-        self.on.store(is_on, core::sync::atomic::Ordering::Relaxed);
-        if is_on {
-            self.render_pause.signal(true);
-        } else {
-            self.render_pause.reset();
-        }
-    }
-}
-
-impl core::fmt::Debug for DisplayControls {
-    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> Result<(), core::fmt::Error> {
-        f.debug_struct("DisplayControls")
-            .field("on", &self.on)
-            .field("brightness", &self.brightness)
-            .field("render_is_running", &self.render_is_running.signaled())
-            .finish()
-    }
-}
-
-impl Default for DisplayControls {
-    fn default() -> Self {
-        Self {
-            on: AtomicBool::new(true),
-            brightness: AtomicU8::new(255),
-            render_is_running: Signal::new(),
-            render_pause: Signal::new()
-        }
-    }
-}
-
 #[derive(Debug)]
 pub struct BusGarage {
     pub motion: Channel<NoopRawMutex, Measurement, 5>,
     pub notify: PubSubChannel<CriticalSectionRawMutex, Notification, 5, 2, 4>,
     pub predict: Channel<CriticalSectionRawMutex, Prediction, 15>,
-    pub display: Arc<DisplayControls>
+    pub telemetry: PubSubChannel<CriticalSectionRawMutex, Telemetry, 15, 2, 4>,
+    pub display: DisplayControls
 }
 
 impl Default for BusGarage {
@@ -142,6 +89,7 @@ impl Default for BusGarage {
             motion: Channel::new(),
             notify: PubSubChannel::new(),
             predict: Channel::new(),
+            telemetry: PubSubChannel::new(),
             display: Default::default()
         }
     }

src/tasks/render.rs

@@ -7,14 +7,13 @@ use figments_render::output::{GammaCorrected, OutputAsync};
 use log::{info, warn};
 use rgb::Rgba;
 use nalgebra::ComplexField;
-use alloc::sync::Arc;
 
-use crate::{display::{BikeOutput, SegmentSpace, Uniforms}, events::DisplayControls};
+use crate::display::{BikeOutput, DisplayControls, SegmentSpace, Uniforms};
 
 //TODO: Import the bike surfaces from renderbug-prime, somehow make those surfaces into tasks
 
 #[embassy_executor::task]
-pub async fn render(rmt: esp_hal::peripherals::RMT<'static>, gpio: AnyPin<'static>, surfaces: BufferedSurfacePool<Uniforms, SegmentSpace, Rgba<u8>>, controls: Arc<DisplayControls>, mut wdt: Wdt<esp_hal::peripherals::TIMG0<'static>>) {
+pub async fn render(rmt: esp_hal::peripherals::RMT<'static>, gpio: AnyPin<'static>, surfaces: BufferedSurfacePool<Uniforms, SegmentSpace, Rgba<u8>>, safety_surfaces: BufferedSurfacePool<Uniforms, SegmentSpace, Rgba<u8>>, mut controls: DisplayControls, mut wdt: Wdt<esp_hal::peripherals::TIMG0<'static>>) {
     let frequency: Rate = Rate::from_mhz(80);
     let rmt = Rmt::new(rmt, frequency)
         .expect("Failed to initialize RMT").into_async();
@@ -45,7 +44,7 @@ pub async fn render(rmt: esp_hal::peripherals::RMT<'static>, gpio: AnyPin<'stati
     //output.set_gamma(GammaCurve::new(2.1));
 
     info!("Rendering started! {}ms since boot", Instant::now().as_millis());
-    controls.render_is_running.signal(true);
+    controls.notify_render_is_running(true);
 
     const FPS: u64 = 80;
     const RENDER_BUDGET: Duration = Duration::from_millis(1000 / FPS);
@@ -57,6 +56,8 @@ pub async fn render(rmt: esp_hal::peripherals::RMT<'static>, gpio: AnyPin<'stati
         output.blank();
 
         surfaces.render_to(&mut output, &uniforms);
+        // TODO: We should split up the safety layers so they always have full power
+        safety_surfaces.render_to(&mut output, &uniforms);
 
         // Finally, write out the rendered frame
         output.commit_async().await.expect("Failed to commit frame");
@@ -65,14 +66,14 @@ pub async fn render(rmt: esp_hal::peripherals::RMT<'static>, gpio: AnyPin<'stati
 
         if !controls.is_on() {
             warn!("Renderer is sleeping zzzz");
-            //controls.render_is_running.signal(false);
+            controls.notify_render_is_running(false);
             output.blank();
             wdt.disable();
-            controls.wait_for_on().await;
+            controls.wait_until_display_is_on().await;
             wdt.feed();
             wdt.enable();
             warn!("Renderer is awake !!!!");
-            //controls.render_is_running.signal(true);
+            controls.notify_render_is_running(true);
         }
 
         if render_duration < RENDER_BUDGET {