distinguish between change and notify commands

gem-remotes-esp32/src/ble_server.rs

@@ -14,6 +14,9 @@ const BLE_MAX_INTERVAL: u16 = 48; // x 1.25ms
 const BLE_LATENCY: u16 = 0; // Number of packets that can be missed, extending interval
 const BLE_TIMEOUT: u16 = 500; // x10ms
 
+const BUTTON_PRESSED: [u8; 1] = [0x1];
+const BUTTON_RELEASED: [u8; 1] = [0x0];
+
 const DEVICE_NAME: &str = "Gem Remotes";
 
 const UUID_SERVICE_PAIR: BleUuid = uuid128!("9966ad5a-f13c-4b61-ba66-0861e08d09b4");
@@ -31,9 +34,13 @@ pub struct BleServer {
 impl BleServer {
     pub fn new(dp: &mut Dispatch) -> Self {
         let cmds = vec![
-            Commands::BluetoothUp { data: 0 },
+            // Switch to getting and updating notices; use the command version for sending commands only.
-            Commands::BluetoothDown { data: 0 },
+            //Commands::BluetoothUp { data: 0 },
-            Commands::BluetoothStop { data: 0 },
+            //Commands::BluetoothDown { data: 0 },
+            //Commands::BluetoothStop { data: 0 },
+            Commands::NotifyMotorDown,
+            Commands::NotifyMotorStop,
+            Commands::NotifyMotorUp,
         ];
         let r = dp.get_callback_channel(&cmds);
         let s = dp.get_cmd_channel();
@@ -45,11 +52,9 @@ impl BleServer {
 
     pub async fn run(&mut self) -> Result<()> {
         match self.do_run().await {
-            Ok(_) => {}
+            Ok(_) => {error!("Exited bluetooth server wait loop with no error.");panic!();}
-            Err(e) => {error!("Bluetooth task encountered error {}", e);}
+            Err(e) => {error!("Bluetooth task encountered error {}", e);panic!();}
         }
-        Ok(()) //TODO this is not ok; reboot the chip!
-        //TODO: we need a structure like this at each spawn point;; apparently functions don't pass errors up tasks.
     }
 
     pub async fn do_run(&mut self) -> Result<()> {
@@ -71,7 +76,7 @@ impl BleServer {
         );
         button_up.lock().set_value(&[0])
         .on_write(closure!(clone sender, |args: &mut OnWriteArgs| {
-            on_single_byte(&sender, args, Commands::BluetoothUp {data: 0})
+            on_bluetooth_cmd(&sender, args, Commands::BluetoothUp {data: 0})
         }));
 
         let button_down = lift_service.lock().create_characteristic(
@@ -80,7 +85,7 @@ impl BleServer {
         );
         button_down.lock().set_value(&[0])
         .on_write(closure!(clone sender, |args: &mut OnWriteArgs| {
-            on_single_byte(&sender, args, Commands::BluetoothDown {data: 0})
+            on_bluetooth_cmd(&sender, args, Commands::BluetoothDown {data: 0})
         }));
 
         let button_stop = lift_service.lock().create_characteristic(
@@ -89,7 +94,7 @@ impl BleServer {
         );
         button_stop.lock().set_value(&[1])
         .on_write(closure!(clone sender, |args: &mut OnWriteArgs| {
-            on_single_byte(&sender, args, Commands::BluetoothStop {data: 0})
+            on_bluetooth_cmd(&sender, args, Commands::BluetoothStop {data: 0})
         }));
 
 
@@ -99,26 +104,29 @@ impl BleServer {
         advertise_pairing(ble_advertiser)?;
         
         loop {
-            debug!("Waiting for updates");
+            debug!("Waiting for updates that should be notified via bluetooth");
             let cmd = self.recv_q.recv().await?;
             trace!("Received update to bluetooth variable {:?}", cmd);
             match cmd {
-                Commands::BluetoothUp { data } => {
+                // TODO: This logic (if one button is pressed others are released) could be done in app instead.
-                    //TODO: this sends a notify even if the command initially came from phone.  Is this correct?
+                Commands::NotifyMotorUp => {
-                    trace!("Updating BluetoothUp with {:?}", data);
+                    button_up.lock().set_value(&BUTTON_PRESSED).notify();
-                    button_up.lock().set_value(&[data]).notify();
+                    button_down.lock().set_value(&BUTTON_RELEASED).notify();
+                    button_stop.lock().set_value(&BUTTON_RELEASED).notify();
                 }
-                Commands::BluetoothDown { data } => {
+                Commands::NotifyMotorDown => {
-                    trace!("Updating BluetoothDown with {:?}", data);
+                    button_up.lock().set_value(&BUTTON_RELEASED).notify();
-                    button_down.lock().set_value(&[data]).notify();
+                    button_down.lock().set_value(&BUTTON_PRESSED).notify();
+                    button_stop.lock().set_value(&BUTTON_RELEASED).notify();
                 }
-                Commands::BluetoothStop { data} => {
+                Commands::NotifyMotorStop => {
-                    trace!("Updating BluetoothStop with {:?}", data);
+                    button_up.lock().set_value(&BUTTON_RELEASED).notify();
-                    button_stop.lock().set_value(&[data]).notify();
+                    button_down.lock().set_value(&BUTTON_RELEASED).notify();
-
+                    button_stop.lock().set_value(&BUTTON_PRESSED).notify();
                 }
                 _ => {
                     error!("Invalid command received by bluetooth handler {:?}", cmd);
+                    // No need to reboot as state is recoverable.
                 }
             }
         }
@@ -126,26 +134,30 @@ impl BleServer {
 
 }
 
-fn on_single_byte(sender: &SendQ, args: &mut OnWriteArgs, cmd: Commands) {
+fn on_bluetooth_cmd(sender: &SendQ, args: &mut OnWriteArgs, cmd: Commands) {
     let v = args.recv_data();
-    //TODO: add "update" versions of these commands, instead of the "got changes from bluetooth" versions, and handle those instead.
+    // receiving incorrect data isn't fatal, but being unable to send events is.
-    match cmd {
+    let attempt = match cmd {
         Commands::BluetoothUp { data: _ } => {
             if v.len() > 0 {
-                sender.send_blocking(Commands::BluetoothUp {data: v[0]} ).ok();
+                sender.send_blocking(Commands::BluetoothUp {data: v[0]} )
-            } else {error!("Received zero-byte bluetooth characteristic update {:?}", cmd)}
+            } else {error!("Received zero-byte bluetooth characteristic update {:?}", cmd); Ok(())}
         }
         Commands::BluetoothDown { data: _ } => {
             if v.len() > 0 {
-                sender.send_blocking(Commands::BluetoothDown {data: v[0]} ).ok();
+                sender.send_blocking(Commands::BluetoothDown {data: v[0]} )
-            } else {error!("Received zero-byte bluetooth characteristic update {:?}", cmd)}
+            } else {error!("Received zero-byte bluetooth characteristic update {:?}", cmd); Ok(())}
         }
         Commands::BluetoothStop { data: _ } => {
             if v.len() > 0 {
-                sender.send_blocking(Commands::BluetoothStop {data: v[0]} ).ok();
+                sender.send_blocking(Commands::BluetoothStop {data: v[0]} )
-            } else {error!("Received zero-byte bluetooth characteristic update {:?}", cmd)}
+            } else {error!("Received zero-byte bluetooth characteristic update {:?}", cmd); Ok(())}
         }
-        _ => {error!("Tried to handle an unknown bluetooth command: {:?}",cmd);}
+        _ => {error!("Tried to handle an unknown bluetooth command: {:?}",cmd);Ok(())}
+    };
+    match attempt {
+        Ok(_) => {}
+        Err(_) => {panic!("Unable to send notifications of bluetooth events")}
     }
 }
 

gem-remotes-esp32/src/main.rs

@@ -1,15 +1,13 @@
 const BUTTON_HOLD_TIME_MS: u64 = 500;
 const STOP_SAFETY_TIME_MS: u64 = 2000;
 
-// Debug crates
-//use esp_idf_svc::timer::EspTaskTimerService;
-//use core::time::Duration;
-
 // Crates used in release
 use log::*; //{trace, debug, info, warn, error}
 use anyhow::Result;
 use async_executor::Executor;
 use futures_lite::future;
+use std::panic;
+use std::ops::Deref;
 
 // Debug modules
 mod test_console;
@@ -28,7 +26,6 @@ use crate::commands::Commands;
 
 //TODO: limit switch driver, would be good in long run if it checked limit switches periodically (every 1s?) to ensure they are still functioning
 
-
 fn main() {
     // Do basic initialization
     esp_idf_svc::sys::link_patches();
@@ -36,6 +33,22 @@ fn main() {
     log::set_max_level(log::LevelFilter::Trace);
     //log::set_max_level(log::LevelFilter::Info);
 
+    // Set panic to use error! instead of write! (as we will potentially log errors)
+    panic::set_hook(Box::new(|panic_info| {
+        let (filename, line) =
+            panic_info.location().map(|loc| (loc.file(), loc.line()))
+                .unwrap_or(("<unknown>", 0));
+
+        let cause = panic_info.payload().downcast_ref::<String>().map(String::deref);
+
+        let cause = cause.unwrap_or_else(||
+            panic_info.payload().downcast_ref::<&str>().map(|s| *s)
+                .unwrap_or("<cause unknown>")
+        );
+
+        error!("A panic occurred at {}:{}: {}", filename, line, cause);
+    }));
+
     match future::block_on(main_loop()) {
         Ok(_) => {error!("Exited main loop with no errors, but this should not happen."); panic!();}
         Err(e) => {error!("Exited main loop with error {}", e); panic!();}