From c49d99457240c924bca5c2516db17688a7711a67 Mon Sep 17 00:00:00 2001 From: Rich Neswold Date: Wed, 14 Aug 2024 01:03:50 -0500 Subject: [PATCH 1/6] :wrench: add tests for `.uses_solar()` --- drmemd/src/logic/compile.rs | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/drmemd/src/logic/compile.rs b/drmemd/src/logic/compile.rs index 034e40e..c612544 100644 --- a/drmemd/src/logic/compile.rs +++ b/drmemd/src/logic/compile.rs @@ -2934,4 +2934,40 @@ mod tests { ); } } + + #[test] + fn test_solar_usage() { + const DATA: &[(&str, bool)] = &[ + // Make sure literals, variables, and time values don't + // return a field. + ("{a}", false), + ("1", false), + ("1.0", false), + ("true", false), + ("#green", false), + ("\"test\"", false), + ("{utc:second}", false), + // Make sure the solar values return true. + ("{solar:alt}", true), + ("{solar:dec}", true), + ("{solar:ra}", true), + ("{solar:az}", true), + // Now test more complicated expressions to make sure each + // subtree is correctly compared. + ("not (2 > 3)", false), + ("2 + 2", false), + ("{solar:alt} + 2", true), + ("2 + {solar:az}", true), + ("{solar:dec} + {solar:az}", true), + ]; + + for (expr, result) in DATA { + assert_eq!( + &to_expr(expr).uses_solar(), + result, + "error using {}", + expr + ); + } + } } From 8d71f9f54ecd1e4621105786d2883bfd97f27642 Mon Sep 17 00:00:00 2001 From: Rich Neswold Date: Wed, 14 Aug 2024 01:05:58 -0500 Subject: [PATCH 2/6] :recycle: adjust the loop in logic blocks - The `select!` macro uses the `biased` option so clauses are evaluated in order. The time-of-day and solar futures are placed first because they happen infrequently and we don't want high-speed devices starving these channels. - For the solar channel, we now do more error reporting. This helped uncover a bug (fixed in a different commit.) - Rather than returning Futures that return None, we now use the never resolving future. --- drmemd/src/logic/mod.rs | 54 ++++++++++++++++++++++++++--------------- 1 file changed, 34 insertions(+), 20 deletions(-) diff --git a/drmemd/src/logic/mod.rs b/drmemd/src/logic/mod.rs index d3b835c..cb34479 100644 --- a/drmemd/src/logic/mod.rs +++ b/drmemd/src/logic/mod.rs @@ -1,5 +1,5 @@ use drmem_api::{client, device, driver, Result}; -use futures::future::join_all; +use futures::future::{join_all, pending}; use std::collections::HashMap; use std::convert::Infallible; use std::sync::Arc; @@ -395,7 +395,7 @@ impl Node { let wait_for_time = async { match self.time_ch.as_mut() { - None => None, + None => pending().await, Some(s) => s.next().await, } }; @@ -407,23 +407,32 @@ impl Node { let wait_for_solar = async { match self.solar_ch.as_mut() { - None => None, - Some(ch) => ch.recv().await.ok(), + None => pending().await, + Some(ch) => ch.recv().await, } }; #[rustfmt::skip] tokio::select! { - // Wait for the next reading to arrive. All the - // incoming streams have been combined into one and - // the returned value is a pair consisting of an index - // and the actual reading. + biased; - Some((idx, reading)) = self.in_stream.next() => { - // Save the reading in our array for future - // recalculations. + // If we need the solar channel, wait for the next + // update. - self.inputs[idx] = Some(reading.value); + v = wait_for_solar => { + match v { + Ok(v) => solar = Some(v), + Err(broadcast::error::RecvError::Lagged(_)) => { + warn!("not handling solar info fast enough"); + continue + } + Err(broadcast::error::RecvError::Closed) => { + error!("solar info channel is closed"); + return Err(drmem_api::Error::OperationError( + "solar channel closed".into() + )); + } + } } // If we need the time channel, wait for the next @@ -433,11 +442,16 @@ impl Node { time = v; } - // If we need the solar channel, wait for the next - // update. + // Wait for the next reading to arrive. All the + // incoming streams have been combined into one and + // the returned value is a pair consisting of an index + // and the actual reading. - Some(v) = wait_for_solar => { - solar = Some(v); + Some((idx, reading)) = self.in_stream.next() => { + // Save the reading in our array for future + // recalculations. + + self.inputs[idx] = Some(reading.value); } } @@ -557,16 +571,16 @@ mod test { // Create the common channels used by DrMem. let (tx_req, mut c_recv) = mpsc::channel(100); - let (tx_tod, rx_tod) = broadcast::channel(100); - let (tx_solar, rx_solar) = broadcast::channel(100); + let (tx_tod, _) = broadcast::channel(100); + let (tx_solar, _) = broadcast::channel(100); // Start the logic block with the proper communciation // channels and configuration. let node = Node::start( client::RequestChan::new(tx_req), - rx_tod, - rx_solar, + tx_tod.subscribe(), + tx_solar.subscribe(), cfg, ); From 1979088a9808793559d3fb6553edf742fba346e6 Mon Sep 17 00:00:00 2001 From: Rich Neswold Date: Wed, 14 Aug 2024 01:09:39 -0500 Subject: [PATCH 3/6] :wrench: add test for logic block This tests a logic block that only uses solar parameters for inputs. --- drmemd/src/logic/mod.rs | 56 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/drmemd/src/logic/mod.rs b/drmemd/src/logic/mod.rs index cb34479..19a852f 100644 --- a/drmemd/src/logic/mod.rs +++ b/drmemd/src/logic/mod.rs @@ -968,6 +968,62 @@ mod test { assert_eq!(emu.await.unwrap(), Ok(true)); } + // Test a basic logic block in which forwards a solar parameter to + // a memory device. + + #[tokio::test] + async fn test_basic_solar_node() { + const OUT1: &str = "device:out1"; + const OUT2: &str = "device:out2"; + let cfg = build_config( + &[], + &[("alt", OUT1), ("dec", OUT2)], + &[], + &["{solar:alt} -> {alt}", "{solar:dec} -> {dec}"], + ); + let (tx_out1, mut rx_out1) = mpsc::channel(100); + let (tx_out2, mut rx_out2) = mpsc::channel(100); + + let (_, tx_solar, emu, tx_stop) = Emulator::start( + vec![], + vec![(OUT1.into(), tx_out1), (OUT2.into(), tx_out2)], + cfg, + ) + .await + .unwrap(); + + // Send a value and see if it was forwarded. + + assert!(tx_solar + .send(Arc::new(solar::SolarInfo { + elevation: 1.0, + azimuth: 2.0, + right_ascension: 3.0, + declination: 4.0 + })) + .is_ok()); + + { + let (value, rpy) = rx_out1.recv().await.unwrap(); + let _ = rpy.send(Ok(value.clone())); + + assert_eq!(value, device::Value::Flt(1.0)); + } + + { + let (value, rpy) = rx_out2.recv().await.unwrap(); + let _ = rpy.send(Ok(value.clone())); + + assert_eq!(value, device::Value::Flt(4.0)); + } + + // Stop the emulator and see that its return status is good. + + let _ = tx_stop.send(()); + + assert_eq!(emu.await.unwrap(), Ok(true)); + } + // Test a logic block with two outputs. Make sure they are sent // "in parallel". From e205b392c8602d443a8a8605323e639184dddf61 Mon Sep 17 00:00:00 2001 From: Rich Neswold Date: Wed, 14 Aug 2024 01:11:17 -0500 Subject: [PATCH 4/6] :bug: make channel larger When this channel was only 1 deep, logic blocks would occasionally get a Lagging error which would cause them to panic. The logic block code was improved, but the channel was enlarged so these error wouldn't occur under normal circumstances. --- drmemd/src/logic/solar.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drmemd/src/logic/solar.rs b/drmemd/src/logic/solar.rs index 1d2dbfe..b43d0ad 100644 --- a/drmemd/src/logic/solar.rs +++ b/drmemd/src/logic/solar.rs @@ -135,7 +135,7 @@ pub fn create_task( lat: f64, long: f64, ) -> (broadcast::Sender, broadcast::Receiver) { - let (tx, rx) = broadcast::channel(1); + let (tx, rx) = broadcast::channel(10); let tx_copy = tx.clone(); tokio::spawn( From f88d402e0a6e3cde785443f262d59f432caf5f3d Mon Sep 17 00:00:00 2001 From: Rich Neswold Date: Wed, 14 Aug 2024 01:13:33 -0500 Subject: [PATCH 5/6] :recycle: rewrite section A section was placed in a nested scope. This allowed us to remove two calls to `std::mem::drop()`. --- drmemd/src/main.rs | 47 ++++++++++++++++++++++------------------------ 1 file changed, 22 insertions(+), 25 deletions(-) diff --git a/drmemd/src/main.rs b/drmemd/src/main.rs index f076c1f..108e396 100644 --- a/drmemd/src/main.rs +++ b/drmemd/src/main.rs @@ -156,37 +156,34 @@ async fn run() -> Result<()> { } } - // Start the time-of-day task. This needs to be done *before* - // any logic blocks are started because logic blocks *may* - // have an expression that uses the time-of-day. + // Create a nested scope so that the tod and solar handles are + // freed up. - let (tx_tod, rx_tod) = logic::tod::create_task(); - - // Start the solar task. This, too, needs to be done before - // any logic blocks are started. + { + // Start the time-of-day task. This needs to be done + // *before* any logic blocks are started because logic + // blocks *may* have an expression that uses the + // time-of-day. - let (tx_solar, rx_solar) = - logic::solar::create_task(cfg.latitude, cfg.longitude); + let (tx_tod, _) = logic::tod::create_task(); - // Iterate through the [[logic]] sections of the config. + // Start the solar task. This, too, needs to be done + // before any logic blocks are started. - for logic in cfg.logic { - tasks.push(wrap_task(logic::Node::start( - tx_clnt_req.clone(), - tx_tod.subscribe(), - tx_solar.subscribe(), - logic, - ))); - } + let (tx_solar, _) = + logic::solar::create_task(cfg.latitude, cfg.longitude); - // Now that we've given all the logic blocks receive handles - // for the time-of-day and solar tasks, we can free up our - // copy. If we freed up our copy *before* creating new - // subscriptions, the tod or solar task may have briefly seen - // no clients and would exit. + // Iterate through the [[logic]] sections of the config. - std::mem::drop(rx_tod); - std::mem::drop(rx_solar); + for logic in cfg.logic { + tasks.push(wrap_task(logic::Node::start( + tx_clnt_req.clone(), + tx_tod.subscribe(), + tx_solar.subscribe(), + logic, + ))); + } + } // Now run all the tasks. From a6acbf23298b4c5a447b77df4b77d254dac0ba36 Mon Sep 17 00:00:00 2001 From: Rich Neswold Date: Wed, 14 Aug 2024 01:24:17 -0500 Subject: [PATCH 6/6] :label: bump `drmemd` to v0.4.1 --- drmemd/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drmemd/Cargo.toml b/drmemd/Cargo.toml index 43960ec..819e284 100644 --- a/drmemd/Cargo.toml +++ b/drmemd/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "drmemd" -version = "0.4.0" +version = "0.4.1" authors = ["Rich Neswold "] edition = "2021" description = "Main process of the DrMem control system"