Fix a bug in graph query function and add regression test (#283)

Author: nnmm

rclrs/src/node/graph.rs

@@ -47,6 +47,7 @@ pub struct NodeNameInfo {
 }
 
 /// Contains topic endpoint information
+#[derive(Debug, PartialEq, Eq)]
 pub struct TopicEndpointInfo {
     /// The name of the endpoint node
     pub node_name: String,
@@ -374,7 +375,7 @@ impl Node {
                 &*self.rcl_node_mtx.lock().unwrap(),
                 &mut rcutils_get_default_allocator(),
                 topic.as_ptr(),
-                true,
+                false,
                 &mut rcl_publishers_info,
             )
             .ok()?;

rclrs_tests/src/graph_tests.rs

@@ -1,5 +1,6 @@
 use rclrs::{
-    Context, Node, NodeBuilder, RclrsError, TopicNamesAndTypes, QOS_PROFILE_SYSTEM_DEFAULT,
+    Context, Node, NodeBuilder, RclrsError, TopicEndpointInfo, TopicNamesAndTypes,
+    QOS_PROFILE_SYSTEM_DEFAULT,
 };
 use test_msgs::{msg, srv};
 
@@ -22,122 +23,127 @@ fn construct_test_graph(namespace: &str) -> Result<TestGraph, RclrsError> {
 
 #[test]
 fn test_publishers() -> Result<(), RclrsError> {
-    let graph = construct_test_graph("test_publishers_graph")?;
+    let namespace = "/test_publishers_graph";
+    let graph = construct_test_graph(namespace)?;
 
-    let _node_1_empty_publisher = graph
+    let node_1_empty_publisher = graph
         .node1
         .create_publisher::<msg::Empty>("graph_test_topic_1", QOS_PROFILE_SYSTEM_DEFAULT)?;
-    let _node_1_basic_types_publisher = graph
+    let topic1 = node_1_empty_publisher.topic_name();
+    let node_1_basic_types_publisher = graph
         .node1
         .create_publisher::<msg::BasicTypes>("graph_test_topic_2", QOS_PROFILE_SYSTEM_DEFAULT)?;
-    let _node_2_default_publisher = graph
+    let topic2 = node_1_basic_types_publisher.topic_name();
+    let node_2_default_publisher = graph
         .node2
         .create_publisher::<msg::Defaults>("graph_test_topic_3", QOS_PROFILE_SYSTEM_DEFAULT)?;
+    let topic3 = node_2_default_publisher.topic_name();
 
     std::thread::sleep(std::time::Duration::from_millis(100));
 
-    assert_eq!(
-        graph
-            .node1
-            .count_publishers(&(graph.node1.namespace() + "/graph_test_topic_1"))?,
-        1
-    );
-    assert_eq!(
-        graph
-            .node1
-            .count_publishers(&(graph.node1.namespace() + "/graph_test_topic_2"))?,
-        1
-    );
+    // Test count_publishers()
+    assert_eq!(graph.node1.count_publishers(&topic1)?, 1);
+    assert_eq!(graph.node1.count_publishers(&topic2)?, 1);
+    assert_eq!(graph.node1.count_publishers(&topic3)?, 1);
+
+    // Test get_publisher_names_and_types_by_node()
     let node_1_publisher_names_and_types = graph
         .node1
-        .get_publisher_names_and_types_by_node(&graph.node1.name(), &graph.node1.namespace())?;
+        .get_publisher_names_and_types_by_node(&graph.node1.name(), namespace)?;
 
-    let types = node_1_publisher_names_and_types
-        .get(&(graph.node1.namespace() + "/graph_test_topic_1"))
-        .unwrap();
+    let types = node_1_publisher_names_and_types.get(&topic1).unwrap();
     assert!(types.contains(&"test_msgs/msg/Empty".to_string()));
 
-    let types = node_1_publisher_names_and_types
-        .get(&(graph.node1.namespace() + "/graph_test_topic_2"))
-        .unwrap();
+    let types = node_1_publisher_names_and_types.get(&topic2).unwrap();
     assert!(types.contains(&"test_msgs/msg/BasicTypes".to_string()));
 
     let node_2_publisher_names_and_types = graph
-        .node2
-        .get_publisher_names_and_types_by_node(&graph.node2.name(), &graph.node2.namespace())?;
+        .node1
+        .get_publisher_names_and_types_by_node(&graph.node2.name(), namespace)?;
 
-    let types = node_2_publisher_names_and_types
-        .get(&(graph.node2.namespace() + "/graph_test_topic_3"))
-        .unwrap();
+    let types = node_2_publisher_names_and_types.get(&topic3).unwrap();
+    assert!(types.contains(&"test_msgs/msg/Defaults".to_string()));
+
+    // Test get_publishers_info_by_topic()
+    let expected_publishers_info = vec![TopicEndpointInfo {
+        node_name: String::from("graph_test_node_1"),
+        node_namespace: String::from(namespace),
+        topic_type: String::from("test_msgs/msg/Empty"),
+    }];
     assert_eq!(
-        graph
-            .node2
-            .count_publishers(&(graph.node2.namespace() + "/graph_test_topic_3"))?,
-        1
+        graph.node1.get_publishers_info_by_topic(&topic1)?,
+        expected_publishers_info
+    );
+    assert_eq!(
+        graph.node2.get_publishers_info_by_topic(&topic1)?,
+        expected_publishers_info
     );
-    assert!(types.contains(&"test_msgs/msg/Defaults".to_string()));
 
     Ok(())
 }
 
 #[test]
 fn test_subscriptions() -> Result<(), RclrsError> {
-    let mut graph = construct_test_graph("test_subscriptions_graph")?;
+    let namespace = "/test_subscriptions_graph";
+    let mut graph = construct_test_graph(namespace)?;
 
-    let _node_1_defaults_subscription = graph.node1.create_subscription::<msg::Defaults, _>(
-        "graph_test_topic_3",
-        QOS_PROFILE_SYSTEM_DEFAULT,
-        |_msg: msg::Defaults| {},
-    )?;
-    let _node_2_empty_subscription = graph.node2.create_subscription::<msg::Empty, _>(
+    let node_2_empty_subscription = graph.node2.create_subscription::<msg::Empty, _>(
         "graph_test_topic_1",
         QOS_PROFILE_SYSTEM_DEFAULT,
         |_msg: msg::Empty| {},
     )?;
-    let _node_2_basic_types_subscription = graph.node2.create_subscription::<msg::BasicTypes, _>(
+    let topic1 = node_2_empty_subscription.topic_name();
+    let node_2_basic_types_subscription = graph.node2.create_subscription::<msg::BasicTypes, _>(
         "graph_test_topic_2",
         QOS_PROFILE_SYSTEM_DEFAULT,
         |_msg: msg::BasicTypes| {},
     )?;
+    let topic2 = node_2_basic_types_subscription.topic_name();
+    let node_1_defaults_subscription = graph.node1.create_subscription::<msg::Defaults, _>(
+        "graph_test_topic_3",
+        QOS_PROFILE_SYSTEM_DEFAULT,
+        |_msg: msg::Defaults| {},
+    )?;
+    let topic3 = node_1_defaults_subscription.topic_name();
 
     std::thread::sleep(std::time::Duration::from_millis(100));
 
+    // Test count_subscriptions()
+    assert_eq!(graph.node2.count_subscriptions(&topic1)?, 1);
+    assert_eq!(graph.node2.count_subscriptions(&topic2)?, 1);
+
+    // Test get_subscription_names_and_types_by_node()
     let node_1_subscription_names_and_types = graph
         .node1
-        .get_subscription_names_and_types_by_node(&graph.node1.name(), &graph.node1.namespace())?;
+        .get_subscription_names_and_types_by_node(&graph.node1.name(), namespace)?;
 
-    let types = node_1_subscription_names_and_types
-        .get(&(graph.node2.namespace() + "/graph_test_topic_3"))
-        .unwrap();
+    let types = node_1_subscription_names_and_types.get(&topic3).unwrap();
     assert!(types.contains(&"test_msgs/msg/Defaults".to_string()));
 
-    assert_eq!(
-        graph
-            .node2
-            .count_subscriptions(&(graph.node2.namespace() + "/graph_test_topic_1"))?,
-        1
-    );
-    assert_eq!(
-        graph
-            .node2
-            .count_subscriptions(&(graph.node2.namespace() + "/graph_test_topic_2"))?,
-        1
-    );
-
     let node_2_subscription_names_and_types = graph
         .node2
-        .get_subscription_names_and_types_by_node(&graph.node2.name(), &graph.node2.namespace())?;
+        .get_subscription_names_and_types_by_node(&graph.node2.name(), namespace)?;
 
-    let types = node_2_subscription_names_and_types
-        .get(&(graph.node1.namespace() + "/graph_test_topic_1"))
-        .unwrap();
+    let types = node_2_subscription_names_and_types.get(&topic1).unwrap();
     assert!(types.contains(&"test_msgs/msg/Empty".to_string()));
 
-    let types = node_2_subscription_names_and_types
-        .get(&(graph.node2.namespace() + "/graph_test_topic_2"))
-        .unwrap();
+    let types = node_2_subscription_names_and_types.get(&topic2).unwrap();
     assert!(types.contains(&"test_msgs/msg/BasicTypes".to_string()));
 
+    // Test get_subscriptions_info_by_topic()
+    let expected_subscriptions_info = vec![TopicEndpointInfo {
+        node_name: String::from("graph_test_node_2"),
+        node_namespace: String::from(namespace),
+        topic_type: String::from("test_msgs/msg/Empty"),
+    }];
+    assert_eq!(
+        graph.node1.get_subscriptions_info_by_topic(&topic1)?,
+        expected_subscriptions_info
+    );
+    assert_eq!(
+        graph.node2.get_subscriptions_info_by_topic(&topic1)?,
+        expected_subscriptions_info
+    );
     Ok(())
 }
 
@@ -184,7 +190,8 @@ fn test_topic_names_and_types() -> Result<(), RclrsError> {
 
 #[test]
 fn test_services() -> Result<(), RclrsError> {
-    let mut graph = construct_test_graph("test_services_graph")?;
+    let namespace = "/test_services_graph";
+    let mut graph = construct_test_graph(namespace)?;
     let check_names_and_types = |names_and_types: TopicNamesAndTypes| {
         let types = names_and_types
             .get("/test_services_graph/graph_test_topic_4")
@@ -209,15 +216,16 @@ fn test_services() -> Result<(), RclrsError> {
 
     let service_names_and_types = graph
         .node1
-        .get_service_names_and_types_by_node(&graph.node1.name(), &graph.node1.namespace())?;
+        .get_service_names_and_types_by_node(&graph.node1.name(), namespace)?;
     check_names_and_types(service_names_and_types);
 
     Ok(())
 }
 
 #[test]
 fn test_clients() -> Result<(), RclrsError> {
-    let mut graph = construct_test_graph("test_clients_graph")?;
+    let namespace = "/test_clients_graph";
+    let mut graph = construct_test_graph(namespace)?;
     let _node_2_empty_client = graph
         .node2
         .create_client::<srv::Empty>("graph_test_topic_4")?;