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

Support service 2/2 --- rosbag2 service play #1481

Merged
merged 73 commits into from
Apr 13, 2024
Merged
Show file tree
Hide file tree
Changes from 64 commits
Commits
Show all changes
73 commits
Select commit Hold shift + click to select a range
bd06295
Implement service play
Barry-Xu-2018 Oct 20, 2023
9f7eeba
Maintain the future queue of the request by timeout
Barry-Xu-2018 Oct 27, 2023
7300741
Use get_message_typesupport_handle() instead of deprecated get_typesu…
Barry-Xu-2018 Dec 6, 2023
caf9134
Resolve the conflicts caused by the rebase
Barry-Xu-2018 Dec 22, 2023
5bb3687
Address review comments from Fujita-san
Barry-Xu-2018 Jan 5, 2024
1c881f0
Move codes to new function remove_complete_request_future
Barry-Xu-2018 Jan 14, 2024
723daab
Move codes to new function remove_all_timeout_request_future()
Barry-Xu-2018 Jan 15, 2024
a4249e2
Add warning log and lock protection
Barry-Xu-2018 Jan 15, 2024
5a29cf5
Optimize code in PlayerImpl::publish_message()
Barry-Xu-2018 Jan 15, 2024
b23cbf6
Changed the code logic for determining whether to use request data of…
Barry-Xu-2018 Jan 15, 2024
81ff4f2
Add warning log and tests
Barry-Xu-2018 Jan 17, 2024
2b85f7f
Correct the parameter descriptions and remove unnecessary code
Barry-Xu-2018 Feb 3, 2024
d024a9b
Extend the parameters for the "play" command
Barry-Xu-2018 Feb 5, 2024
50ecde9
Move implmentation of struct client_id_hash to cpp
Barry-Xu-2018 Feb 5, 2024
2cfa9e6
Address some minor review comments
Barry-Xu-2018 Feb 5, 2024
2315487
Replace std::variant<SharedPlayerPublisher, SharedPlayerClient>>
Barry-Xu-2018 Feb 6, 2024
e5fad27
Added code for cleaning up pending requests
Barry-Xu-2018 Feb 6, 2024
5a96af4
Avoid creating publisher or client for filtered topic
Barry-Xu-2018 Feb 6, 2024
7fd8786
Update code on filtering message for mcap
Barry-Xu-2018 Feb 6, 2024
bf8decc
Use explicit namespace for topic name and update tests
Barry-Xu-2018 Feb 23, 2024
99df5a5
Update for the rebase
Barry-Xu-2018 Feb 26, 2024
06d8911
Simplify variable names
Barry-Xu-2018 Feb 26, 2024
6e9032c
Adjust the default timeout and queue length for request future
Barry-Xu-2018 Mar 4, 2024
2a265b7
Cleanup and optimization in mcap_storage.cpp
MichaelOrlov Mar 22, 2024
bd2e1de
Move mcap topic_filter tests to a separate file
MichaelOrlov Mar 23, 2024
b9db0bf
Add TestResetFilter and CanSelectWithTopicsListOnly to the mcap storage
MichaelOrlov Mar 25, 2024
c5fd5b3
Add CanSelectWithServiceEventsListOnly test to the mcap storage
MichaelOrlov Mar 25, 2024
e784e70
Cleanups and renames in the sqlite_storage.cpp
MichaelOrlov Mar 21, 2024
a94777e
Move SQLite3 topic_filter tests to a separate file
MichaelOrlov Mar 25, 2024
1de50be
Bugfix in sqlite topic filters when service_events or topics lists empty
MichaelOrlov Mar 25, 2024
72abcdf
Rename test_sqlite3_topic_filter.cpp for consistency
MichaelOrlov Mar 25, 2024
456a184
Change API to use rcl_serialized_message_t to avoid 2 extra message copy
MichaelOrlov Mar 21, 2024
154a1c7
Misc minor fixes from previous rounds of review and new findings (1)
MichaelOrlov Mar 21, 2024
8594720
Address ament_flake8 warning in regards whitespaces in the blank lines
MichaelOrlov Mar 25, 2024
d7f0f49
Simplify logic in is_topic_selected_by_white_list_or_regex(..)
MichaelOrlov Mar 28, 2024
fa6b96d
Check is_topic_in_black_list_or_exclude_regex(..) first
MichaelOrlov Mar 28, 2024
8e626df
Add test coverage for the cases when exclude lists overlap with include
MichaelOrlov Mar 28, 2024
627c4be
Bugfix for incorrectly including all services when regex is not empty
MichaelOrlov Mar 28, 2024
a844532
Rename test_sqlite3_topic_filter in CMakeList.txt
MichaelOrlov Mar 28, 2024
c356bdb
Update spin & termination and add service ready check before play
Barry-Xu-2018 Mar 29, 2024
9fbce1b
Fix the issue of published_messages_from_multiple_services_are_record…
Barry-Xu-2018 Mar 29, 2024
7860163
Make service_event_ts_lib as private member again
MichaelOrlov Mar 31, 2024
842ac41
Cleanup in PlayerServiceClient::async_send_request(ser_message)
MichaelOrlov Mar 31, 2024
924ac88
Refactoring. Do full deserialization and only once
MichaelOrlov Mar 31, 2024
0ffc4f7
Specify service request from which introspection message and fix uncr…
Barry-Xu-2018 Apr 7, 2024
8d27495
Revert uncrustify changes from previous commit.
MichaelOrlov Apr 9, 2024
ed823cc
Rename service_request_from to the service_requests_source
MichaelOrlov Apr 9, 2024
96429de
Add Player::wait_for_sent_service_requests_to_finish() API
MichaelOrlov Apr 10, 2024
85ca975
Mitigate potential issues related to the operations reordering on ARM
MichaelOrlov Apr 10, 2024
ae5bc26
Make tests play_service_requests_from_service(client) deterministic
MichaelOrlov Apr 10, 2024
9e7089c
Misc findings and improvements 1
MichaelOrlov Apr 10, 2024
3e6675f
Rename get_services_clients() to the get_service_clients()
MichaelOrlov Apr 10, 2024
c5fc51c
Add a new CLI parameter "--publish-service-requests" for Player
Barry-Xu-2018 Apr 8, 2024
15f5562
Fix an issue on filtering topic when prepare publishers
MichaelOrlov Apr 10, 2024
a3f9418
Cleanup in play_without_publish_service_requests
MichaelOrlov Apr 10, 2024
35cf8e8
Wrap code which can throw with try-catch in the publish_message(..)
MichaelOrlov Apr 11, 2024
bb61984
Delete some part of the code which became absolute and shall not be used
MichaelOrlov Apr 11, 2024
c853d60
Update test codes
Barry-Xu-2018 Apr 11, 2024
599f390
Remove code on meaningless waiting for published_messages_from_multip…
Barry-Xu-2018 Apr 12, 2024
19e7f62
Update the code following the rebase
Barry-Xu-2018 Apr 12, 2024
22958b5
Remove a unnecessary check and simplify the code
Barry-Xu-2018 Apr 12, 2024
d686e15
Cleanup in service replay related tests
MichaelOrlov Apr 12, 2024
318f1b7
Regenerate Python stub files (.pyi) after altering python API in PR
MichaelOrlov Apr 12, 2024
728b791
Increase the timeout of waiting for the service to be ready
Barry-Xu-2018 Apr 12, 2024
dc19f60
Update the code for waiting on all futures of one service client
Barry-Xu-2018 Apr 12, 2024
1577041
Cleanup API for wait_for_sent_requests_to_finish(..)
MichaelOrlov Apr 12, 2024
dbd0212
Fixes for Windows CI build failure
MichaelOrlov Apr 13, 2024
96dce04
Fix a typo
Barry-Xu-2018 Apr 13, 2024
87526a7
Increase timeout value to stabilize a test
Barry-Xu-2018 Apr 13, 2024
e95a51e
Fix a bug in PlayerImpl::wait_for_sent_service_requests_to_finish
Barry-Xu-2018 Apr 13, 2024
aaaac74
Disable test_burst for RTI DDS due to the failure with missing requests
MichaelOrlov Apr 13, 2024
f41b3d3
Revert "Disable test_burst for RTI DDS due to the failure with missin…
MichaelOrlov Apr 13, 2024
6c6c00a
Disable burst_bursting_only_filtered_services for rmw_connextdds
MichaelOrlov Apr 13, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions ros2bag/ros2bag/verb/burst.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
from ros2bag.api import add_standard_reader_args
from ros2bag.api import check_not_negative_int
from ros2bag.api import check_positive_float
from ros2bag.api import convert_service_to_service_event_topic
from ros2bag.api import convert_yaml_to_qos_profile
from ros2bag.api import print_error
from ros2bag.verb import VerbExtension
Expand All @@ -41,8 +42,12 @@ def add_arguments(self, parser, cli_name): # noqa: D102
'delay of message playback.')
parser.add_argument(
'--topics', type=str, default=[], nargs='+',
help='topics to replay, separated by space. If none specified, all topics will be '
'replayed.')
help='topics to replay, separated by space. At least one topic needs to be '
"specified. If this parameter isn\'t specified, all topics will be replayed.")
parser.add_argument(
'--services', type=str, default=[], nargs='+',
help='services to replay, separated by space. At least one service needs to be '
"specified. If this parameter isn\'t specified, all services will be replayed.")
parser.add_argument(
'--qos-profile-overrides-path', type=FileType('r'),
help='Path to a yaml file defining overrides of the QoS profile for specific topics.')
Expand Down Expand Up @@ -90,6 +95,8 @@ def main(self, *, args): # noqa: D102
play_options.node_prefix = NODE_NAME_PREFIX
play_options.rate = 1.0
play_options.topics_to_filter = args.topics
# Convert service name to service event topic name
play_options.services_to_filter = convert_service_to_service_event_topic(args.services)
play_options.topic_qos_profile_overrides = qos_profile_overrides
play_options.loop = False
play_options.topic_remapping_options = topic_remapping
Expand Down
51 changes: 43 additions & 8 deletions ros2bag/ros2bag/verb/play.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,14 @@
from ros2bag.api import add_standard_reader_args
from ros2bag.api import check_not_negative_int
from ros2bag.api import check_positive_float
from ros2bag.api import convert_service_to_service_event_topic
from ros2bag.api import convert_yaml_to_qos_profile
from ros2bag.api import print_error
from ros2bag.verb import VerbExtension
from ros2cli.node import NODE_NAME_PREFIX
from rosbag2_py import Player
from rosbag2_py import PlayOptions
from rosbag2_py import ServiceRequestsSource
from rosbag2_py import StorageOptions
import yaml

Expand All @@ -49,16 +51,23 @@ def add_arguments(self, parser, cli_name): # noqa: D102
'-r', '--rate', type=check_positive_float, default=1.0,
help='rate at which to play back messages. Valid range > 0.0.')
parser.add_argument(
'--topics', type=str, default=[], nargs='+',
'--topics', type=str, default=[], metavar='topic', nargs='+',
help='Space-delimited list of topics to play.')
parser.add_argument(
'--services', type=str, default=[], metavar='service', nargs='+',
help='Space-delimited list of services to play.')
parser.add_argument(
'-e', '--regex', default='',
help='filter topics by regular expression to replay, separated by space. If none '
'specified, all topics will be replayed.')
help='Play only topics and services matches with regular expression.')
parser.add_argument(
'-x', '--exclude-regex', default='',
help='regular expressions to exclude topics and services from replay.')
parser.add_argument(
'--exclude-topics', type=str, default=[], metavar='topic', nargs='+',
help='Space-delimited list of topics not to play.')
parser.add_argument(
'-x', '--exclude', default='',
help='regular expressions to exclude topics from replay, separated by space. If none '
'specified, all topics will be replayed.')
'--exclude-services', type=str, default=[], metavar='service', nargs='+',
help='Space-delimited list of services not to play.')
parser.add_argument(
'--qos-profile-overrides-path', type=FileType('r'),
help='Path to a yaml file defining overrides of the QoS profile for specific topics.')
Expand Down Expand Up @@ -144,6 +153,15 @@ def add_arguments(self, parser, cli_name): # noqa: D102
'By default, if loaned message can be used, messages are published as loaned '
'message. It can help to reduce the number of data copies, so there is a greater '
'benefit for sending big data.')
parser.add_argument(
'--publish-service-requests', action='store_true', default=False,
help='Publish recorded service requests instead of recorded service events')
parser.add_argument(
'--service-requests-source', default='service_introspection',
choices=['service_introspection', 'client_introspection'],
help='Determine the source of the service requests to be replayed. This option only '
'makes sense if the "--publish-service-requests" option is set. By default,'
' the service requests replaying from recorded service introspection message.')

def get_playback_until_from_arg_group(self, playback_until_sec, playback_until_nsec) -> int:
nano_scale = 1000 * 1000 * 1000
Expand Down Expand Up @@ -182,8 +200,19 @@ def main(self, *, args): # noqa: D102
play_options.node_prefix = NODE_NAME_PREFIX
play_options.rate = args.rate
play_options.topics_to_filter = args.topics
play_options.topics_regex_to_filter = args.regex
play_options.topics_regex_to_exclude = args.exclude

# Convert service name to service event topic name
play_options.services_to_filter = convert_service_to_service_event_topic(args.services)

play_options.regex_to_filter = args.regex

play_options.exclude_regex_to_filter = args.exclude_regex

play_options.exclude_topics_to_filter = args.exclude_topics if args.exclude_topics else []

play_options.exclude_service_events_to_filter = \
convert_service_to_service_event_topic(args.exclude_services)

play_options.topic_qos_profile_overrides = qos_profile_overrides
play_options.loop = args.loop
play_options.topic_remapping_options = topic_remapping
Expand All @@ -200,6 +229,12 @@ def main(self, *, args): # noqa: D102
play_options.start_offset = args.start_offset
play_options.wait_acked_timeout = args.wait_for_all_acked
play_options.disable_loan_message = args.disable_loan_message
play_options.publish_service_requests = args.publish_service_requests
if not args.service_requests_source or \
args.service_requests_source == 'service_introspection':
play_options.service_requests_source = ServiceRequestsSource.SERVICE_INTROSPECTION
else:
play_options.service_requests_source = ServiceRequestsSource.CLIENT_INTROSPECTION

player = Player()
try:
Expand Down
6 changes: 5 additions & 1 deletion rosbag2_cpp/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,11 @@ if(BUILD_TESTING)
ament_add_gmock(test_service_utils
test/rosbag2_cpp/test_service_utils.cpp)
if(TARGET test_service_utils)
target_link_libraries(test_service_utils ${PROJECT_NAME})
target_link_libraries(test_service_utils
${PROJECT_NAME}
rosbag2_test_common::rosbag2_test_common
${test_msgs_TARGETS}
)
endif()
endif()

Expand Down
24 changes: 12 additions & 12 deletions rosbag2_cpp/include/rosbag2_cpp/logging.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,36 +28,36 @@
RCUTILS_LOG_INFO_NAMED(ROSBAG2_CPP_PACKAGE_NAME, __VA_ARGS__)

#define ROSBAG2_CPP_LOG_INFO_STREAM(args) do { \
std::stringstream __ss; \
__ss << args; \
RCUTILS_LOG_INFO_NAMED(ROSBAG2_CPP_PACKAGE_NAME, "%s", __ss.str().c_str()); \
std::stringstream __ss; \
__ss << args; \
RCUTILS_LOG_INFO_NAMED(ROSBAG2_CPP_PACKAGE_NAME, "%s", __ss.str().c_str()); \
} while (0)

#define ROSBAG2_CPP_LOG_ERROR(...) \
RCUTILS_LOG_ERROR_NAMED(ROSBAG2_CPP_PACKAGE_NAME, __VA_ARGS__)

#define ROSBAG2_CPP_LOG_ERROR_STREAM(args) do { \
std::stringstream __ss; \
__ss << args; \
RCUTILS_LOG_ERROR_NAMED(ROSBAG2_CPP_PACKAGE_NAME, "%s", __ss.str().c_str()); \
std::stringstream __ss; \
__ss << args; \
RCUTILS_LOG_ERROR_NAMED(ROSBAG2_CPP_PACKAGE_NAME, "%s", __ss.str().c_str()); \
} while (0)

#define ROSBAG2_CPP_LOG_WARN(...) \
RCUTILS_LOG_WARN_NAMED(ROSBAG2_CPP_PACKAGE_NAME, __VA_ARGS__)

#define ROSBAG2_CPP_LOG_WARN_STREAM(args) do { \
std::stringstream __ss; \
__ss << args; \
RCUTILS_LOG_WARN_NAMED(ROSBAG2_CPP_PACKAGE_NAME, "%s", __ss.str().c_str()); \
std::stringstream __ss; \
__ss << args; \
RCUTILS_LOG_WARN_NAMED(ROSBAG2_CPP_PACKAGE_NAME, "%s", __ss.str().c_str()); \
} while (0)

#define ROSBAG2_CPP_LOG_DEBUG(...) \
RCUTILS_LOG_DEBUG_NAMED(ROSBAG2_CPP_PACKAGE_NAME, __VA_ARGS__)

#define ROSBAG2_CPP_LOG_DEBUG_STREAM(args) do { \
std::stringstream __ss; \
__ss << args; \
RCUTILS_LOG_DEBUG_NAMED(ROSBAG2_CPP_PACKAGE_NAME, "%s", __ss.str().c_str()); \
std::stringstream __ss; \
__ss << args; \
RCUTILS_LOG_DEBUG_NAMED(ROSBAG2_CPP_PACKAGE_NAME, "%s", __ss.str().c_str()); \
} while (0)

// *INDENT-ON*
Expand Down
19 changes: 15 additions & 4 deletions rosbag2_cpp/include/rosbag2_cpp/service_utils.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,18 @@
#ifndef ROSBAG2_CPP__SERVICE_UTILS_HPP_
#define ROSBAG2_CPP__SERVICE_UTILS_HPP_

#include <array>
#include <string>

#include "rosbag2_cpp/visibility_control.hpp"

#include "service_msgs/msg/service_event_info.hpp"

namespace rosbag2_cpp
{
ROSBAG2_CPP_PUBLIC
bool
is_service_event_topic(const std::string & topic, const std::string & topic_type);
is_service_event_topic(const std::string & topic_name, const std::string & topic_type);

// Call this function after is_service_event_topic() return true
ROSBAG2_CPP_PUBLIC
Expand All @@ -36,12 +39,20 @@ std::string
service_event_topic_type_to_service_type(const std::string & topic_type);

ROSBAG2_CPP_PUBLIC
size_t
get_serialization_size_for_service_metadata_event();
std::string
service_name_to_service_event_topic_name(const std::string & service_name);

ROSBAG2_CPP_PUBLIC
std::string
service_name_to_service_event_topic_name(const std::string & service_name);
client_id_to_string(std::array<uint8_t, 16> & client_id);

struct client_id_hash
{
static_assert(
std::is_same<std::array<uint8_t, 16>,
service_msgs::msg::ServiceEventInfo::_client_gid_type>::value);
std::size_t operator()(const std::array<uint8_t, 16> & client_id) const;
};
} // namespace rosbag2_cpp

#endif // ROSBAG2_CPP__SERVICE_UTILS_HPP_
17 changes: 0 additions & 17 deletions rosbag2_cpp/src/rosbag2_cpp/info.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,23 +60,6 @@ rosbag2_storage::BagMetadata Info::read_metadata(

namespace
{
struct client_id_hash
{
static_assert(
std::is_same<std::array<uint8_t, 16>,
service_msgs::msg::ServiceEventInfo::_client_gid_type>::value);
std::size_t operator()(const std::array<uint8_t, 16> & client_id) const
{
std::hash<uint8_t> hasher;
std::size_t seed = 0;
for (const auto & value : client_id) {
// 0x9e3779b9 is from https://cryptography.fandom.com/wiki/Tiny_Encryption_Algorithm
seed ^= hasher(value) + 0x9e3779b9 + (seed << 6) + (seed >> 2);
}
return seed;
}
};

using client_id = service_msgs::msg::ServiceEventInfo::_client_gid_type;
using sequence_set = std::unordered_set<int64_t>;
struct service_req_resp_info
Expand Down
Loading
Loading