From 47dda575a9d9e0a62d140b2b7f14c2cb7361d5da Mon Sep 17 00:00:00 2001 From: ChenYing Kuo Date: Thu, 2 Jan 2025 15:35:40 +0800 Subject: [PATCH 1/5] actions/get_result has a different default timeout. Signed-off-by: ChenYing Kuo --- DEFAULT_CONFIG.json5 | 4 +++- zenoh-plugin-ros2dds/src/config.rs | 17 +++++++---------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/DEFAULT_CONFIG.json5 b/DEFAULT_CONFIG.json5 index f330cde..6267d45 100644 --- a/DEFAULT_CONFIG.json5 +++ b/DEFAULT_CONFIG.json5 @@ -126,13 +126,15 @@ //// //// queries_timeout: Timeouts configuration for various Zenoh queries. //// Each field is optional. If not set, the 'default' timeout (5.0 seconds by default) applies to all queries. + //// Note that actions/get_result is the special case, and we need a larger timeout (300.0 seconds by default). + //// Refer to https://github.com/eclipse-zenoh/zenoh-plugin-ros2dds/issues/369#issuecomment-2563725619 //// Each value can be either a float in seconds that will apply as a timeout to all queries, //// either a list of strings with format "=" where: //// - "regex" is a regular expression matching an interface name //// - "float" is the timeout in seconds // queries_timeout: { // //// default timeout that will apply to all query, except the ones specified below - // //// in 'transient_local_subscribers', 'services' and 'actions' + // //// in 'transient_local_subscribers', 'services' and 'actions' (except get_result) // default: 5.0, // //// timeouts for TRANSIENT_LOCAL subscriber when querying publishers for historical publications // transient_local_subscribers: 1.0, diff --git a/zenoh-plugin-ros2dds/src/config.rs b/zenoh-plugin-ros2dds/src/config.rs index a8e2c54..5caf141 100644 --- a/zenoh-plugin-ros2dds/src/config.rs +++ b/zenoh-plugin-ros2dds/src/config.rs @@ -25,6 +25,10 @@ pub const DEFAULT_RELIABLE_ROUTES_BLOCKING: bool = true; pub const DEFAULT_TRANSIENT_LOCAL_CACHE_MULTIPLIER: usize = 10; pub const DEFAULT_DDS_LOCALHOST_ONLY: bool = false; pub const DEFAULT_QUERIES_TIMEOUT: f32 = 5.0; +// In the ROS 2 action, get_result is sent out first and then wait for the result. +// It will cause the action client never complete, so we need a larger timeout. +// Refer to https://github.com/eclipse-zenoh/zenoh-plugin-ros2dds/issues/369#issuecomment-2563725619 +pub const DEFAULT_ACTION_GET_RESULT_TIMEOUT: f32 = 300.0; pub const DEFAULT_WORK_THREAD_NUM: usize = 2; pub const DEFAULT_MAX_BLOCK_THREAD_NUM: usize = 50; @@ -170,23 +174,16 @@ impl Config { pub fn get_queries_timeout_action_get_result(&self, ros2_name: &str) -> Duration { match &self.queries_timeout { Some(QueriesTimeouts { - default, - actions: Some(at), - .. + actions: Some(at), .. }) => { for (re, secs) in &at.get_result { if re.is_match(ros2_name) { return Duration::from_secs_f32(*secs); } } - Duration::from_secs_f32(*default) + Duration::from_secs_f32(DEFAULT_ACTION_GET_RESULT_TIMEOUT) } - Some(QueriesTimeouts { - default, - actions: None, - .. - }) => Duration::from_secs_f32(*default), - _ => Duration::from_secs_f32(DEFAULT_QUERIES_TIMEOUT), + _ => Duration::from_secs_f32(DEFAULT_ACTION_GET_RESULT_TIMEOUT), } } } From d2dd53ef34e03e24bddec1c831ce83f245ed227e Mon Sep 17 00:00:00 2001 From: Julien Enoch Date: Fri, 10 Jan 2025 18:55:52 +0100 Subject: [PATCH 2/5] Clarify description --- DEFAULT_CONFIG.json5 | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/DEFAULT_CONFIG.json5 b/DEFAULT_CONFIG.json5 index 6267d45..a1232cb 100644 --- a/DEFAULT_CONFIG.json5 +++ b/DEFAULT_CONFIG.json5 @@ -125,8 +125,9 @@ //// //// queries_timeout: Timeouts configuration for various Zenoh queries. - //// Each field is optional. If not set, the 'default' timeout (5.0 seconds by default) applies to all queries. - //// Note that actions/get_result is the special case, and we need a larger timeout (300.0 seconds by default). + //// Each field is optional. If not set, the 'default' timeout (5.0 seconds by default) applies to all queries, + //// except for actions' get_result that is by default configured with a larget timeout (300.0 seconds) + //// to support actions that last a long time. //// Refer to https://github.com/eclipse-zenoh/zenoh-plugin-ros2dds/issues/369#issuecomment-2563725619 //// Each value can be either a float in seconds that will apply as a timeout to all queries, //// either a list of strings with format "=" where: From ea158517460ff1e91b7e5005a35a41b40debbaf9 Mon Sep 17 00:00:00 2001 From: Julien Enoch Date: Fri, 10 Jan 2025 18:56:38 +0100 Subject: [PATCH 3/5] Make default queries_timeout not None --- zenoh-plugin-ros2dds/src/config.rs | 31 +++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/zenoh-plugin-ros2dds/src/config.rs b/zenoh-plugin-ros2dds/src/config.rs index 5caf141..3875d07 100644 --- a/zenoh-plugin-ros2dds/src/config.rs +++ b/zenoh-plugin-ros2dds/src/config.rs @@ -63,7 +63,7 @@ pub struct Config { pub shm_enabled: bool, #[serde(default = "default_transient_local_cache_multiplier")] pub transient_local_cache_multiplier: usize, - #[serde(default)] + #[serde(default = "default_queries_timeout")] pub queries_timeout: Option, #[serde(default = "default_reliable_routes_blocking")] pub reliable_routes_blocking: bool, @@ -191,7 +191,7 @@ impl Config { #[derive(Deserialize, Debug, Serialize)] #[serde(deny_unknown_fields)] pub struct QueriesTimeouts { - #[serde(default = "default_queries_timeout")] + #[serde(default = "default_queries_timeout_default")] default: f32, #[serde( default, @@ -205,7 +205,7 @@ pub struct QueriesTimeouts { serialize_with = "serialize_vec_regex_f32" )] services: Vec<(Regex, f32)>, - #[serde(default)] + #[serde(default = "default_actions_timeout")] actions: Option, } @@ -225,7 +225,7 @@ pub struct ActionsTimeouts { )] cancel_goal: Vec<(Regex, f32)>, #[serde( - default, + default = "default_actions_get_result_timeout", deserialize_with = "deserialize_vec_regex_f32", serialize_with = "serialize_vec_regex_f32" )] @@ -400,10 +400,31 @@ fn default_domain() -> u32 { } } -fn default_queries_timeout() -> f32 { +fn default_queries_timeout() -> Option { + Some(QueriesTimeouts{ + default: default_queries_timeout_default(), + transient_local_subscribers: Vec::new(), + services: Vec::new(), + actions: default_actions_timeout() + }) +} + +fn default_queries_timeout_default() -> f32 { DEFAULT_QUERIES_TIMEOUT } +fn default_actions_timeout() -> Option { + Some(ActionsTimeouts { + send_goal: Vec::new(), + cancel_goal: Vec::new(), + get_result: default_actions_get_result_timeout(), + }) +} + +fn default_actions_get_result_timeout() -> Vec<(Regex, f32)> { + vec![(Regex::new(".*").unwrap(), DEFAULT_ACTION_GET_RESULT_TIMEOUT)] +} + fn deserialize_path<'de, D>(deserializer: D) -> Result>, D::Error> where D: Deserializer<'de>, From 18bf6ea69a0eaae2de0d4cd646ebfb1b3bd9c2cd Mon Sep 17 00:00:00 2001 From: Julien Enoch Date: Fri, 10 Jan 2025 19:03:28 +0100 Subject: [PATCH 4/5] Clarify description --- DEFAULT_CONFIG.json5 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/DEFAULT_CONFIG.json5 b/DEFAULT_CONFIG.json5 index a1232cb..5498efb 100644 --- a/DEFAULT_CONFIG.json5 +++ b/DEFAULT_CONFIG.json5 @@ -135,7 +135,7 @@ //// - "float" is the timeout in seconds // queries_timeout: { // //// default timeout that will apply to all query, except the ones specified below - // //// in 'transient_local_subscribers', 'services' and 'actions' (except get_result) + // //// in 'transient_local_subscribers', 'services' and 'actions' // default: 5.0, // //// timeouts for TRANSIENT_LOCAL subscriber when querying publishers for historical publications // transient_local_subscribers: 1.0, @@ -145,7 +145,7 @@ // actions: { // send_goal: 1.0, // cancel_goal: 1.0, - // get_result: [".*long_mission=3600", ".*short_action=10.0"], + // get_result: [".*long_mission=3600", ".*short_action=10.0", ".*=300"], // } // }, From 567937e154b7a695fcefdd9a474e779df749e1d7 Mon Sep 17 00:00:00 2001 From: Julien Enoch Date: Fri, 10 Jan 2025 19:04:33 +0100 Subject: [PATCH 5/5] cargo fmt --- zenoh-plugin-ros2dds/src/config.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/zenoh-plugin-ros2dds/src/config.rs b/zenoh-plugin-ros2dds/src/config.rs index 3875d07..30bf504 100644 --- a/zenoh-plugin-ros2dds/src/config.rs +++ b/zenoh-plugin-ros2dds/src/config.rs @@ -401,11 +401,11 @@ fn default_domain() -> u32 { } fn default_queries_timeout() -> Option { - Some(QueriesTimeouts{ + Some(QueriesTimeouts { default: default_queries_timeout_default(), transient_local_subscribers: Vec::new(), services: Vec::new(), - actions: default_actions_timeout() + actions: default_actions_timeout(), }) }