fix: Correct the unnecessary constrict updates in JoyHub protocols

buttplug/src/server/device/protocol/joyhub.rs

@@ -45,6 +45,7 @@ async fn delayed_constrict_handler(device: Arc<Hardware>, scalar: u8) {
     error!("Delayed JoyHub Constrict command error: {:?}", res.err());
   }
 }
+
 fn vibes_changed(
   old_commands_lock: &RwLock<Vec<Option<(ActuatorType, u32)>>>,
   new_commands: &[Option<(ActuatorType, u32)>],
@@ -70,6 +71,28 @@ fn vibes_changed(
   false
 }
 
+fn scalar_changed(
+  old_commands_lock: &RwLock<Vec<Option<(ActuatorType, u32)>>>,
+  new_commands: &[Option<(ActuatorType, u32)>],
+  index: usize,
+) -> bool {
+  let old_commands = old_commands_lock.read().expect("locks should work");
+  if old_commands.len() != new_commands.len() {
+    return true;
+  }
+
+  if index < old_commands.len() {
+    if let Some(ocmd) = old_commands[index] {
+      if let Some(ncmd) = new_commands[index] {
+        if ocmd.1 != ncmd.1 {
+          return true;
+        }
+      }
+    }
+  }
+  false
+}
+
 #[derive(Default)]
 pub struct JoyHubInitializer {}
 
@@ -123,10 +146,12 @@ impl ProtocolHandler for JoyHub {
 
     if let Some(cmd) = cmd2 {
       if cmd.0 == ActuatorType::Constrict {
-        if vibes_changed(&self.last_cmds, commands, vec![1usize]) {
+        cmd2 = None;
+        if !scalar_changed(&self.last_cmds, commands, 1usize) {
+          // no-op
+        } else if vibes_changed(&self.last_cmds, commands, vec![1usize]) {
           let dev = self.device.clone();
           async_manager::spawn(async move { delayed_constrict_handler(dev, cmd.1 as u8).await });
-          cmd2 = None;
         } else {
           let mut command_writer = self.last_cmds.write().expect("Locks should work");
           *command_writer = commands.to_vec();

buttplug/src/server/device/protocol/joyhub_v2.rs

@@ -38,6 +38,7 @@ async fn delayed_constrict_handler(device: Arc<Hardware>, scalar: u8) {
     error!("Delayed JoyHub Constrict command error: {:?}", res.err());
   }
 }
+
 fn vibes_changed(
   old_commands_lock: &RwLock<Vec<Option<(ActuatorType, u32)>>>,
   new_commands: &[Option<(ActuatorType, u32)>],
@@ -63,6 +64,28 @@ fn vibes_changed(
   false
 }
 
+fn scalar_changed(
+  old_commands_lock: &RwLock<Vec<Option<(ActuatorType, u32)>>>,
+  new_commands: &[Option<(ActuatorType, u32)>],
+  index: usize,
+) -> bool {
+  let old_commands = old_commands_lock.read().expect("locks should work");
+  if old_commands.len() != new_commands.len() {
+    return true;
+  }
+
+  if index < old_commands.len() {
+    if let Some(ocmd) = old_commands[index] {
+      if let Some(ncmd) = new_commands[index] {
+        if ocmd.1 != ncmd.1 {
+          return true;
+        }
+      }
+    }
+  }
+  false
+}
+
 #[derive(Default)]
 pub struct JoyHubV2Initializer {}
 
@@ -116,10 +139,12 @@ impl ProtocolHandler for JoyHubV2 {
 
     if let Some(cmd) = cmd2 {
       if cmd.0 == ActuatorType::Constrict {
-        if vibes_changed(&self.last_cmds, commands, vec![1usize]) {
+        cmd2 = None;
+        if !scalar_changed(&self.last_cmds, commands, 1usize) {
+          // no-op
+        } else if vibes_changed(&self.last_cmds, commands, vec![1usize]) {
           let dev = self.device.clone();
           async_manager::spawn(async move { delayed_constrict_handler(dev, cmd.1 as u8).await });
-          cmd2 = None;
         } else {
           let mut command_writer = self.last_cmds.write().expect("Locks should work");
           *command_writer = commands.to_vec();
@@ -136,10 +161,12 @@ impl ProtocolHandler for JoyHubV2 {
 
     if let Some(cmd) = cmd3 {
       if cmd.0 == ActuatorType::Constrict {
-        if vibes_changed(&self.last_cmds, commands, vec![2usize]) {
+        cmd3 = None;
+        if !scalar_changed(&self.last_cmds, commands, 2usize) {
+          // no-op
+        } else if vibes_changed(&self.last_cmds, commands, vec![2usize]) {
           let dev = self.device.clone();
           async_manager::spawn(async move { delayed_constrict_handler(dev, cmd.1 as u8).await });
-          cmd3 = None;
         } else {
           let mut command_writer = self.last_cmds.write().expect("Locks should work");
           *command_writer = commands.to_vec();

buttplug/src/server/device/protocol/joyhub_v4.rs

@@ -70,6 +70,28 @@ fn vibes_changed(
   false
 }
 
+fn scalar_changed(
+  old_commands_lock: &RwLock<Vec<Option<(ActuatorType, u32)>>>,
+  new_commands: &[Option<(ActuatorType, u32)>],
+  index: usize,
+) -> bool {
+  let old_commands = old_commands_lock.read().expect("locks should work");
+  if old_commands.len() != new_commands.len() {
+    return true;
+  }
+
+  if index < old_commands.len() {
+    if let Some(ocmd) = old_commands[index] {
+      if let Some(ncmd) = new_commands[index] {
+        if ocmd.1 != ncmd.1 {
+          return true;
+        }
+      }
+    }
+  }
+  false
+}
+
 #[derive(Default)]
 pub struct JoyHubV4Initializer {}
 
@@ -123,10 +145,12 @@ impl ProtocolHandler for JoyHubV4 {
 
     if let Some(cmd) = cmd3 {
       if cmd.0 == ActuatorType::Constrict {
-        if vibes_changed(&self.last_cmds, commands, vec![2usize]) {
+        cmd3 = None;
+        if !scalar_changed(&self.last_cmds, commands, 2usize) {
+          // no-op
+        } else if vibes_changed(&self.last_cmds, commands, vec![2usize]) {
           let dev = self.device.clone();
           async_manager::spawn(async move { delayed_constrict_handler(dev, cmd.1 as u8).await });
-          cmd3 = None;
         } else {
           let mut command_writer = self.last_cmds.write().expect("Locks should work");
           *command_writer = commands.to_vec();

buttplug/src/server/device/protocol/joyhub_v5.rs

@@ -45,6 +45,7 @@ async fn delayed_constrict_handler(device: Arc<Hardware>, scalar: u8) {
     error!("Delayed JoyHub Constrict command error: {:?}", res.err());
   }
 }
+
 fn vibes_changed(
   old_commands_lock: &RwLock<Vec<Option<(ActuatorType, u32)>>>,
   new_commands: &[Option<(ActuatorType, u32)>],
@@ -70,6 +71,28 @@ fn vibes_changed(
   false
 }
 
+fn scalar_changed(
+  old_commands_lock: &RwLock<Vec<Option<(ActuatorType, u32)>>>,
+  new_commands: &[Option<(ActuatorType, u32)>],
+  index: usize,
+) -> bool {
+  let old_commands = old_commands_lock.read().expect("locks should work");
+  if old_commands.len() != new_commands.len() {
+    return true;
+  }
+
+  if index < old_commands.len() {
+    if let Some(ocmd) = old_commands[index] {
+      if let Some(ncmd) = new_commands[index] {
+        if ocmd.1 != ncmd.1 {
+          return true;
+        }
+      }
+    }
+  }
+  false
+}
+
 #[derive(Default)]
 pub struct JoyHubV5Initializer {}
 
@@ -118,7 +141,10 @@ impl ProtocolHandler for JoyHubV5 {
 
     if let Some(cmd) = cmd2 {
       if cmd.0 == ActuatorType::Constrict {
-        if vibes_changed(&self.last_cmds, commands, vec![2usize]) {
+        // cmd2 = None; // Not used again in this protocol variant
+        if !scalar_changed(&self.last_cmds, commands, 1usize) {
+          // no-op
+        } else if vibes_changed(&self.last_cmds, commands, vec![1usize]) {
           let dev = self.device.clone();
           async_manager::spawn(async move { delayed_constrict_handler(dev, cmd.1 as u8).await });
         } else {

buttplug/tests/test_device_protocols.rs

@@ -101,6 +101,7 @@ async fn load_test_case(test_file: &str) -> DeviceTestCase {
 #[test_case("test_joyhub_petalwish.yaml" ; "JoyHub Protocol - Petalwish")]
 #[test_case("test_joyhub_petalwish_compat.yaml" ; "JoyHub Protocol - Petalwish Compat")]
 #[test_case("test_joyhub_roselin.yaml" ; "JoyHub Protocol - RoseLin")]
+#[test_case("test_joyhub_moonhorn.yaml" ; "JoyHub Protocol - Moonhorn")]
 #[test_case("test_itoys_protocol.yaml" ; "iToys Protocol")]
 #[test_case("test_leten_protocol.yaml" ; "Leten Protocol")]
 #[test_case("test_motorbunny_protocol.yaml" ; "Motorbunny Protocol")]
@@ -199,6 +200,7 @@ async fn test_device_protocols_embedded_v3(test_file: &str) {
 #[test_case("test_foreo_protocol.yaml" ; "Foreo Protocol")]
 #[test_case("test_joyhub_petalwish.yaml" ; "JoyHub Protocol - Petalwish")]
 #[test_case("test_joyhub_petalwish_compat.yaml" ; "JoyHub Protocol - Petalwish Compat")]
+#[test_case("test_joyhub_moonhorn.yaml" ; "JoyHub Protocol - Moonhorn")]
 #[test_case("test_joyhub_roselin.yaml" ; "JoyHub Protocol - RoseLin")]
 #[test_case("test_itoys_protocol.yaml" ; "iToys Protocol")]
 #[test_case("test_leten_protocol.yaml" ; "Leten Protocol")]

buttplug/tests/util/device_test/device_test_case/test_joyhub_moonhorn.yaml

@@ -0,0 +1,93 @@
+devices:
+  - identifier: 
+      name: "J-MoonHorn"
+    expected_name: "JoyHub Moon Horn"
+device_commands:
+  # Commands
+  - !Messages
+      device_index: 0
+      messages: 
+        - !Vibrate
+          - Index: 0
+            Speed: 0.5
+  - !Commands
+      device_index: 0
+      commands: 
+        - !Write
+            endpoint: tx
+            data: [0xa0, 0x03, 0x80, 0x00, 0x00, 0x00, 0xaa]
+            write_with_response: false
+        - !Write
+            endpoint: tx
+            data: [0xa0, 0x0d, 0x00, 0x00, 0x00, 0xff] # First 0 is free
+            write_with_response: false
+  - !Messages
+      device_index: 0
+      messages:
+        - !Scalar
+          - Index: 0
+            Scalar: 0.5
+            ActuatorType: Vibrate
+          - Index: 1
+            Scalar: 0.1
+            ActuatorType: Constrict
+  - !Commands
+      device_index: 0
+      commands:
+        - !Write
+            endpoint: tx
+            data: [0xa0, 0x0d, 0x00, 0x00, 0x01, 0xff]
+            write_with_response: false
+  - !Messages
+      device_index: 0
+      messages:
+        - !Scalar
+          - Index: 0
+            Scalar: 1.0
+            ActuatorType: Vibrate
+          - Index: 1
+            Scalar: 0.1
+            ActuatorType: Constrict
+  - !Commands
+      device_index: 0
+      commands:
+        - !Write
+            endpoint: tx
+            data: [0xa0, 0x03, 0xff, 0x00, 0x00, 0x00, 0xaa]
+            write_with_response: false
+  - !Messages
+      device_index: 0
+      messages:
+        - !Scalar
+          - Index: 0
+            Scalar: 0.5
+            ActuatorType: Vibrate
+          - Index: 1
+            Scalar: 0.5
+            ActuatorType: Constrict
+  - !Commands
+      device_index: 0
+      commands:
+        - !Write
+            endpoint: tx
+            data: [0xa0, 0x03, 0x80, 0x00, 0x00, 0x00, 0xaa]
+            write_with_response: false
+        - !Write
+            endpoint: tx
+            data: [0xa0, 0x0d, 0x00, 0x00, 0x05, 0xff]
+            write_with_response: false
+  - !Messages
+      device_index: 0
+      messages:
+        - !Stop
+  - !Commands
+      device_index: 0
+      commands:
+        - !Write
+            endpoint: tx
+            data: [0xa0, 0x03, 0x00, 0x00, 0x00, 0x00, 0xaa]
+            write_with_response: false
+        - !Write
+            endpoint: tx
+            data: [0xa0, 0x0d, 0x00, 0x00, 0x00, 0xff]
+            write_with_response: false