Address comments from Fujita-san

Signed-off-by: Barry Xu <[email protected]>

docs/message_definition_encoding.md

@@ -39,6 +39,7 @@ float32 my_float
 ```
 
 Another example is a service message definition for `my_msgs/srv/ExampleSrv` in `ros2msg` form
+
 ```
 # defines a service message that includes a field of a custom message type
 my_msgs/BasicMsg request

rosbag2_cpp/include/rosbag2_cpp/service_utils.hpp

@@ -25,10 +25,12 @@ ROSBAG2_CPP_PUBLIC
 bool
 is_service_event_topic(const std::string & topic, const std::string & topic_type);
 
+// Call this function after is_service_event_topic() return true
 ROSBAG2_CPP_PUBLIC
 std::string
 service_event_topic_name_to_service_name(const std::string & topic_name);
 
+// Call this function after is_service_event_topic() return true
 ROSBAG2_CPP_PUBLIC
 std::string
 service_event_topic_type_to_service_type(const std::string & topic_type);

rosbag2_cpp/src/rosbag2_cpp/service_utils.cpp

@@ -38,6 +38,11 @@ bool is_service_event_topic(const std::string & topic, const std::string & topic
   std::string end_topic_name = topic.substr(
     topic.length() - (sizeof(RCL_SERVICE_INTROSPECTION_TOPIC_POSTFIX) - 1));
 
+  // Should be "/_service_event"
+  if (end_topic_name != RCL_SERVICE_INTROSPECTION_TOPIC_POSTFIX) {
+    return false;
+  }
+
   if (topic_type.length() <= std::strlen(service_event_topic_type_postfix)) {
     return false;
   }
@@ -51,11 +56,10 @@ bool is_service_event_topic(const std::string & topic, const std::string & topic
     return false;
   }
 
-  return (topic_type.compare(
-           topic_type.length() - std::strlen(service_event_topic_type_postfix),
-           std::strlen(service_event_topic_type_postfix),
-           service_event_topic_type_postfix) == 0) &&
-         (end_topic_name == RCL_SERVICE_INTROSPECTION_TOPIC_POSTFIX);
+  return topic_type.compare(
+    topic_type.length() - std::strlen(service_event_topic_type_postfix),
+    std::strlen(service_event_topic_type_postfix),
+    service_event_topic_type_postfix) == 0;
 }
 
 std::string service_event_topic_name_to_service_name(const std::string & topic_name)

rosbag2_test_common/include/rosbag2_test_common/client_manager.hpp

@@ -43,7 +43,7 @@ class ClientManager : public rclcpp::Node
     enable_service_event_contents_(service_event_contents),
     enable_client_event_contents_(client_event_contents)
   {
-    auto echo_process =
+    auto do_nothing_srv_callback =
       [this](const std::shared_ptr<rmw_request_id_t> request_header,
         const std::shared_ptr<typename ServiceT::Request> request,
         std::shared_ptr<typename ServiceT::Response> response) -> void
@@ -54,7 +54,7 @@ class ClientManager : public rclcpp::Node
         (void)response;
       };
 
-    service_ = create_service<ServiceT>(service_name_, echo_process);
+    service_ = create_service<ServiceT>(service_name_, do_nothing_srv_callback);
 
     rcl_service_introspection_state_t introspection_state;
     if (enable_service_event_contents_) {

rosbag2_tests/test/rosbag2_tests/test_rosbag2_info_end_to_end.cpp

@@ -68,6 +68,40 @@ TEST_P(InfoEndToEndTestFixture, info_end_to_end_test) {
       "Serialization Format: cdr"));
 }
 
+TEST_P(InfoEndToEndTestFixture, info_with_verbose_option_and_topic_name_option) {
+  internal::CaptureStdout();
+  auto exit_code = execute_and_wait_until_completion(
+    "ros2 bag info bag_with_topics_and_service_events --verbose --topic-name",
+    bags_path_);
+  std::string output = internal::GetCapturedStdout();
+  auto expected_storage = GetParam();
+  auto expected_file = rosbag2_test_common::bag_filename_for_storage_id(
+    "bag_with_topics_and_service_events", GetParam());
+  std::string expected_ros_distro = "unknown";
+
+  EXPECT_THAT(exit_code, Eq(EXIT_SUCCESS));
+
+  EXPECT_THAT(
+    output, HasSubstr(
+      "Warning! You have set both the '-t' and '-v' parameters. The '-t' parameter "
+      "will be ignored.\n"));
+
+  // The bag size depends on the os and is not asserted, the time is asserted time zone independent
+  EXPECT_THAT(
+    output, ContainsRegex(
+      "\nFiles:             " + expected_file +
+      "\nBag size:          .*B"
+      "\nStorage id:        " + expected_storage +
+      "\nROS Distro:        " + expected_ros_distro +
+      "\nDuration:          0\\.70s"
+      "\nStart:             Nov  7 2023 .*:30:36\\..* \\(1699345836\\..*\\)"
+      "\nEnd:               Nov  7 2023 .*:30:36\\..* \\(1699345836\\..*\\)"
+      "\nMessages:          2"
+      "\nTopic information: "));
+
+  EXPECT_THAT(output, HasSubstr("Service:           2\n"));
+}
+
 TEST_P(InfoEndToEndTestFixture, info_with_verbose_option_end_to_end_test) {
   internal::CaptureStdout();
   auto exit_code = execute_and_wait_until_completion(

rosbag2_transport/src/rosbag2_transport/topic_filter.cpp

@@ -71,6 +71,7 @@ bool topic_in_list(const std::string & topic_name, const std::vector<std::string
   return it != topics.end();
 }
 
+// Check if service event topic is in a service event topic list.
 bool service_in_list(
   const std::string & topic_name,
   const std::vector<std::string> & service_event_topics)