refactor: clean stale report when switching, remove u8 repr of ConnectionType

Signed-off-by: Haobo Gu <haobogu@outlook.com>

Author: HaoboGu

rmk-types/src/connection.rs

@@ -16,28 +16,6 @@ pub enum ConnectionType {
     Ble,
 }
 
-/// Unknown stored values default to [`ConnectionType::Usb`] so a downgrade
-/// from a newer firmware that wrote an unknown variant falls back to USB
-/// rather than refusing to boot.
-impl From<u8> for ConnectionType {
-    fn from(value: u8) -> Self {
-        match value {
-            0 => ConnectionType::Usb,
-            1 => ConnectionType::Ble,
-            _ => ConnectionType::Usb,
-        }
-    }
-}
-
-impl From<ConnectionType> for u8 {
-    fn from(value: ConnectionType) -> Self {
-        match value {
-            ConnectionType::Usb => 0,
-            ConnectionType::Ble => 1,
-        }
-    }
-}
-
 /// USB device lifecycle. `Suspended` is distinct from `Configured` because
 /// the bus is enumerated but transmission is gated on remote wakeup — the
 /// first key still needs to reach the USB writer to trigger that wakeup.

rmk/src/ble/profile.rs

@@ -395,9 +395,7 @@ impl<'a, C: Controller + ControllerCmdAsync<LeSetPhy>, P: PacketPool> ProfileMan
                             info!("Switching preferred transport to: {:?}", updated);
 
                             #[cfg(feature = "storage")]
-                            FLASH_CHANNEL
-                                .send(FlashOperationMessage::ConnectionType(updated.into()))
-                                .await;
+                            FLASH_CHANNEL.send(FlashOperationMessage::ConnectionType(updated)).await;
                         }
                     }
                     #[cfg(feature = "storage")]

rmk/src/channel.rs

@@ -12,7 +12,7 @@ use {crate::ble::profile::BleProfileAction, rmk_types::led_indicator::LedIndicat
 
 #[cfg(feature = "host")]
 use crate::VIAL_CHANNEL_SIZE;
-use crate::hid::Report;
+use crate::hid::{KeyboardReport, Report};
 #[cfg(feature = "storage")]
 use crate::{FLASH_CHANNEL_SIZE, storage::FlashOperationMessage};
 use crate::{REPORT_CHANNEL_SIZE, RawMutex};
@@ -32,17 +32,22 @@ pub static USB_REPORT_CHANNEL: ReportChannel = Channel::new();
 #[cfg(feature = "_ble")]
 pub static BLE_REPORT_CHANNEL: ReportChannel = Channel::new();
 
-fn active_report_channel() -> Option<(ConnectionType, &'static ReportChannel)> {
-    match crate::state::active_transport()? {
+fn report_channel(transport: ConnectionType) -> Option<&'static ReportChannel> {
+    match transport {
         #[cfg(not(feature = "_no_usb"))]
-        ConnectionType::Usb => Some((ConnectionType::Usb, &USB_REPORT_CHANNEL)),
+        ConnectionType::Usb => Some(&USB_REPORT_CHANNEL),
         #[cfg(feature = "_ble")]
-        ConnectionType::Ble => Some((ConnectionType::Ble, &BLE_REPORT_CHANNEL)),
+        ConnectionType::Ble => Some(&BLE_REPORT_CHANNEL),
         #[allow(unreachable_patterns)]
         _ => None,
     }
 }
 
+fn active_report_channel() -> Option<(ConnectionType, &'static ReportChannel)> {
+    let transport = crate::state::active_transport()?;
+    report_channel(transport).map(|ch| (transport, ch))
+}
+
 /// Reports generated while no transport is selected are dropped on the floor.
 pub(crate) async fn send_hid_report(mut report: Report) {
     let Some((transport, ch)) = active_report_channel() else {
@@ -71,16 +76,13 @@ pub(crate) fn try_send_hid_report(report: Report) {
     }
 }
 
-/// Drains queued reports for `transport`. Called on active-transport flips so
-/// a future re-activation doesn't replay stale presses without their releases.
-pub(crate) fn clear_report_channel(transport: ConnectionType) {
-    match transport {
-        #[cfg(not(feature = "_no_usb"))]
-        ConnectionType::Usb => USB_REPORT_CHANNEL.clear(),
-        #[cfg(feature = "_ble")]
-        ConnectionType::Ble => BLE_REPORT_CHANNEL.clear(),
-        #[allow(unreachable_patterns)]
-        _ => {}
+/// Drains queued reports for `transport` and leaves an all-up keyboard report
+/// for its writer. Called on active-transport flips so the previous host
+/// releases any pressed keys without replaying stale queued reports later.
+pub(crate) fn clear_and_release_report_channel(transport: ConnectionType) {
+    if let Some(ch) = report_channel(transport) {
+        ch.clear();
+        let _ = ch.try_send(Report::KeyboardReport(KeyboardReport::default()));
     }
 }
 

rmk/src/state.rs

@@ -70,7 +70,7 @@ fn update_status(f: impl FnOnce(&mut ConnectionStatus)) {
         // Drain after the commit so any producer racing past the mutex reads
         // the new state and routes to the new channel rather than the one
         // about to be cleared.
-        crate::channel::clear_report_channel(prev_active);
+        crate::channel::clear_and_release_report_channel(prev_active);
     }
 
     #[cfg(feature = "_ble")]
@@ -119,9 +119,9 @@ pub(crate) async fn load_preferred_connection() -> ConnectionType {
     #[cfg(feature = "storage")]
     let stored = crate::storage::read_connection_type().await;
     #[cfg(not(feature = "storage"))]
-    let stored: Option<u8> = None;
+    let stored: Option<ConnectionType> = None;
     match stored {
-        Some(c) => c.into(),
+        Some(c) => c,
         #[cfg(feature = "_no_usb")]
         None => ConnectionType::Ble,
         #[cfg(not(feature = "_no_usb"))]
@@ -160,6 +160,7 @@ mod tests {
         set_ble_state, set_preferred_connection, set_usb_state,
     };
     use crate::event::{ConnectionChangeEvent, EventSubscriber, SubscribableEvent};
+    use crate::hid::{KeyboardReport, Report};
     use crate::test_support::test_block_on as block_on;
 
     fn state_test_lock() -> &'static Mutex<()> {
@@ -176,6 +177,27 @@ mod tests {
         crate::channel::BLE_REPORT_CHANNEL.clear();
     }
 
+    fn pressed_keyboard_report() -> Report {
+        Report::KeyboardReport(KeyboardReport {
+            modifier: 0x02,
+            reserved: 0,
+            leds: 0,
+            keycodes: [4, 0, 0, 0, 0, 0],
+        })
+    }
+
+    fn assert_all_up_keyboard_report(report: Report) {
+        match report {
+            Report::KeyboardReport(r) => {
+                assert_eq!(r.modifier, 0);
+                assert_eq!(r.reserved, 0);
+                assert_eq!(r.leds, 0);
+                assert_eq!(r.keycodes, [0; 6]);
+            }
+            _ => panic!("expected keyboard all-up report"),
+        }
+    }
+
     #[cfg(not(feature = "_no_usb"))]
     #[test]
     fn usb_builds_keep_input_processing_active_without_transport() {
@@ -223,9 +245,8 @@ mod tests {
 
     #[cfg(not(feature = "_no_usb"))]
     #[test]
-    fn flipping_away_from_active_clears_its_report_channel() {
+    fn flipping_away_from_active_clears_stale_reports_and_queues_all_up() {
         use crate::channel::USB_REPORT_CHANNEL;
-        use crate::hid::{KeyboardReport, Report};
 
         let _guard = state_test_lock().lock().unwrap();
         reset_state();
@@ -236,18 +257,23 @@ mod tests {
         // that would otherwise persist across a flip.
         USB_REPORT_CHANNEL.clear();
         USB_REPORT_CHANNEL
-            .try_send(Report::KeyboardReport(KeyboardReport::default()))
+            .try_send(pressed_keyboard_report())
             .expect("channel should have capacity for sentinel");
         assert!(USB_REPORT_CHANNEL.try_receive().is_ok());
         USB_REPORT_CHANNEL
-            .try_send(Report::KeyboardReport(KeyboardReport::default()))
+            .try_send(pressed_keyboard_report())
             .expect("channel should have capacity for sentinel");
 
         set_usb_state(UsbState::Disabled);
         assert!(super::active_transport().is_none());
+        assert_all_up_keyboard_report(
+            USB_REPORT_CHANNEL
+                .try_receive()
+                .expect("USB_REPORT_CHANNEL should contain keyboard all-up report"),
+        );
         assert!(
             USB_REPORT_CHANNEL.try_receive().is_err(),
-            "USB_REPORT_CHANNEL should be drained when USB stops being active"
+            "USB_REPORT_CHANNEL should contain only the all-up report"
         );
     }
 
@@ -257,15 +283,14 @@ mod tests {
         use embassy_futures::join::join;
 
         use crate::channel::{USB_REPORT_CHANNEL, send_hid_report};
-        use crate::hid::{KeyboardReport, Report};
 
         let _guard = state_test_lock().lock().unwrap();
         reset_state();
         set_usb_state(UsbState::Configured);
 
         for _ in 0..crate::REPORT_CHANNEL_SIZE {
             USB_REPORT_CHANNEL
-                .try_send(Report::KeyboardReport(KeyboardReport::default()))
+                .try_send(pressed_keyboard_report())
                 .expect("channel should have capacity while filling");
         }
 
@@ -277,7 +302,43 @@ mod tests {
             },
         ));
 
-        assert!(USB_REPORT_CHANNEL.try_receive().is_err());
+        assert_all_up_keyboard_report(
+            USB_REPORT_CHANNEL
+                .try_receive()
+                .expect("USB_REPORT_CHANNEL should contain keyboard all-up report"),
+        );
+        assert!(
+            USB_REPORT_CHANNEL.try_receive().is_err(),
+            "USB_REPORT_CHANNEL should contain only the all-up report"
+        );
+    }
+
+    #[cfg(all(not(feature = "_no_usb"), feature = "_ble"))]
+    #[test]
+    fn usb_preference_flip_releases_previous_ble_transport() {
+        use crate::channel::BLE_REPORT_CHANNEL;
+
+        let _guard = state_test_lock().lock().unwrap();
+        reset_state();
+        set_preferred_connection(ConnectionType::Usb);
+        set_ble_state(BleState::Connected);
+        assert_eq!(super::active_transport(), Some(ConnectionType::Ble));
+
+        BLE_REPORT_CHANNEL
+            .try_send(pressed_keyboard_report())
+            .expect("BLE report channel should have capacity for sentinel");
+
+        set_usb_state(UsbState::Configured);
+        assert_eq!(super::active_transport(), Some(ConnectionType::Usb));
+        assert_all_up_keyboard_report(
+            BLE_REPORT_CHANNEL
+                .try_receive()
+                .expect("BLE_REPORT_CHANNEL should contain keyboard all-up report"),
+        );
+        assert!(
+            BLE_REPORT_CHANNEL.try_receive().is_err(),
+            "BLE_REPORT_CHANNEL should contain only the all-up report"
+        );
     }
 
     #[test]
@@ -305,7 +366,6 @@ mod tests {
         use embassy_futures::join::join;
 
         use crate::channel::{USB_REPORT_CHANNEL, send_hid_report};
-        use crate::hid::{KeyboardReport, Report};
 
         let _guard = state_test_lock().lock().unwrap();
         reset_state();

rmk/src/storage/mod.rs

@@ -6,6 +6,7 @@ use embassy_time::Duration;
 use embedded_storage::nor_flash::NorFlash;
 use embedded_storage_async::nor_flash::NorFlash as AsyncNorFlash;
 use postcard::experimental::max_size::MaxSize;
+use rmk_types::connection::ConnectionType;
 use rmk_types::morse::MorseProfile;
 use sequential_storage::Error as SSError;
 use sequential_storage::cache::NoCache;
@@ -40,7 +41,7 @@ static BOND_INFO_RESPONSE: Signal<crate::RawMutex, Option<ProfileInfo>> = Signal
 #[cfg(all(feature = "_ble", feature = "split"))]
 static PEER_ADDRESS_RESPONSE: Signal<crate::RawMutex, Option<PeerAddress>> = Signal::new();
 #[cfg(feature = "_ble")]
-static CONNECTION_TYPE_RESPONSE: Signal<crate::RawMutex, Option<u8>> = Signal::new();
+static CONNECTION_TYPE_RESPONSE: Signal<crate::RawMutex, Option<ConnectionType>> = Signal::new();
 #[cfg(feature = "_ble")]
 static ACTIVE_BLE_PROFILE_RESPONSE: Signal<crate::RawMutex, Option<u8>> = Signal::new();
 
@@ -62,7 +63,7 @@ pub(crate) async fn read_peer_address(peer_id: u8) -> Option<PeerAddress> {
 }
 
 #[cfg(feature = "_ble")]
-pub(crate) async fn read_connection_type() -> Option<u8> {
+pub(crate) async fn read_connection_type() -> Option<ConnectionType> {
     request_read(FlashOperationMessage::ReadConnectionType, &CONNECTION_TYPE_RESPONSE).await
 }
 
@@ -140,7 +141,7 @@ pub(crate) enum FlashOperationMessage {
         morse: Morse,
     },
     // Current saved connection type
-    ConnectionType(u8),
+    ConnectionType(ConnectionType),
     // Timeout time for combos
     ComboTimeout(u16),
     // Timeout time for one-shot keys
@@ -261,7 +262,7 @@ pub(crate) enum StorageData {
     StorageConfig(LocalStorageConfig),
     LayoutConfig(LayoutConfig),
     BehaviorConfig(BehaviorConfig),
-    ConnectionType(u8),
+    ConnectionType(ConnectionType),
     #[cfg(feature = "host")]
     MacroData(#[serde(with = "crate::host::storage::macro_bytes_serde")] [u8; MACRO_SPACE_SIZE]),
     #[cfg(feature = "host")]