From a313601266d8722c4cc172ab1d2c5e784ac25a9a Mon Sep 17 00:00:00 2001 From: Marcel Guzik Date: Fri, 15 Nov 2024 15:36:48 +0100 Subject: [PATCH] Read operations directory for child devices when registering a single operation [1] disabled dynamic operation reload for child devices because it needed to support nested child devices as well. For this reason, when receiving an MQTT command metadata message and registering that operation, we still need to read the operation directory and register and send all the new operations. [1]: https://github.com/thin-edge/thin-edge.io/pull/2614 Signed-off-by: Marcel Guzik --- .../c8y_mapper_ext/src/converter.rs | 61 +++++++++++++------ crates/extensions/c8y_mapper_ext/src/tests.rs | 2 - 2 files changed, 44 insertions(+), 19 deletions(-) diff --git a/crates/extensions/c8y_mapper_ext/src/converter.rs b/crates/extensions/c8y_mapper_ext/src/converter.rs index 9a8b3269787..14395a7fab5 100644 --- a/crates/extensions/c8y_mapper_ext/src/converter.rs +++ b/crates/extensions/c8y_mapper_ext/src/converter.rs @@ -1545,6 +1545,12 @@ fn is_child_operation_path(path: &Path) -> bool { impl CumulocityConverter { /// Register on C8y an operation capability for a device. + /// + /// Additionally when the target is a child device, operation directory for the device will be loaded and operations + /// not already registered will be registered. + /// + /// Returns a Set Supported Operations (114) message if among registered operations there were new operations that + /// were not announced to the cloud. pub fn register_operation( &mut self, target: &EntityTopicId, @@ -1568,27 +1574,45 @@ impl CumulocityConverter { } }; + // Save the operation to the operation directory let ops_file = ops_dir.join(c8y_operation_name); create_directory_with_defaults(&*ops_dir)?; create_file_with_defaults(&ops_file, None)?; - let operation = c8y_api::smartrest::operations::get_operation( - ops_file.as_std_path(), - &self.config.bridge_config, - )?; - let operations = self - .operations_for_device_mut(target) - .expect("entity should've been checked before that it's not a service"); + let ops_dir = ops_dir.as_std_path(); + + let need_cloud_update = match is_child_operation_path(ops_dir) { + // for devices other than the main device, dynamic update of supported operations via file events is + // disabled, so we have to additionally load new operations from the c8y operations for that device + true => self.update_operations(ops_dir)?, + + // for main devices new operation files are loaded dynamically as they are created, so only register one + // operation we need + false => { + let bridge_config = &self.config.bridge_config; + let operation = c8y_api::smartrest::operations::get_operation( + ops_file.as_std_path(), + bridge_config, + )?; + + let current_operations = self + .operations_for_device_mut(target) + .expect("entity should've been checked before that it's not a service"); - let prev_operation = operations.remove_operation(&operation.name); - // even if the body of the operation is different, as long as it has the same name, supported operations message - // will be the same, so we don't need to resend - let need_cloud_update = prev_operation.is_none(); - operations.add_operation(operation); + let prev_operation = current_operations.remove_operation(&operation.name); + + // even if the body of the operation is different, as long as it has the same name, supported operations message + // will be the same, so we don't need to resend + let need_cloud_update = prev_operation.is_none(); + + current_operations.add_operation(operation); + + need_cloud_update + } + }; if need_cloud_update { - let cloud_update_operations_message = - self.create_supported_operations(ops_dir.as_std_path())?; + let cloud_update_operations_message = self.create_supported_operations(ops_dir)?; return Ok(vec![cloud_update_operations_message]); } @@ -1596,10 +1620,13 @@ impl CumulocityConverter { Ok(vec![]) } - /// Saves a new supported operation set for a given device. + /// Loads and saves a new supported operation set for a given device. + /// + /// All operation files from the given operation directory are loaded and set as the new supported operation set for + /// a given device. Invalid operation files are ignored. /// - /// If the supported operation set changed, `Ok(true)` is returned to denote that this change - /// should be sent to the cloud. + /// If the supported operation set changed, `Ok(true)` is returned to denote that this change should be sent to the + /// cloud. fn update_operations(&mut self, dir: &Path) -> Result { let operations = get_operations(dir, &self.config.bridge_config)?; let current_operations = if is_child_operation_path(dir) { diff --git a/crates/extensions/c8y_mapper_ext/src/tests.rs b/crates/extensions/c8y_mapper_ext/src/tests.rs index c4d5caeed26..be8f714a1c6 100644 --- a/crates/extensions/c8y_mapper_ext/src/tests.rs +++ b/crates/extensions/c8y_mapper_ext/src/tests.rs @@ -1425,8 +1425,6 @@ async fn mapper_dynamically_updates_supported_operations_for_tedge_device() { } #[tokio::test] -// TODO: fix or remove test -#[ignore = "asserts that publishing a single operation capability message causes full rescan of c8y operations directory, which is undesirable behaviour"] async fn mapper_dynamically_updates_supported_operations_for_child_device() { // The test assures tedge-mapper reads the operations for the child devices from the operations directory, and then it publishes them on to `c8y/s/us/child1`. // When mapper is running test adds a new operation for a child into the operations directory, then the mapper discovers the new