#297 improving the behaviour of IntervalText to run all updates in a single thread

crates/penrose_ui/src/bar/mod.rs

@@ -9,8 +9,10 @@ use penrose::{
 use std::fmt;
 use tracing::{debug, error, info};
 
+pub mod schedule;
 pub mod widgets;
 
+use schedule::{run_update_schedules, UpdateSchedule};
 use widgets::Widget;
 
 /// The position of a status bar
@@ -151,10 +153,29 @@ impl<X: XConn> StatusBar<X> {
 
  /// Add this [`StatusBar`] into the given [`WindowManager`] along with the required
  /// hooks for driving it from the main WindowManager event loop.
- pub fn add_to(self, mut wm: WindowManager<X>) -> WindowManager<X>
+ ///
+ /// If any [UpdateSchedule]s are requested by [Widgets] then they will be extracted and run as
+ /// part of calling this method.
+ pub fn add_to(mut self, mut wm: WindowManager<X>) -> WindowManager<X>
  where
  X: 'static,
  {
+ let schedules: Vec<UpdateSchedule> = match &mut self.widgets {
+ Widgets::Shared(ps) => ps
+ .ws
+ .iter_mut()
+ .filter_map(|w| w.update_schedule())
+ .collect(),
+ Widgets::PerScreen(pss) => pss
+ .iter_mut()
+ .flat_map(|ps| ps.ws.iter_mut().filter_map(|w| w.update_schedule()))
+ .collect(),
+ };
+
+ if !schedules.is_empty() {
+ run_update_schedules(schedules);
+ }
+
  wm.state.add_extension(self);
  wm.state.config.compose_or_set_event_hook(event_hook);
  wm.state.config.compose_or_set_manage_hook(manage_hook);

crates/penrose_ui/src/bar/schedule.rs

@@ -0,0 +1,104 @@
+//! Utilities for running scheduled updates to widgets
+use crate::bar::widgets::Text;
+use penrose::util::spawn_with_args;
+use std::{
+ cmp::max,
+ fmt,
+ sync::{Arc, Mutex},
+ thread,
+ time::{Duration, Instant},
+};
+use tracing::{debug, trace};
+
+/// The minimum allowed interval for an [UpdateSchedule].
+pub const MIN_DURATION: Duration = Duration::from_secs(1);
+
+/// For widgets that want to have their content updated periodically by the status bar by calling
+/// an external function.
+///
+/// See [IntervalText] for a simple implementation of this behaviour
+pub struct UpdateSchedule {
+ pub(crate) next: Instant,
+ pub(crate) interval: Duration,
+ pub(crate) get_text: Box<dyn Fn() -> Option<String> + Send + 'static>,
+ pub(crate) txt: Arc<Mutex<Text>>,
+}
+
+impl fmt::Debug for UpdateSchedule {
+ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+ f.debug_struct("UpdateSchedule")
+ .field("next", &self.next)
+ .field("interval", &self.interval)
+ .field("txt", &self.txt)
+ .finish()
+ }
+}
+
+impl UpdateSchedule {
+ /// Construct a new [UpdateSchedule] specifying the interval that the [Widget] content should
+ /// be updated on and an update function for producing the widget content.
+ ///
+ /// The updated content will then be stored in the provided `Arc<Mutex<Text>>` for access
+ /// within your widget logic.
+ pub fn new(
+ interval: Duration,
+ get_text: Box<dyn Fn() -> Option<String> + Send + 'static>,
+ txt: Arc<Mutex<Text>>,
+ ) -> Self {
+ if interval < MIN_DURATION {
+ panic!("UpdateSchedule interval is too small: {interval:?} < {MIN_DURATION:?}");
+ }
+
+ Self {
+ next: Instant::now(),
+ interval,
+ get_text,
+ txt,
+ }
+ }
+
+ /// Call our `get_text` function to update the contents of our paired [CronText] and then bump
+ /// our `next` time to the next interval point.
+ ///
+ /// This is gives us behaviour of a consistent interval between invocation end/start but not
+ /// necessarily a consistent interval between start/start depending on how long `get_text`
+ /// takes to run.
+ fn update_text(&mut self) {
+ trace!("running UpdateSchedule get_text");
+ let s = (self.get_text)();
+ trace!(?s, "ouput from running get_text");
+
+ if let Some(s) = s {
+ let mut t = match self.txt.lock() {
+ Ok(inner) => inner,
+ Err(poisoned) => poisoned.into_inner(),
+ };
+ t.set_text(s);
+ }
+
+ let next = self.next + self.interval;
+ let now = Instant::now();
+ self.next = max(next, now);
+ trace!(next = ?self.next, "next update at");
+ }
+}
+
+/// Run the polling thread for a set of [UpdateSchedule]s and update their contents on
+/// their requested intervals.
+pub(crate) fn run_update_schedules(mut schedules: Vec<UpdateSchedule>) {
+ thread::spawn(move || loop {
+ debug!("running UpdateSchedule updates for all pending widgets");
+ while schedules[0].next < Instant::now() {
+ schedules[0].update_text();
+ schedules.sort_by(|a, b| a.next.cmp(&b.next));
+ }
+
+ // FIXME: this is a hack at the moment to ensure that an event drops into the main
+ // window manager event loop and triggers the `on_event` hook of the status bar.
+ let _ = spawn_with_args("xsetroot", &["-name", ""]);
+
+ let interval = schedules[0].next - Instant::now();
+ debug!(?interval, "sleeping until next update point");
+ thread::sleep(interval);
+ });
+}

crates/penrose_ui/src/bar/widgets/mod.rs

@@ -1,5 +1,5 @@
 //! Self rendering building blocks for text based UI elements
-use crate::{Context, Result, TextStyle};
+use crate::{bar::schedule::UpdateSchedule, Context, Result, TextStyle};
 use penrose::{
  core::State,
  pure::geometry::Rect,
@@ -8,19 +8,17 @@ use penrose::{
 };
 use std::{
  fmt,
- sync::{Arc, Mutex},
- thread,
+ sync::{Arc, Mutex, MutexGuard},
  time::Duration,
 };
-use tracing::trace;
 
 pub mod debug;
+pub mod sys;
+
 mod simple;
-mod sys;
 mod workspaces;
 
 pub use simple::{ActiveWindowName, CurrentLayout, RootWindowName};
-pub use sys::{amixer_volume, battery_summary, current_date_and_time, wifi_network};
 pub use workspaces::Workspaces;
 
 /// A status bar widget that can be rendered using a [Context]
@@ -49,6 +47,12 @@ where
  /// space will be split evenly between all widgets.
  fn is_greedy(&self) -> bool;
 
+ /// An [UpdateSchedule] to allow for external updates to this Widget's state independently of
+ /// the window manager event loop.
+ fn update_schedule(&mut self) -> Option<UpdateSchedule> {
+ None
+ }
+
  #[allow(unused_variables)]
  /// A startup hook to be run in order to initialise this Widget
  fn on_startup(&mut self, state: &mut State<X>, x: &X) -> Result<()> {
@@ -288,11 +292,13 @@ impl<X: XConn> Widget<X> for RefreshText {
 ///
 /// // Make a curl request to wttr.in to fetch the current weather information
 /// // for our location.
-/// fn my_get_text() -> String {
-/// spawn_for_output_with_args("curl", &["-s", "http://wttr.in?format=3"])
+/// fn my_get_text() -> Option<String> {
+/// let s = spawn_for_output_with_args("curl", &["-s", "http://wttr.in?format=3"])
 /// .unwrap_or_default()
 /// .trim()
-/// .to_string()
+/// .to_string();
+///
+/// Some(s)
 /// }
 ///
 /// let style = TextStyle {
@@ -308,9 +314,19 @@ impl<X: XConn> Widget<X> for RefreshText {
 /// Duration::from_secs(60 * 5)
 /// );
 /// ```
-#[derive(Debug)]
 pub struct IntervalText {
  inner: Arc<Mutex<Text>>,
+ interval: Duration,
+ get_text: Option<Box<dyn Fn() -> Option<String> + Send + 'static>>,
+}
+
+impl fmt::Debug for IntervalText {
+ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+ f.debug_struct("RefreshText")
+ .field("inner", &self.inner)
+ .field("interval", &self.interval)
+ .finish()
+ }
 }
 
 impl IntervalText {
@@ -319,64 +335,47 @@ impl IntervalText {
  /// will be run in its own thread on the interval provided.
  pub fn new<F>(style: TextStyle, get_text: F, interval: Duration) -> Self
  where
- F: Fn() -> String + 'static + Send,
+ F: Fn() -> Option<String> + Send + 'static,
  {
  let inner = Arc::new(Mutex::new(Text::new("", style, false, false)));
- let txt = Arc::clone(&inner);
-
- thread::spawn(move || loop {
- trace!("updating text for IntervalText widget");
- let s = (get_text)();
-
- {
- let mut t = match txt.lock() {
- Ok(inner) => inner,
- Err(poisoned) => poisoned.into_inner(),
- };
- t.set_text(s);
- }
 
- thread::sleep(interval);
- });
+ Self {
+ inner,
+ interval,
+ get_text: Some(Box::new(get_text)),
+ }
+ }
 
- Self { inner }
+ fn inner_guard(&self) -> MutexGuard<'_, Text> {
+ match self.inner.lock() {
+ Ok(inner) => inner,
+ Err(poisoned) => poisoned.into_inner(),
+ }
  }
 }
 
 impl<X: XConn> Widget<X> for IntervalText {
  fn draw(&mut self, ctx: &mut Context<'_>, s: usize, f: bool, w: u32, h: u32) -> Result<()> {
- let mut inner = match self.inner.lock() {
- Ok(inner) => inner,
- Err(poisoned) => poisoned.into_inner(),
- };
-
- Widget::<X>::draw(&mut *inner, ctx, s, f, w, h)
+ Widget::<X>::draw(&mut *self.inner_guard(), ctx, s, f, w, h)
  }
 
  fn current_extent(&mut self, ctx: &mut Context<'_>, h: u32) -> Result<(u32, u32)> {
- let mut inner = match self.inner.lock() {
- Ok(inner) => inner,
- Err(poisoned) => poisoned.into_inner(),
- };
-
- Widget::<X>::current_extent(&mut *inner, ctx, h)
+ Widget::<X>::current_extent(&mut *self.inner_guard(), ctx, h)
  }
 
  fn is_greedy(&self) -> bool {
- let inner = match self.inner.lock() {
- Ok(inner) => inner,
- Err(poisoned) => poisoned.into_inner(),
- };
-
- Widget::<X>::is_greedy(&*inner)
+ Widget::<X>::is_greedy(&*self.inner_guard())
  }
 
  fn require_draw(&self) -> bool {
- let inner = match self.inner.lock() {
- Ok(inner) => inner,
- Err(poisoned) => poisoned.into_inner(),
- };
+ Widget::<X>::require_draw(&*self.inner_guard())
+ }
 
- Widget::<X>::require_draw(&*inner)
+ fn update_schedule(&mut self) -> Option<UpdateSchedule> {
+ Some(UpdateSchedule::new(
+ self.interval,
+ self.get_text.take().unwrap(),
+ self.inner.clone(),
+ ))
  }
 }