add mode to further improve input latency while maintaining accuracy (#8)

This feels even hackier than consistency mode... we hold off on writing USB reports until the button state actually changes. Passes the input integrity benchmark with ~99.6%, but that doesn't mean much since the challenge is preserving input integrity for inputs less than 8.33ms apart.

Current rate limiter looks good from initial measurements, so will probably merge this at some point and leave it up to users to try the mode out or not. For now, I'd still recommend regular consistency mode, it's still hacky, but way less than this.

Also, not breaking because the new enum for input consistency mode is backward compatible with the bool.

Reviewed-on: #8

.forgejo/workflows/clippy-check.yml

@@ -3,7 +3,7 @@ name: Code quality
 on: pull_request
 
 concurrency:
-  group: "${{ github.ref }}"
+  group: ${{ gitea.ref }}
   cancel-in-progress: true
 
 jobs:

src/config.rs

@@ -10,12 +10,13 @@ use embassy_rp::{
     flash::{Async, Flash, ERASE_SIZE},
     peripherals::FLASH,
 };
-use packed_struct::{derive::PackedStruct, PackedStruct};
+use packed_struct::{
+    derive::{PackedStruct, PrimitiveEnum_u8},
+    PackedStruct,
+};
 
 use crate::{
-    gcc_hid::{
+    gcc_hid::{Buttons1, Buttons2, MUTEX_INPUT_CONSISTENCY_MODE, SIGNAL_CHANGE_RUMBLE_STRENGTH},
-        Buttons1, Buttons2, SIGNAL_CHANGE_RUMBLE_STRENGTH, SIGNAL_INPUT_CONSISTENCY_MODE_STATUS,
-    },
     helpers::{PackedFloat, ToPackedFloatArray, ToRegularArray, XyValuePair},
     input::{
         read_ext_adc, Stick, StickAxis, FLOAT_ORIGIN, SPI_ACS_SHARED, SPI_CCS_SHARED, SPI_SHARED,
@@ -544,6 +545,19 @@ impl Default for StickConfig {
     }
 }
 
+#[derive(Debug, Clone, Copy, Format, PrimitiveEnum_u8, PartialEq, Eq)]
+pub enum InputConsistencyMode {
+    /// Transmit inputs every 8ms, same as the original GCC adapter (and any other).
+    Original = 0,
+    /// Forcibly delay transmissions to be 8.33ms apart, to better align with the game's frame rate.
+    ConsistencyHack = 1,
+    /// Transmit inputs _at most_ every 8.33ms, but don't transmit anything at all if the controller state doesn't change.
+    /// This has the potential to drastically improve latency in certain situations, such as when you are waiting to react
+    /// to something your opponent does.
+    /// The name is not meant to imply that this is a hack that is super, but rather that this is super hacky.
+    SuperHack = 2,
+}
+
 #[derive(Debug, Clone, Format, PackedStruct)]
 #[packed_struct(endian = "msb")]
 pub struct ControllerConfig {
@@ -553,8 +567,8 @@ pub struct ControllerConfig {
     /// will trick the Switch into updating the state every 8.33ms
     /// instead of every 8ms. The tradeoff is a slight increase in
     /// input lag.
-    #[packed_field(size_bits = "8")]
+    #[packed_field(size_bits = "8", ty = "enum")]
-    pub input_consistency_mode: bool,
+    pub input_consistency_mode: InputConsistencyMode,
     #[packed_field(size_bits = "8")]
     pub rumble_strength: u8,
     #[packed_field(size_bytes = "328")]
@@ -567,7 +581,7 @@ impl Default for ControllerConfig {
     fn default() -> Self {
         Self {
             config_revision: CONTROLLER_CONFIG_REVISION,
-            input_consistency_mode: true,
+            input_consistency_mode: InputConsistencyMode::ConsistencyHack,
             astick_config: StickConfig::default(),
             rumble_strength: 9,
             cstick_config: StickConfig::default(),
@@ -1645,7 +1659,11 @@ async fn configuration_main_loop<
             }
             // input consistency toggle
             37 => {
-                final_config.input_consistency_mode = !final_config.input_consistency_mode;
+                final_config.input_consistency_mode = match final_config.input_consistency_mode {
+                    InputConsistencyMode::Original => InputConsistencyMode::ConsistencyHack,
+                    InputConsistencyMode::ConsistencyHack => InputConsistencyMode::SuperHack,
+                    InputConsistencyMode::SuperHack => InputConsistencyMode::Original,
+                };
 
                 override_gcc_state_and_wait(&OverrideGcReportInstruction {
                     report: GcReport {
@@ -1665,8 +1683,9 @@ async fn configuration_main_loop<
                         stick_x: 127,
                         stick_y: (127_i8
                             + match final_config.input_consistency_mode {
-                                true => 69,
+                                InputConsistencyMode::Original => -69,
-                                false => -69,
+                                InputConsistencyMode::ConsistencyHack => 42,
+                                InputConsistencyMode::SuperHack => 69,
                             }) as u8,
                         cstick_x: 127,
                         cstick_y: 127,
@@ -1784,7 +1803,11 @@ pub async fn config_task(mut flash: Flash<'static, FLASH, Async, FLASH_SIZE>) {
 
     let mut current_config = ControllerConfig::from_flash_memory(&mut flash).unwrap();
 
-    SIGNAL_INPUT_CONSISTENCY_MODE_STATUS.signal(current_config.input_consistency_mode);
+    {
+        let mut m_input_consistency = MUTEX_INPUT_CONSISTENCY_MODE.lock().await;
+        *m_input_consistency = Some(current_config.input_consistency_mode);
+    }
+
     SIGNAL_CHANGE_RUMBLE_STRENGTH.signal(current_config.rumble_strength);
     SIGNAL_CONFIG_CHANGE.signal(current_config.clone());
 

src/gcc_hid.rs

@@ -12,8 +12,8 @@ use embassy_rp::{
     usb::Driver,
 };
 
-use embassy_sync::{blocking_mutex::raw::CriticalSectionRawMutex, signal::Signal};
+use embassy_sync::{blocking_mutex::raw::CriticalSectionRawMutex, mutex::Mutex, signal::Signal};
-use embassy_time::{Duration, Instant, Ticker};
+use embassy_time::{Duration, Instant, Ticker, Timer};
 use embassy_usb::{
     class::hid::{HidReaderWriter, ReportId, RequestHandler, State},
     control::OutResponse,
@@ -22,7 +22,7 @@ use embassy_usb::{
 use libm::powf;
 use packed_struct::{derive::PackedStruct, PackedStruct};
 
-use crate::input::CHANNEL_GCC_STATE;
+use crate::{config::InputConsistencyMode, input::CHANNEL_GCC_STATE};
 
 static SIGNAL_RUMBLE: Signal<CriticalSectionRawMutex, bool> = Signal::new();
 
@@ -31,8 +31,10 @@ static SIGNAL_RUMBLE: Signal<CriticalSectionRawMutex, bool> = Signal::new();
 pub static SIGNAL_CHANGE_RUMBLE_STRENGTH: Signal<CriticalSectionRawMutex, u8> = Signal::new();
 
 /// Only dispatched ONCE after powerup, to determine how to advertise itself via USB.
-pub static SIGNAL_INPUT_CONSISTENCY_MODE_STATUS: Signal<CriticalSectionRawMutex, bool> =
+pub static MUTEX_INPUT_CONSISTENCY_MODE: Mutex<
-    Signal::new();
+    CriticalSectionRawMutex,
+    Option<InputConsistencyMode>,
+> = Mutex::new(None);
 
 #[rustfmt::skip]
 pub const GCC_REPORT_DESCRIPTOR: &[u8] = &[
@@ -266,7 +268,12 @@ impl Handler for MyDeviceHandler {
 
 #[embassy_executor::task]
 pub async fn usb_transfer_task(raw_serial: [u8; 8], driver: Driver<'static, USB>) {
-    let input_consistency_mode = SIGNAL_INPUT_CONSISTENCY_MODE_STATUS.wait().await;
+    let input_consistency_mode = {
+        while MUTEX_INPUT_CONSISTENCY_MODE.lock().await.is_none() {
+            Timer::after(Duration::from_millis(100)).await;
+        }
+        MUTEX_INPUT_CONSISTENCY_MODE.lock().await.unwrap()
+    };
 
     let mut serial_buffer = [0u8; 64];
 
@@ -291,10 +298,10 @@ pub async fn usb_transfer_task(raw_serial: [u8; 8], driver: Driver<'static, USB>
     trace!("Start of config");
     let mut usb_config = embassy_usb::Config::new(0x057e, 0x0337);
     usb_config.manufacturer = Some("Naxdy");
-    usb_config.product = Some(if input_consistency_mode {
+    usb_config.product = Some(match input_consistency_mode {
-        "NaxGCC (Consistency Mode)"
+        InputConsistencyMode::Original => "NaxGCC (OG Mode)",
-    } else {
+        InputConsistencyMode::ConsistencyHack => "NaxGCC (Consistency Mode)",
-        "NaxGCC (OG Mode)"
+        InputConsistencyMode::SuperHack => "NaxGCC (SuperHack Mode)",
     });
     usb_config.serial_number = Some(serial);
     usb_config.max_power = 200;
@@ -331,7 +338,7 @@ pub async fn usb_transfer_task(raw_serial: [u8; 8], driver: Driver<'static, USB>
     let hid_config = embassy_usb::class::hid::Config {
         report_descriptor: GCC_REPORT_DESCRIPTOR,
         request_handler: Some(&request_handler),
-        poll_ms: if input_consistency_mode { 4 } else { 8 },
+        poll_ms: 8,
         max_packet_size_in: 37,
         max_packet_size_out: 5,
     };
@@ -350,25 +357,37 @@ pub async fn usb_transfer_task(raw_serial: [u8; 8], driver: Driver<'static, USB>
 
     let (mut reader, mut writer) = hid.split();
 
-    let mut lasttime = Instant::now();
-
     let in_fut = async {
         let mut gcc_subscriber = CHANNEL_GCC_STATE.subscriber().unwrap();
 
+        let mut last_report_time = Instant::now();
         let mut ticker = Ticker::every(Duration::from_micros(8333));
 
         loop {
-            if input_consistency_mode {
+            // This is what we like to call a "hack".
-                // This is what we like to call a "hack".
+            // It forces reports to be sent at least every 8.33ms instead of every 8ms.
-                // It forces reports to be sent every 8.33ms instead of every 8ms.
+            // 8.33ms is a multiple of the game's frame interval (16.66ms), so if we
-                // 8.33ms is a multiple of the game's frame interval (16.66ms), so if we
+            // send a report every 8.33ms, it should (in theory) ensure (close to)
-                // send a report every 8.33ms, it should (in theory) ensure (close to)
+            // 100% input accuracy.
-                // 100% input accuracy.
+            //
-                //
+            // From the console's perspective, we are basically a laggy adapter, taking
-                // From the console's perspective, we are basically a laggy adapter, taking
+            // a minimum of 333 extra us to send a report every time it's polled, but it
-                // a minimum of 333 extra us to send a report every time it's polled, but it
+            // works to our advantage.
-                // works to our advantage.
+            match input_consistency_mode {
-                ticker.next().await;
+                InputConsistencyMode::SuperHack => {
+                    // In SuperHack mode, we send reports only if the state changes, but
+                    // in order to not mess up very fast inputs (like sticks travelling, for example),
+                    // we still need to "rate limit" the reports to every 8.33ms at most.
+                    // This does rate limit it to ~8.33ms fairly well, my only
+                    // gripe with it is that I hate it :)
+                    Timer::at(last_report_time + Duration::from_micros(8100)).await;
+                }
+                InputConsistencyMode::ConsistencyHack => {
+                    // Ticker better maintains a consistent interval than Timer, so
+                    // we prefer it for consistency mode, where we send reports regularly.
+                    ticker.next().await;
+                }
+                InputConsistencyMode::Original => {}
             }
 
             match writer
@@ -382,15 +401,17 @@ pub async fn usb_transfer_task(raw_serial: [u8; 8], driver: Driver<'static, USB>
             {
                 Ok(()) => {
                     let currtime = Instant::now();
-                    let polltime = currtime.duration_since(lasttime);
+                    let polltime = currtime.duration_since(last_report_time);
                     let micros = polltime.as_micros();
-                    trace!("Report written in {}us", micros);
+                    debug!("Report written in {}us", micros);
-                    // If we're sending reports too fast, reset the ticker.
+                    // If we're sending reports too fast in regular consistency mode, reset the ticker.
                     // This might happen right after plug-in, or after suspend.
-                    if micros < 8150 {
+                    if input_consistency_mode == InputConsistencyMode::ConsistencyHack
-                        ticker.reset();
+                        && micros < 8150
+                    {
+                        ticker.reset()
                     }
-                    lasttime = currtime;
+                    last_report_time = currtime;
                 }
                 Err(e) => warn!("Failed to send report: {:?}", e),
             }

src/input.rs

@@ -16,11 +16,12 @@ use libm::{fmaxf, fminf};
 
 use crate::{
     config::{
-        ControllerConfig, OverrideGcReportInstruction, OverrideStickState, SIGNAL_CONFIG_CHANGE,
+        ControllerConfig, InputConsistencyMode, OverrideGcReportInstruction, OverrideStickState,
-        SIGNAL_IS_CALIBRATING, SIGNAL_OVERRIDE_GCC_STATE, SIGNAL_OVERRIDE_STICK_STATE,
+        SIGNAL_CONFIG_CHANGE, SIGNAL_IS_CALIBRATING, SIGNAL_OVERRIDE_GCC_STATE,
+        SIGNAL_OVERRIDE_STICK_STATE,
     },
     filter::{run_waveshaping, FilterGains, KalmanState, WaveshapingValues, FILTER_GAINS},
-    gcc_hid::GcReport,
+    gcc_hid::{GcReport, MUTEX_INPUT_CONSISTENCY_MODE},
     helpers::XyValuePair,
     input_filter::{DummyFilter, InputFilter},
     stick::{linearize, notch_remap, StickParams},
@@ -439,6 +440,15 @@ pub async fn update_button_state_task(
         loop {}
     }
 
+    let input_consistency_mode = {
+        while MUTEX_INPUT_CONSISTENCY_MODE.lock().await.is_none() {
+            Timer::after(Duration::from_millis(100)).await;
+        }
+        MUTEX_INPUT_CONSISTENCY_MODE.lock().await.unwrap()
+    };
+
+    let mut previous_state = GcReport::default();
+
     let mut gcc_state = GcReport::default();
 
     let gcc_publisher = CHANNEL_GCC_STATE.publisher().unwrap();
@@ -483,7 +493,15 @@ pub async fn update_button_state_task(
             trace!("Overridden gcc state: {:?}", override_gcc_state.report);
             let end_time = Instant::now() + Duration::from_millis(override_gcc_state.duration_ms);
             while Instant::now() < end_time {
-                gcc_publisher.publish_immediate(override_gcc_state.report);
+                if input_consistency_mode == InputConsistencyMode::SuperHack {
+                    if override_gcc_state.report != previous_state {
+                        gcc_publisher.publish_immediate(override_gcc_state.report);
+                        previous_state = override_gcc_state.report;
+                    }
+                } else {
+                    gcc_publisher.publish_immediate(override_gcc_state.report);
+                }
+
                 yield_now().await;
             }
         };
@@ -503,7 +521,14 @@ pub async fn update_button_state_task(
             gcc_publisher.publish_immediate(overriden_gcc_state);
         } else {
             input_filter.apply_filter(&mut gcc_state);
-            gcc_publisher.publish_immediate(gcc_state);
+            if input_consistency_mode == InputConsistencyMode::SuperHack {
+                if gcc_state != previous_state {
+                    gcc_publisher.publish_immediate(gcc_state);
+                    previous_state = gcc_state;
+                }
+            } else {
+                gcc_publisher.publish_immediate(gcc_state);
+            }
         }
 
         // give other tasks a chance to do something

src/main.rs

@@ -104,21 +104,6 @@ fn main() -> ! {
         });
     });
 
-    // Stick loop has to run on core0 because it makes use of SPI0.
-    // Perhaps in the future we can rewire the board to have it make use of SPI1 instead.
-    // This way it could be the sole task running on core1, and everything else could happen on core0.
-    // Also, it needs to run on a higher prio executor to ensure consistent polling.
-    // interrupt::SWI_IRQ_1.set_priority(interrupt::Priority::P0);
-    // let spawner_high = EXECUTOR_HIGH.start(interrupt::SWI_IRQ_1);
-    // spawner_high
-    //     .spawn(update_stick_states_task(
-    //         spi,
-    //         spi_acs,
-    //         spi_ccs,
-    //         controller_config.clone(),
-    //     ))
-    //     .unwrap();
-
     let executor0 = EXECUTOR0.init(Executor::new());
     info!("Initialized.");