chore: Actually remove DeviceAdded/Removed from v4

Just send DeviceList every time we have an update and let the
client deal with it.

Fixes #544

Author: qdot

buttplug-schema/schema/buttplug-schema.json

@@ -509,37 +509,12 @@
           "Endpoint",
           "RawCommand"
         ]
-      },
-      "DeviceAdded": {
-        "type": "object",
-        "description": "List of all available devices known to the system.",
-        "properties": {
-          "Id": { "$ref": "#/components/SystemId" },
-          "DeviceName": { "$ref": "#/components/DeviceName" },
-          "DeviceIndex": { "$ref": "#/components/DeviceIndex" },
-          "DeviceDisplayName": { "type": "string" },
-          "DeviceMessageTimingGap": { "type": "integer" },
-          "DeviceFeatures": {
-            "type": "array",
-            "items": {
-              "$ref": "#/components/DeviceFeatureV4" 
-            }
-          }
-        },
-        "additionalProperties": false,
-        "required": [
-          "Id",
-          "DeviceName",
-          "DeviceIndex",
-          "DeviceMessageTimingGap",
-          "DeviceFeatures"
-        ]
-      },      
+      },     
       "DeviceList": {
         "type": "object",
         "description": "List of all available devices known to the system.",
         "properties": {
-          "Id": { "$ref": "#/components/SystemId" },
+          "Id": { "$ref": "#/components/ServerId" },
           "Devices": {
             "description": "Array of device ids and names.",
             "type": "array",
@@ -559,7 +534,6 @@
               },
               "additionalProperties": false,
               "required": [
-                "Id",
                 "DeviceName",
                 "DeviceIndex",
                 "DeviceMessageTimingGap",
@@ -1674,9 +1648,7 @@
         "type": "object",
         "description": "All messages valid in Buttplug Spec v4",
         "properties": {
-          "DeviceAdded": { "$ref": "#/messages/SpecV4Messages/DeviceAdded" },
           "DeviceList": { "$ref": "#/messages/SpecV4Messages/DeviceList" },
-          "DeviceRemoved": { "$ref": "#/messages/SpecV0Messages/DeviceRemoved" },
           "Error": { "$ref": "#/messages/SpecV0Messages/Error" },
           "Ok": { "$ref": "#/messages/SpecV0Messages/Ok" },
           "Ping": { "$ref": "#/messages/SpecV0Messages/Ping" },

crates/buttplug_client/src/client_event_loop.rs

@@ -16,7 +16,7 @@ use super::{
 };
 use buttplug_core::{
   connector::{ButtplugConnector, ButtplugConnectorStateShared},
-  errors::{ButtplugDeviceError, ButtplugError},
+  errors::ButtplugError,
   message::{
     ButtplugClientMessageV4,
     ButtplugDeviceMessage,
@@ -214,30 +214,22 @@ where
     trace!("Message future not found, assuming server event.");
     info!("{:?}", msg);
     match msg {
-      ButtplugServerMessageV4::DeviceAdded(dev) => {
-        trace!("Device added, updating map and sending to client");
-        // We already have this device. Emit an error to let the client know the
-        // server is being weird.
-        if self.device_map.get(&dev.device_index()).is_some() {
-          self.send_client_event(ButtplugClientEvent::Error(
-            ButtplugDeviceError::DeviceConnectionError(
-              "Device already exists in client. Server may be in a weird state.".to_owned(),
-            )
-            .into(),
-          ));
-          return;
+      ButtplugServerMessageV4::DeviceList(list) => {
+        trace!("Got device list, devices either added or removed");
+        for dev in list.devices() {
+          if self.device_map.contains_key(&dev.device_index()) {
+            continue;
+          }
+          trace!("Device added, updating map and sending to client");
+          let info = DeviceMessageInfoV4::from(dev.clone());
+          let device = self.create_client_device(&info);
+          self.send_client_event(ButtplugClientEvent::DeviceAdded(device));
         }
-        let info = DeviceMessageInfoV4::from(dev);
-        let device = self.create_client_device(&info);
-        self.send_client_event(ButtplugClientEvent::DeviceAdded(device));
-      }
-      ButtplugServerMessageV4::DeviceRemoved(dev) => {
-        if self.device_map.contains_key(&dev.device_index()) {
+        let new_indexes: Vec<u32> = list.devices().iter().map(|x| x.device_index()).collect();
+        let disconnected_indexes: Vec<u32> = self.device_map.iter().filter(|x| !new_indexes.contains(x.key())).map(|x| *x.key()).collect();
+        for index in disconnected_indexes {
           trace!("Device removed, updating map and sending to client");
-          self.disconnect_device(dev.device_index());
-        } else {
-          error!("Received DeviceRemoved for non-existent device index");
-          self.send_client_event(ButtplugClientEvent::Error(ButtplugDeviceError::DeviceConnectionError("Device removal requested for a device the client does not know about. Server may be in a weird state.".to_owned()).into()));
+          self.disconnect_device(index);
         }
       }
       ButtplugServerMessageV4::ScanningFinished(_) => {

crates/buttplug_core/src/message/v4/device_added.rs

@@ -5,7 +5,6 @@
 // Licensed under the BSD 3-Clause license. See LICENSE file in the project root
 // for full license information.
 
-use super::DeviceAddedV4;
 use crate::message::DeviceFeature;
 use getset::{CopyGetters, Getters, MutGetters};
 use serde::{Deserialize, Serialize};
@@ -47,15 +46,3 @@ impl DeviceMessageInfoV4 {
     }
   }
 }
-
-impl From<DeviceAddedV4> for DeviceMessageInfoV4 {
-  fn from(device_added: DeviceAddedV4) -> Self {
-    Self {
-      device_index: device_added.device_index(),
-      device_name: device_added.device_name().clone(),
-      device_display_name: device_added.device_display_name().clone(),
-      device_message_timing_gap: device_added.device_message_timing_gap(),
-      device_features: device_added.device_features().clone(),
-    }
-  }
-}

crates/buttplug_core/src/message/v4/device_message_info.rs

@@ -5,7 +5,6 @@
 // Licensed under the BSD 3-Clause license. See LICENSE file in the project root
 // for full license information.
 
-mod device_added;
 mod device_list;
 mod device_message_info;
 mod input_cmd;
@@ -16,7 +15,6 @@ mod server_info;
 mod spec_enums;
 
 pub use {
-  device_added::DeviceAddedV4,
   device_list::DeviceListV4,
   device_message_info::DeviceMessageInfoV4,
   input_cmd::{InputCmdV4, InputCommandType},

crates/buttplug_core/src/message/v4/mod.rs

@@ -11,7 +11,6 @@ use crate::message::{
   ButtplugMessageError,
   ButtplugMessageFinalizer,
   ButtplugMessageValidator,
-  DeviceRemovedV0,
   ErrorV0,
   OkV0,
   OutputCmdV4,
@@ -27,7 +26,7 @@ use crate::message::{
 };
 use serde::{Deserialize, Serialize};
 
-use super::{DeviceAddedV4, DeviceListV4, InputReadingV4};
+use super::{DeviceListV4, InputReadingV4};
 
 /// Represents all client-to-server messages in v3 of the Buttplug Spec
 #[derive(
@@ -75,8 +74,6 @@ pub enum ButtplugServerMessageV4 {
   ServerInfo(ServerInfoV4),
   // Device enumeration messages
   DeviceList(DeviceListV4),
-  DeviceAdded(DeviceAddedV4),
-  DeviceRemoved(DeviceRemovedV0),
   ScanningFinished(ScanningFinishedV0),
   // Sensor commands
   InputReading(InputReadingV4),
@@ -85,7 +82,6 @@ pub enum ButtplugServerMessageV4 {
 impl ButtplugMessageFinalizer for ButtplugServerMessageV4 {
   fn finalize(&mut self) {
     match self {
-      ButtplugServerMessageV4::DeviceAdded(da) => da.finalize(),
       ButtplugServerMessageV4::DeviceList(dl) => dl.finalize(),
       _ => (),
     }

crates/buttplug_core/src/message/v4/spec_enums.rs

@@ -243,18 +243,22 @@ impl ServerDeviceManager {
     }
   }
 
+  fn generate_device_list(&self) -> DeviceListV4 {
+    let devices = self
+      .devices
+      .iter()
+      .map(|device| device.value().as_device_message_info(*device.key()))
+      .collect();
+    DeviceListV4::new(devices)
+  }
+
   fn parse_device_manager_message(
     &self,
     manager_msg: ButtplugDeviceManagerMessageUnion,
   ) -> ButtplugServerResultFuture {
     match manager_msg {
       ButtplugDeviceManagerMessageUnion::RequestDeviceList(msg) => {
-        let devices = self
-          .devices
-          .iter()
-          .map(|device| device.value().as_device_message_info(*device.key()))
-          .collect();
-        let mut device_list = DeviceListV4::new(devices);
+        let mut device_list = self.generate_device_list();
         device_list.set_id(msg.id());
         future::ready(Ok(device_list.into())).boxed()
       }

crates/buttplug_server/src/device/server_device_manager.rs

@@ -6,7 +6,7 @@
 // for full license information.
 
 use buttplug_core::{
-  message::{ButtplugServerMessageV4, DeviceAddedV4, DeviceRemovedV0, ScanningFinishedV0},
+  message::{ButtplugServerMessageV4, DeviceListV4, ScanningFinishedV0},
   util::async_manager,
 };
 use buttplug_server_device_config::DeviceConfigurationManager;
@@ -55,7 +55,7 @@ pub(super) struct ServerDeviceManagerEventLoop {
   /// True if stop scanning message was sent, means we won't send scanning finished.
   stop_scanning_received: AtomicBool,
   /// Protocol map, for mapping user definitions to protocols
-  protocol_manager: ProtocolManager
+  protocol_manager: ProtocolManager,
 }
 
 impl ServerDeviceManagerEventLoop {
@@ -83,7 +83,7 @@ impl ServerDeviceManagerEventLoop {
       connecting_devices: Arc::new(DashSet::new()),
       loop_cancellation_token,
       stop_scanning_received: AtomicBool::new(false),
-      protocol_manager: ProtocolManager::default()
+      protocol_manager: ProtocolManager::default(),
     }
   }
 
@@ -256,6 +256,15 @@ impl ServerDeviceManagerEventLoop {
     }
   }
 
+  fn generate_device_list(&self) -> DeviceListV4 {
+    let devices = self
+      .device_map
+      .iter()
+      .map(|device| device.value().as_device_message_info(*device.key()))
+      .collect();
+    DeviceListV4::new(devices)
+  }
+
   async fn handle_device_event(&mut self, device_event: ServerDeviceEvent) {
     trace!("Got device event: {:?}", device_event);
     match device_event {
@@ -304,13 +313,15 @@ impl ServerDeviceManagerEventLoop {
         });
 
         info!("Assigning index {} to {}", device_index, device.name());
-        let device_added_message = DeviceAddedV4::from(device.as_device_message_info(device_index));
-        self.device_map.insert(device_index, device);
+        self.device_map.insert(device_index, device.clone());
+
+        let device_update_message: ButtplugServerMessageV4 = self.generate_device_list().into();
+
         // After that, we can send out to the server's event listeners to let
         // them know a device has been added.
         if self
           .server_sender
-          .send(device_added_message.into())
+          .send(device_update_message.into())
           .is_err()
         {
           debug!("Server not currently available, dropping Device Added event.");
@@ -329,9 +340,10 @@ impl ServerDeviceManagerEventLoop {
             .device_map
             .remove(&device_index)
             .expect("Remove will always work.");
+          let device_update_message: ButtplugServerMessageV4 = self.generate_device_list().into();
           if self
             .server_sender
-            .send(DeviceRemovedV0::new(device_index).into())
+            .send(device_update_message)
             .is_err()
           {
             debug!("Server not currently available, dropping Device Removed event.");