Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add "--sort" CLI option to the "ros2 bag info" command #1804

Merged
merged 11 commits into from
Oct 17, 2024
11 changes: 8 additions & 3 deletions ros2bag/ros2bag/verb/info.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,21 @@ def add_arguments(self, parser, cli_name): # noqa: D102
'-v', '--verbose', action='store_true',
help='Display request/response information for services'
)
available_sorting_methods = Info().get_sorting_methods()
parser.add_argument(
'--sort', default='name', choices=available_sorting_methods,
help='Configuration for sorting of output. '
'By default sorts by topic name - use this argument to override.')

def main(self, *, args): # noqa: D102
if args.topic_name and args.verbose:
print("Warning! You have set both the '-t' and '-v' parameters. The '-t' parameter "
'will be ignored.')
m = Info().read_metadata(args.bag_path, args.storage)
if args.verbose:
Info().print_output_verbose(args.bag_path, m)
Info().print_output_verbose(args.bag_path, m, args.sort)
else:
if args.topic_name:
Info().print_output_topic_name_only(m)
Info().print_output_topic_name_only(m, args.sort)
else:
Info().print_output(m)
Info().print_output(m, args.sort)
2 changes: 2 additions & 0 deletions rosbag2_py/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -86,6 +87,7 @@ pybind11_add_module(_info SHARED
src/rosbag2_py/_info.cpp
src/rosbag2_py/format_bag_metadata.cpp
src/rosbag2_py/format_service_info.cpp
src/rosbag2_py/info_sorting_method.cpp
)
target_link_libraries(_info PUBLIC
rosbag2_cpp::rosbag2_cpp
Expand Down
7 changes: 4 additions & 3 deletions rosbag2_py/rosbag2_py/_info.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ import rosbag2_py._storage

class Info:
def __init__(self) -> None: ...
def print_output(self, arg0: rosbag2_py._storage.BagMetadata) -> None: ...
def print_output_topic_name_only(self, arg0: rosbag2_py._storage.BagMetadata) -> None: ...
def print_output_verbose(self, arg0: str, arg1: rosbag2_py._storage.BagMetadata) -> None: ...
def get_sorting_methods(self) -> Set[str]: ...
def print_output(self, arg0: rosbag2_py._storage.BagMetadata, arg1: str) -> None: ...
def print_output_topic_name_only(self, arg0: rosbag2_py._storage.BagMetadata, arg1: str) -> None: ...
def print_output_verbose(self, arg0: str, arg1: rosbag2_py._storage.BagMetadata, arg2: str) -> None: ...
Comment on lines +5 to +8
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like we are changing API here, which is publicly exposed. Likely we can't backport this PR.
@clalancette Could you please give a second opinion on that?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not totally familiar with what is going on here, but I can offer a couple of thoughts:

  1. It looks to me that the API break is only in adding an additional parameter to print_output, print_output_topic_name_only, and print_output_verbose. Given that this is Python, can we give that final parameter a default value, so that anything depending on the "old" API will continue to work?
  2. The intent of the "no breaks in API" rule is so that third-party code that depends on these APIs continue to work across updates to the core. If these are considered entirely internal APIs (i.e. there are no external users), then it is probably acceptable to break API here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@clalancette Thank you for the information I will take it into consideration in the backporting PR #1838.

def read_metadata(self, arg0: str, arg1: str) -> rosbag2_py._storage.BagMetadata: ...
41 changes: 33 additions & 8 deletions rosbag2_py/src/rosbag2_py/_info.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@
#include <iostream>
#include <memory>
#include <string>
#include <algorithm>

#include "info_sorting_method.hpp"
#include "format_bag_metadata.hpp"
#include "format_service_info.hpp"
#include "rosbag2_cpp/info.hpp"
Expand All @@ -42,15 +44,24 @@ class Info
return info_->read_metadata(uri, storage_id);
}

void print_output(const rosbag2_storage::BagMetadata & metadata_info)
void print_output(
const rosbag2_storage::BagMetadata & metadata_info, const std::string & sorting_method)
{
InfoSortingMethod sort_method = info_sorting_method_from_string(sorting_method);
// Output formatted metadata
std::cout << format_bag_meta_data(metadata_info) << std::endl;
std::cout << format_bag_meta_data(metadata_info, {}, false, false, sort_method) << std::endl;
}

void print_output_topic_name_only(const rosbag2_storage::BagMetadata & metadata_info)
void print_output_topic_name_only(
const rosbag2_storage::BagMetadata & metadata_info, const std::string & sorting_method)
{
for (const auto & topic_info : metadata_info.topics_with_message_count) {
InfoSortingMethod sort_method = info_sorting_method_from_string(sorting_method);
std::vector<size_t> sorted_idx = generate_sorted_idx(
metadata_info.topics_with_message_count,
sort_method);

for (auto idx : sorted_idx) {
const auto & topic_info = metadata_info.topics_with_message_count[idx];
if (!rosbag2_cpp::is_service_event_topic(
topic_info.topic_metadata.name,
topic_info.topic_metadata.type))
Expand All @@ -61,7 +72,9 @@ class Info
}

void print_output_verbose(
const std::string & uri, const rosbag2_storage::BagMetadata & metadata_info)
const std::string & uri,
const rosbag2_storage::BagMetadata & metadata_info,
const std::string & sorting_method)
{
std::vector<std::shared_ptr<rosbag2_cpp::rosbag2_service_info_t>> all_services_info;
for (auto & file_info : metadata_info.files) {
Expand All @@ -86,9 +99,20 @@ 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);
std::cout << format_service_info(all_services_info, messages_size, true) << std::endl;
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, sort_method) << std::endl;
}

std::unordered_set<std::string> get_sorting_methods()
{
std::unordered_set<std::string> sorting_methods;
for (auto sorting_method : rosbag2_py::sorting_method_map) {
sorting_methods.insert(sorting_method.first);
}
return sorting_methods;
}

protected:
Expand All @@ -105,5 +129,6 @@ PYBIND11_MODULE(_info, m) {
.def("read_metadata", &rosbag2_py::Info::read_metadata)
.def("print_output", &rosbag2_py::Info::print_output)
.def("print_output_topic_name_only", &rosbag2_py::Info::print_output_topic_name_only)
.def("print_output_verbose", &rosbag2_py::Info::print_output_verbose);
.def("print_output_verbose", &rosbag2_py::Info::print_output_verbose)
.def("get_sorting_methods", &rosbag2_py::Info::get_sorting_methods);
}
68 changes: 35 additions & 33 deletions rosbag2_py/src/rosbag2_py/format_bag_metadata.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include "rosbag2_storage/bag_metadata.hpp"

#include "format_bag_metadata.hpp"
#include "service_event_info.hpp"

namespace
{
Expand Down Expand Up @@ -115,7 +116,8 @@ void format_topics_with_type(
const std::unordered_map<std::string, uint64_t> & messages_size,
bool verbose,
std::stringstream & info_stream,
int indentation_spaces)
int indentation_spaces,
const rosbag2_py::InfoSortingMethod sort_method = rosbag2_py::InfoSortingMethod::NAME)
{
if (topics.empty()) {
info_stream << std::endl;
Expand All @@ -139,13 +141,15 @@ void format_topics_with_type(
info_stream << std::endl;
};

std::vector<size_t> sorted_idx = rosbag2_py::generate_sorted_idx(topics, sort_method);

size_t number_of_topics = topics.size();
size_t i = 0;
// Find first topic which isn't service event topic
while (i < number_of_topics &&
rosbag2_cpp::is_service_event_topic(
topics[i].topic_metadata.name,
topics[i].topic_metadata.type))
topics[sorted_idx[i]].topic_metadata.name,
topics[sorted_idx[i]].topic_metadata.type))
{
i++;
}
Expand All @@ -155,43 +159,31 @@ void format_topics_with_type(
return;
}

print_topic_info(topics[i]);
print_topic_info(topics[sorted_idx[i]]);
for (size_t j = ++i; j < number_of_topics; ++j) {
if (rosbag2_cpp::is_service_event_topic(
topics[j].topic_metadata.name, topics[j].topic_metadata.type))
topics[sorted_idx[j]].topic_metadata.name, topics[sorted_idx[j]].topic_metadata.type))
{
continue;
}
indent(info_stream, indentation_spaces);
print_topic_info(topics[j]);
print_topic_info(topics[sorted_idx[j]]);
}
}

struct ServiceMetadata
{
std::string name;
std::string type;
std::string serialization_format;
};

struct ServiceInformation
{
ServiceMetadata service_metadata;
size_t event_message_count = 0;
};

std::vector<std::shared_ptr<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<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<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 =
Expand All @@ -208,11 +200,12 @@ std::vector<std::shared_ptr<ServiceInformation>> filter_service_event_topic(
}

void format_service_with_type(
const std::vector<std::shared_ptr<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,
int indentation_spaces)
int indentation_spaces,
const rosbag2_py::InfoSortingMethod sort_method = rosbag2_py::InfoSortingMethod::NAME)
{
if (services.empty()) {
info_stream << std::endl;
Expand All @@ -221,7 +214,7 @@ void format_service_with_type(

auto print_service_info =
[&info_stream, &messages_size, verbose](
const std::shared_ptr<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 << " | ";
Expand All @@ -238,11 +231,13 @@ void format_service_with_type(
info_stream << std::endl;
};

print_service_info(services[0]);
std::vector<size_t> sorted_idx = rosbag2_py::generate_sorted_idx(services, sort_method);

print_service_info(services[sorted_idx[0]]);
auto number_of_services = services.size();
for (size_t j = 1; j < number_of_services; ++j) {
indent(info_stream, indentation_spaces);
print_service_info(services[j]);
print_service_info(services[sorted_idx[j]]);
}
}

Expand All @@ -255,7 +250,8 @@ std::string format_bag_meta_data(
const rosbag2_storage::BagMetadata & metadata,
const std::unordered_map<std::string, uint64_t> & messages_size,
bool verbose,
bool only_topic)
bool only_topic,
const InfoSortingMethod sort_method)
{
auto start_time = metadata.starting_time.time_since_epoch();
auto end_time = start_time + metadata.duration;
Expand All @@ -267,10 +263,8 @@ std::string format_bag_meta_data(
}

size_t total_service_event_msg_count = 0;
std::vector<std::shared_ptr<ServiceInformation>> service_info_list;
service_info_list = filter_service_event_topic(
metadata.topics_with_message_count,
total_service_event_msg_count);
std::vector<std::shared_ptr<ServiceEventInformation>> service_info_list =
filter_service_event_topic(metadata.topics_with_message_count, total_service_event_msg_count);

info_stream << std::endl;
info_stream << "Files: ";
Expand All @@ -288,14 +282,22 @@ std::string format_bag_meta_data(
std::endl;
info_stream << "Topic information: ";
format_topics_with_type(
metadata.topics_with_message_count, messages_size, verbose, info_stream, indentation_spaces);
metadata.topics_with_message_count,
messages_size, verbose, info_stream,
indentation_spaces,
sort_method);

if (!only_topic) {
info_stream << "Service: " << service_info_list.size() << std::endl;
info_stream << "Service information: ";
if (!service_info_list.empty()) {
format_service_with_type(
service_info_list, messages_size, verbose, info_stream, indentation_spaces + 2);
service_info_list,
messages_size,
verbose,
info_stream,
indentation_spaces + 2,
sort_method);
}
}

Expand Down
4 changes: 3 additions & 1 deletion rosbag2_py/src/rosbag2_py/format_bag_metadata.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include <string>
#include <unordered_map>

#include "info_sorting_method.hpp"
#include "rosbag2_storage/bag_metadata.hpp"

namespace rosbag2_py
Expand All @@ -27,7 +28,8 @@ std::string format_bag_meta_data(
const rosbag2_storage::BagMetadata & metadata,
const std::unordered_map<std::string, uint64_t> & messages_size = {},
bool verbose = false,
bool only_topic = false);
bool only_topic = false,
InfoSortingMethod sort_method = InfoSortingMethod::NAME);

} // namespace rosbag2_py

Expand Down
9 changes: 6 additions & 3 deletions rosbag2_py/src/rosbag2_py/format_service_info.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ std::string
format_service_info(
std::vector<std::shared_ptr<rosbag2_cpp::rosbag2_service_info_t>> & service_info_list,
const std::unordered_map<std::string, uint64_t> & messages_size,
bool verbose)
bool verbose,
const InfoSortingMethod sort_method)
{
std::stringstream info_stream;
const std::string service_info_string = "Service information: ";
Expand Down Expand Up @@ -78,11 +79,13 @@ format_service_info(
info_stream << std::endl;
};

print_service_info(service_info_list[0]);
std::vector<size_t> sorted_idx = generate_sorted_idx(service_info_list, sort_method);

print_service_info(service_info_list[sorted_idx[0]]);
auto number_of_services = service_info_list.size();
for (size_t j = 1; j < number_of_services; ++j) {
info_stream << std::string(indentation_spaces, ' ');
print_service_info(service_info_list[j]);
print_service_info(service_info_list[sorted_idx[j]]);
}

return info_stream.str();
Expand Down
4 changes: 3 additions & 1 deletion rosbag2_py/src/rosbag2_py/format_service_info.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include <vector>
#include <unordered_map>

#include "info_sorting_method.hpp"
#include "rosbag2_cpp/info.hpp"

namespace rosbag2_py
Expand All @@ -28,7 +29,8 @@ namespace rosbag2_py
std::string format_service_info(
std::vector<std::shared_ptr<rosbag2_cpp::rosbag2_service_info_t>> & service_info,
const std::unordered_map<std::string, uint64_t> & messages_size = {},
bool verbose = false);
bool verbose = false,
const InfoSortingMethod sort_method = InfoSortingMethod::NAME);

} // namespace rosbag2_py

Expand Down
Loading
Loading