move ServiceInformation struct to its own header file and rename to S…

…erviceEventInformation; replace if-else by switch-case for differantiating between sorting methods; bugfix sorting method from string resolution and service info verbose not being sorted

Signed-off-by: Soenke Prophet <[email protected]>

rosbag2_py/CMakeLists.txt

@@ -67,6 +67,7 @@ target_link_libraries(_reader PUBLIC
 pybind11_add_module(_storage SHARED
   src/rosbag2_py/_storage.cpp
   src/rosbag2_py/format_bag_metadata.cpp
+  src/rosbag2_py/info_sorting_method.cpp
 )
 target_link_libraries(_storage PUBLIC
   rosbag2_cpp::rosbag2_cpp

rosbag2_py/src/rosbag2_py/_info.cpp

@@ -104,7 +104,8 @@ class Info
     rosbag2_py::InfoSortingMethod sort_method = info_sorting_method_from_string(sorting_method);
     // Output formatted metadata and service info
     std::cout << format_bag_meta_data(metadata_info, messages_size, true, true, sort_method);
-    std::cout << format_service_info(all_services_info, messages_size, true) << std::endl;
+    std::cout <<
+      format_service_info(all_services_info, messages_size, true, sort_method) << std::endl;
   }
 
   std::unordered_set<std::string> get_sorting_methods()

rosbag2_py/src/rosbag2_py/format_bag_metadata.cpp

@@ -173,18 +173,18 @@ void format_topics_with_type(
 }
 
 
-std::vector<std::shared_ptr<rosbag2_storage::ServiceInformation>> filter_service_event_topic(
+std::vector<std::shared_ptr<rosbag2_py::ServiceEventInformation>> filter_service_event_topic(
   const std::vector<rosbag2_storage::TopicInformation> & topics_with_message_count,
   size_t & total_service_event_msg_count)
 {
   total_service_event_msg_count = 0;
-  std::vector<std::shared_ptr<rosbag2_storage::ServiceInformation>> service_info_list;
+  std::vector<std::shared_ptr<rosbag2_py::ServiceEventInformation>> service_info_list;
 
   for (auto & topic : topics_with_message_count) {
     if (rosbag2_cpp::is_service_event_topic(
         topic.topic_metadata.name, topic.topic_metadata.type))
     {
-      auto service_info = std::make_shared<rosbag2_storage::ServiceInformation>();
+      auto service_info = std::make_shared<rosbag2_py::ServiceEventInformation>();
       service_info->service_metadata.name =
         rosbag2_cpp::service_event_topic_name_to_service_name(topic.topic_metadata.name);
       service_info->service_metadata.type =
@@ -201,7 +201,7 @@ std::vector<std::shared_ptr<rosbag2_storage::ServiceInformation>> filter_service
 }
 
 void format_service_with_type(
-  const std::vector<std::shared_ptr<rosbag2_storage::ServiceInformation>> & services,
+  const std::vector<std::shared_ptr<rosbag2_py::ServiceEventInformation>> & services,
   const std::unordered_map<std::string, uint64_t> & messages_size,
   bool verbose,
   std::stringstream & info_stream,
@@ -215,7 +215,7 @@ void format_service_with_type(
 
   auto print_service_info =
     [&info_stream, &messages_size, verbose](
-    const std::shared_ptr<rosbag2_storage::ServiceInformation> & si) -> void {
+    const std::shared_ptr<rosbag2_py::ServiceEventInformation> & si) -> void {
       info_stream << "Service: " << si->service_metadata.name << " | ";
       info_stream << "Type: " << si->service_metadata.type << " | ";
       info_stream << "Event Count: " << si->event_message_count << " | ";
@@ -264,7 +264,7 @@ std::string format_bag_meta_data(
   }
 
   size_t total_service_event_msg_count = 0;
-  std::vector<std::shared_ptr<rosbag2_storage::ServiceInformation>> service_info_list;
+  std::vector<std::shared_ptr<ServiceEventInformation>> service_info_list;
   service_info_list = filter_service_event_topic(
     metadata.topics_with_message_count,
     total_service_event_msg_count);

rosbag2_py/src/rosbag2_py/format_bag_metadata.hpp

@@ -19,6 +19,7 @@
 #include <unordered_map>
 
 #include "info_sorting_method.hpp"
+#include "service_event_info.hpp"
 #include "rosbag2_storage/bag_metadata.hpp"
 
 namespace rosbag2_py

rosbag2_py/src/rosbag2_py/format_service_info.cpp

@@ -79,7 +79,36 @@ format_service_info(
       info_stream << std::endl;
     };
 
-  std::vector<size_t> sorted_idx = rosbag2_py::generate_sorted_idx(service_info_list, sort_method);
+  // std::vector<size_t> sorted_idx = generate_sorted_idx(service_info_list, sort_method);
+  std::vector<size_t> sorted_idx(service_info_list.size());
+  std::iota(sorted_idx.begin(), sorted_idx.end(), 0);
+  std::sort(
+    sorted_idx.begin(),
+    sorted_idx.end(),
+    [&service_info_list, sort_method](size_t i1, size_t i2) {
+      bool is_greater = false;
+      switch (sort_method) {
+        case InfoSortingMethod::NAME:
+          is_greater = service_info_list[i1]->name < service_info_list[i2]->name;
+          break;
+        case InfoSortingMethod::TYPE:
+          is_greater = service_info_list[i1]->type < service_info_list[i2]->type;
+          break;
+        case InfoSortingMethod::COUNT:
+          {
+            const auto & count_1 = (
+              service_info_list[i1]->request_count + service_info_list[i1]->response_count);
+            const auto & count_2 = (
+              service_info_list[i2]->request_count + service_info_list[i2]->response_count);
+            is_greater = count_1 < count_2;
+            break;
+          }
+        default:
+          throw std::runtime_error("switch is not exhaustive");
+      }
+      return is_greater;
+    }
+  );
 
   print_service_info(service_info_list[sorted_idx[0]]);
   auto number_of_services = service_info_list.size();

rosbag2_py/src/rosbag2_py/info_sorting_method.cpp

@@ -23,7 +23,7 @@ namespace rosbag2_py
 InfoSortingMethod info_sorting_method_from_string(std::string str)
 {
   std::transform(str.begin(), str.end(), str.begin(), ::tolower);
-  auto find_result = sorting_method_map.find("str");
+  auto find_result = sorting_method_map.find(str);
   if (find_result == sorting_method_map.end()) {
     throw std::runtime_error("Enum value match for \"" + str + "\" string is not found.");
   }
@@ -40,13 +40,21 @@ std::vector<size_t> generate_sorted_idx(
     sorted_idx.begin(),
     sorted_idx.end(),
     [&topics, sort_method](size_t i1, size_t i2) {
-      if (sort_method == InfoSortingMethod::TYPE) {
-        return topics[i1].topic_metadata.type < topics[i2].topic_metadata.type;
+      bool is_greater = false;
+      switch (sort_method) {
+        case InfoSortingMethod::NAME:
+          is_greater = topics[i1].topic_metadata.name < topics[i2].topic_metadata.name;
+          break;
+        case InfoSortingMethod::TYPE:
+          is_greater = topics[i1].topic_metadata.type < topics[i2].topic_metadata.type;
+          break;
+        case InfoSortingMethod::COUNT:
+          is_greater = topics[i1].message_count < topics[i2].message_count;
+          break;
+        default:
+          throw std::runtime_error("switch is not exhaustive");
       }
-      if (sort_method == InfoSortingMethod::COUNT) {
-        return topics[i1].message_count < topics[i2].message_count;
-      }
-      return topics[i1].topic_metadata.name < topics[i2].topic_metadata.name;
+      return is_greater;
     }
   );
   return sorted_idx;
@@ -63,23 +71,33 @@ std::vector<size_t> generate_sorted_idx(
     sorted_idx.begin(),
     sorted_idx.end(),
     [&services, sort_method](size_t i1, size_t i2) {
-      if (sort_method == InfoSortingMethod::TYPE) {
-        return services[i1]->type < services[i2]->type;
-      }
-      if (sort_method == InfoSortingMethod::COUNT) {
-        const auto & count_1 = services[i1]->request_count + services[i1]->response_count;
-        const auto & count_2 = services[i2]->request_count + services[i2]->response_count;
-        return count_1 < count_2;
+      bool is_greater = false;
+      switch (sort_method) {
+        case InfoSortingMethod::NAME:
+          is_greater = services[i1]->name < services[i2]->name;
+          break;
+        case InfoSortingMethod::TYPE:
+          is_greater = services[i1]->type < services[i2]->type;
+          break;
+        case InfoSortingMethod::COUNT:
+          {
+            const auto & count_1 = services[i1]->request_count + services[i1]->response_count;
+            const auto & count_2 = services[i2]->request_count + services[i2]->response_count;
+            is_greater = count_1 < count_2;
+            break;
+          }
+        default:
+          throw std::runtime_error("switch is not exhaustive");
       }
-      return services[i1]->name < services[i2]->name;
+      return is_greater;
     }
   );
   return sorted_idx;
 }
 
 
 std::vector<size_t> generate_sorted_idx(
-  const std::vector<std::shared_ptr<rosbag2_storage::ServiceInformation>> & services,
+  const std::vector<std::shared_ptr<ServiceEventInformation>> & services,
   const InfoSortingMethod sort_method)
 {
   std::vector<size_t> sorted_idx(services.size());
@@ -88,13 +106,21 @@ std::vector<size_t> generate_sorted_idx(
     sorted_idx.begin(),
     sorted_idx.end(),
     [&services, sort_method](size_t i1, size_t i2) {
-      if (sort_method == InfoSortingMethod::TYPE) {
-        return services[i1]->service_metadata.type < services[i2]->service_metadata.type;
-      }
-      if (sort_method == InfoSortingMethod::COUNT) {
-        return services[i1]->event_message_count < services[i2]->event_message_count;
+      bool is_greater = false;
+      switch (sort_method) {
+        case InfoSortingMethod::NAME:
+          is_greater = services[i1]->service_metadata.name < services[i2]->service_metadata.name;
+          break;
+        case InfoSortingMethod::TYPE:
+          is_greater = services[i1]->service_metadata.type < services[i2]->service_metadata.type;
+          break;
+        case InfoSortingMethod::COUNT:
+          is_greater = services[i1]->event_message_count < services[i2]->event_message_count;
+          break;
+        default:
+          throw std::runtime_error("switch is not exhaustive");
       }
-      return services[i1]->service_metadata.name < services[i2]->service_metadata.name;
+      return is_greater;
     }
   );
   return sorted_idx;

rosbag2_py/src/rosbag2_py/info_sorting_method.hpp

@@ -27,6 +27,7 @@
 #include "rosbag2_storage/topic_metadata.hpp"
 #include "rosbag2_storage/bag_metadata.hpp"
 #include "rosbag2_cpp/info.hpp"
+#include "service_event_info.hpp"
 
 namespace rosbag2_py
 {
@@ -55,7 +56,7 @@ std::vector<size_t> generate_sorted_idx(
   const InfoSortingMethod sort_method = InfoSortingMethod::NAME);
 
 std::vector<size_t> generate_sorted_idx(
-  const std::vector<std::shared_ptr<rosbag2_storage::ServiceInformation>> & services,
+  const std::vector<std::shared_ptr<ServiceEventInformation>> & services,
   const InfoSortingMethod sort_method = InfoSortingMethod::NAME);
 
 }  // namespace rosbag2_py

rosbag2_py/src/rosbag2_py/service_event_info.hpp

@@ -0,0 +1,39 @@
+// Copyright 2024 Open Source Robotics Foundation, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+
+#ifndef ROSBAG2_PY__SERVICE_EVENT_INFO_HPP_
+#define ROSBAG2_PY__SERVICE_EVENT_INFO_HPP_
+
+#include <string>
+
+namespace rosbag2_py
+{
+
+struct ServiceMetadata
+{
+  std::string name;
+  std::string type;
+  std::string serialization_format;
+};
+
+struct ServiceEventInformation
+{
+  ServiceMetadata service_metadata;
+  size_t event_message_count = 0;
+};
+
+}  // namespace rosbag2_py
+
+#endif  // ROSBAG2_PY__SERVICE_EVENT_INFO_HPP_

rosbag2_storage/include/rosbag2_storage/bag_metadata.hpp

@@ -32,19 +32,6 @@ struct TopicInformation
   size_t message_count;
 };
 
-struct ServiceMetadata
-{
-  std::string name;
-  std::string type;
-  std::string serialization_format;
-};
-
-struct ServiceInformation
-{
-  ServiceMetadata service_metadata;
-  size_t event_message_count = 0;
-};
-
 struct FileInformation
 {
   std::string path;