Only send 114 when operations changed

Signed-off-by: Marcel Guzik <[email protected]>

crates/core/c8y_api/src/smartrest/operations.rs

@@ -121,7 +121,7 @@ impl Operation {
     }
 }
 
-#[derive(Debug, Default, Clone)]
+#[derive(Debug, Default, Clone, PartialEq, Eq)]
 pub struct Operations {
     operations: Vec<Operation>,
 }

crates/extensions/c8y_mapper_ext/src/converter.rs

@@ -65,7 +65,6 @@ use tedge_actors::LoggingSender;
 use tedge_actors::Sender;
 use tedge_api::entity_store;
 use tedge_api::entity_store::EntityExternalId;
-use tedge_api::entity_store::EntityMetadata;
 use tedge_api::entity_store::EntityRegistrationMessage;
 use tedge_api::entity_store::EntityType;
 use tedge_api::entity_store::Error;
@@ -1264,23 +1263,19 @@ impl CumulocityConverter {
     }
 
     fn create_supported_operations(&self, path: &Path) -> Result<Message, ConversionError> {
-        if is_child_operation_path(path) {
-            // operations for child
+        let topic = if is_child_operation_path(path) {
             let child_id = get_child_id(&path.to_path_buf())?;
             let child_external_id = Self::validate_external_id(&child_id)?;
 
-            let topic = C8yTopic::ChildSmartRestResponse(child_external_id.into()).to_topic()?;
-            Ok(Message::new(
-                &topic,
-                Operations::try_new(path)?.create_smartrest_ops_message(),
-            ))
+            C8yTopic::ChildSmartRestResponse(child_external_id.into()).to_topic()?
         } else {
-            // operations for parent
-            Ok(Message::new(
-                &Topic::new_unchecked(SMARTREST_PUBLISH_TOPIC),
-                Operations::try_new(path)?.create_smartrest_ops_message(),
-            ))
-        }
+            Topic::new_unchecked(SMARTREST_PUBLISH_TOPIC)
+        };
+
+        Ok(Message::new(
+            &topic,
+            Operations::try_new(path)?.create_smartrest_ops_message(),
+        ))
     }
 
     pub fn sync_messages(&mut self) -> Vec<Message> {
@@ -1299,8 +1294,14 @@ impl CumulocityConverter {
             .eq(&self.cfg_dir.join("operations").join("c8y"))
         {
             // Re populate the operations irrespective add/remove/modify event
-            self.operations = get_operations(message.ops_dir.clone())?;
-            Ok(Some(self.create_supported_operations(&message.ops_dir)?))
+            let needs_cloud_update =
+                self.update_operations(&self.entity_store.main_device().clone(), &message.ops_dir)?;
+
+            if needs_cloud_update {
+                Ok(Some(self.create_supported_operations(&message.ops_dir)?))
+            } else {
+                Ok(None)
+            }
 
         // operation for child
         } else if message.ops_dir.eq(&self
@@ -1309,12 +1310,16 @@ impl CumulocityConverter {
             .join("c8y")
             .join(get_child_id(&message.ops_dir)?))
         {
-            self.children.insert(
-                get_child_id(&message.ops_dir)?,
-                get_operations(message.ops_dir.clone())?,
-            );
+            let needs_cloud_update = self.update_operations(
+                &get_child_id(&message.ops_dir)?.parse().unwrap(),
+                &message.ops_dir,
+            )?;
 
-            Ok(Some(self.create_supported_operations(&message.ops_dir)?))
+            if needs_cloud_update {
+                Ok(Some(self.create_supported_operations(&message.ops_dir)?))
+            } else {
+                Ok(None)
+            }
         } else {
             Ok(None)
         }
@@ -1361,10 +1366,11 @@ fn create_request_for_cloud_child_devices() -> Message {
 impl CumulocityConverter {
     /// Register on C8y an operation capability for a device.
     fn register_operation(
-        &self,
-        device: &EntityMetadata,
+        &mut self,
+        target: &EntityTopicId,
         c8y_operation_name: &str,
     ) -> Result<Vec<Message>, ConversionError> {
+        let device = self.entity_store.try_get(target)?;
         let ops_dir = match device.r#type {
             EntityType::MainDevice => self.ops_dir.clone(),
             EntityType::ChildDevice => {
@@ -1380,20 +1386,53 @@ impl CumulocityConverter {
         let ops_file = ops_dir.join(c8y_operation_name);
         create_directory_with_defaults(&ops_dir)?;
         create_file_with_defaults(ops_file, None)?;
-        let device_operations = self.create_supported_operations(&ops_dir)?;
-        Ok(vec![device_operations])
+
+        let need_cloud_update = self.update_operations(target, &ops_dir)?;
+
+        if need_cloud_update {
+            let device_operations = self.create_supported_operations(&ops_dir)?;
+            return Ok(vec![device_operations]);
+        }
+
+        Ok(vec![])
+    }
+
+    fn update_operations(
+        &mut self,
+        target: &EntityTopicId,
+        dir: &Path,
+    ) -> Result<bool, ConversionError> {
+        let device = self.entity_store.try_get(target)?;
+        let operations = get_operations(dir)?;
+        let modified = match device.r#type {
+            EntityType::MainDevice => {
+                let modified = self.operations != operations;
+                self.operations = operations;
+                modified
+            }
+            EntityType::ChildDevice => {
+                let modified =
+                    self.children.get(&device.topic_id.to_string()).unwrap() != &operations;
+                self.children
+                    .insert(device.topic_id.to_string(), operations);
+                modified
+            }
+            EntityType::Service => unimplemented!(),
+        };
+
+        Ok(modified)
     }
 
     async fn register_restart_operation(
-        &self,
+        &mut self,
         target: &EntityTopicId,
     ) -> Result<Vec<Message>, ConversionError> {
-        match self.entity_store.get(target) {
-            None => {
+        match self.register_operation(target, "c8y_Restart") {
+            Err(_) => {
                 error!("Fail to register `restart` operation for unknown device: {target}");
                 Ok(vec![])
             }
-            Some(device) => self.register_operation(device, "c8y_Restart"),
+            Ok(messages) => Ok(messages),
         }
     }
 
@@ -1462,20 +1501,19 @@ impl CumulocityConverter {
     }
 
     async fn register_software_update_operation(
-        &self,
+        &mut self,
         target: &EntityTopicId,
     ) -> Result<Vec<Message>, ConversionError> {
-        match self.entity_store.get(target) {
-            None => {
+        let mut registration = match self.register_operation(target, "c8y_SoftwareUpdate") {
+            Err(_) => {
                 error!("Fail to register `software-list` operation for unknown device: {target}");
-                Ok(vec![])
-            }
-            Some(device) => {
-                let mut registration = self.register_operation(device, "c8y_SoftwareUpdate")?;
-                registration.push(self.request_software_list(target));
-                Ok(registration)
+                return Ok(vec![]);
             }
-        }
+            Ok(messages) => messages,
+        };
+
+        registration.push(self.request_software_list(target));
+        Ok(registration)
     }
 
     async fn publish_software_update_status(

tests/RobotFramework/tests/cumulocity/supported_operations/mapper-publishing-agent-supported-ops.robot

@@ -8,6 +8,10 @@ Test Setup    Custom Setup
 Test Teardown    Get Logs
 
 *** Test Cases ***
+
+Full supported operations message has no duplicates
+    Should Have MQTT Messages    c8y/s/us    message_pattern=114,c8y_DownloadConfigFile,c8y_LogfileRequest,c8y_RemoteAccessConnect,c8y_Restart,c8y_SoftwareUpdate,c8y_UploadConfigFile    minimum=1    maximum=1
+
 Create and publish the tedge agent supported operations on mapper restart
     # stop mapper and remove the supported operations
     ThinEdgeIO.Stop Service     tedge-mapper-c8y