diff --git a/rosbag2_cpp/include/rosbag2_cpp/message_definitions/local_message_definition_source.hpp b/rosbag2_cpp/include/rosbag2_cpp/message_definitions/local_message_definition_source.hpp index 55912f5532..d42cc76f3e 100644 --- a/rosbag2_cpp/include/rosbag2_cpp/message_definitions/local_message_definition_source.hpp +++ b/rosbag2_cpp/include/rosbag2_cpp/message_definitions/local_message_definition_source.hpp @@ -59,17 +59,6 @@ class DefinitionNotFoundError : public std::exception class ROSBAG2_CPP_PUBLIC LocalMessageDefinitionSource final { public: - /** - * Concatenate the message definition with its dependencies into a self-contained schema. - * The format is different for MSG/SRV/ACTION and IDL definitions, and is described fully in - * docs/message_definition_encoding.md - * For SRV type, root_type must include a string '/srv/'. - * For ACTION type, root_type must include a string '/action/'. - */ - [[deprecated("Use get_full_text_ext() instead, which allows specifying the topic name and " - "provides more flexibility in how the message definition is constructed.")]] - rosbag2_storage::MessageDefinition get_full_text(const std::string & root_type); - /** * \brief Try to get the message definition and concatenate it with its dependencies into a * self-contained schema. diff --git a/rosbag2_cpp/src/rosbag2_cpp/message_definitions/local_message_definition_source.cpp b/rosbag2_cpp/src/rosbag2_cpp/message_definitions/local_message_definition_source.cpp index 683d766ca8..6765177bc5 100644 --- a/rosbag2_cpp/src/rosbag2_cpp/message_definitions/local_message_definition_source.cpp +++ b/rosbag2_cpp/src/rosbag2_cpp/message_definitions/local_message_definition_source.cpp @@ -327,12 +327,6 @@ const LocalMessageDefinitionSource::MessageSpec & LocalMessageDefinitionSource:: return spec; } -rosbag2_storage::MessageDefinition LocalMessageDefinitionSource::get_full_text( - const std::string & root_type) -{ - return get_full_text_ext(root_type, std::string{}); -} - rosbag2_storage::MessageDefinition LocalMessageDefinitionSource::get_full_text_ext( const std::string & root_type, const std::string & topic_name) diff --git a/rosbag2_py/rosbag2_py/_transport.pyi b/rosbag2_py/rosbag2_py/_transport.pyi index d744345ac6..49aa5a31b3 100644 --- a/rosbag2_py/rosbag2_py/_transport.pyi +++ b/rosbag2_py/rosbag2_py/_transport.pyi @@ -54,38 +54,16 @@ class PlayOptions: def __init__(self) -> None: ... class Player: - @overload - def __init__(self) -> None: ... - @overload - def __init__(self, log_level: str) -> None: ... @overload def __init__(self, storage_options: rosbag2_py._storage.StorageOptions, play_options: PlayOptions, log_level: str = ..., node_name: str = ...) -> None: ... @overload def __init__(self, storage_options: List[rosbag2_py._storage.StorageOptions], play_options: PlayOptions, log_level: str = ..., node_name: str = ...) -> None: ... - @overload def burst(self, num_messages: int) -> int: ... - @overload - def burst(self, constsize_t) -> Any: ... - @overload - def burst(self, storage_options: rosbag2_py._storage.StorageOptions, play_options: PlayOptions, num_messages: int) -> None: ... - @overload - def burst(self, num_messages) -> Any: ... - @staticmethod - def cancel() -> None: ... def get_playback_duration(self) -> int: ... def get_starting_time(self) -> int: ... def is_paused(self) -> bool: ... def pause(self) -> None: ... - @overload def play(self) -> None: ... - @overload - def play(self, storage_options: rosbag2_py._storage.StorageOptions, play_options: PlayOptions) -> None: ... - @overload - def play(self) -> Any: ... - @overload - def play(self, storage_options: List[rosbag2_py._storage.StorageOptions], play_options: PlayOptions) -> None: ... - @overload - def play(self) -> Any: ... def play_next(self) -> bool: ... def resume(self) -> None: ... def seek(self, time_point: int) -> None: ... @@ -123,7 +101,6 @@ class RecordOptions: regex: str repeat_all_transient_local_depth: int repeat_transient_local_messages: Dict[str, int] - rmw_serialization_format: str services: List[str] start_paused: bool static_topics_uri: str @@ -136,24 +113,10 @@ class RecordOptions: def __init__(self) -> None: ... class Recorder: - @overload - def __init__(self) -> None: ... - @overload - def __init__(self, arg: str) -> None: ... - @overload def __init__(self, storage_options: rosbag2_py._storage.StorageOptions, record_options: RecordOptions, log_level: str = ..., node_name: str = ...) -> None: ... - @staticmethod - def cancel() -> None: ... def is_paused(self) -> bool: ... def pause(self) -> None: ... - @overload def record(self) -> None: ... - @overload - def record(self) -> Any: ... - @overload - def record(self, storage_options: rosbag2_py._storage.StorageOptions, record_options: RecordOptions, node_name: str = ...) -> None: ... - @overload - def record(self) -> Any: ... def resume(self) -> None: ... def start_spin(self) -> None: ... def stop(self) -> None: ... diff --git a/rosbag2_py/src/rosbag2_py/_message_definitions.cpp b/rosbag2_py/src/rosbag2_py/_message_definitions.cpp index 8aba7f8bcb..99c73c41da 100644 --- a/rosbag2_py/src/rosbag2_py/_message_definitions.cpp +++ b/rosbag2_py/src/rosbag2_py/_message_definitions.cpp @@ -22,17 +22,6 @@ PYBIND11_MODULE(_message_definitions, m) { pybind11::class_( m, "LocalMessageDefinitionSource") .def(pybind11::init<>()) - .def( - "get_full_text", - [](rosbag2_cpp::LocalMessageDefinitionSource & self, const std::string & root_type) { - PyErr_WarnEx(PyExc_DeprecationWarning, - "get_full_text() is deprecated, use get_full_text_ext() instead.", 1); - return self.get_full_text_ext(root_type, ""); - }, - pybind11::arg("root_type"), - "The root type of the message definition. (Internally calls get_full_text_ext with an empty" - " topic name)" - ) .def( "get_full_text_ext", &rosbag2_cpp::LocalMessageDefinitionSource::get_full_text_ext); } diff --git a/rosbag2_py/src/rosbag2_py/_transport.cpp b/rosbag2_py/src/rosbag2_py/_transport.cpp index 51ac2763e4..a49e8c3bca 100644 --- a/rosbag2_py/src/rosbag2_py/_transport.cpp +++ b/rosbag2_py/src/rosbag2_py/_transport.cpp @@ -13,7 +13,6 @@ // limitations under the License. #include -#include #include #include #include @@ -163,19 +162,6 @@ namespace rosbag2_py class Player { public: - using SignalHandlerType = void (*)(int); - - // TODO(christophebedard): remove this constructor after a deprecation period - // along with the `if (!player_)` checks in the methods - explicit Player(const std::string & log_level = "info") - { - Arguments arguments({"--ros-args", "--log-level", log_level}); - rclcpp::init(arguments.argc(), arguments.argv()); - // Intentionally not initializing player_ here, because default constructor is intended to be - // used for the composable node only since it will call Player::Play() inside. Also it will - // fail without specifying storage options with valid path to the bag file via ros args. - } - Player( const rosbag2_storage::StorageOptions & storage_options, const PlayOptions & play_options, @@ -207,10 +193,6 @@ class Player void start_spin() { std::lock_guard lock(spin_thread_mutex_); - if (!player_) { - throw std::runtime_error("Player is not initialized. Please use constructor with " - "storage and play options."); - } if (exec_) { // We already have an executor spinning return; @@ -242,10 +224,6 @@ class Player void play() { - if (!player_) { - throw std::runtime_error("Player is not initialized. Please use constructor with " - "storage and play options."); - } player_->play(); } @@ -260,10 +238,6 @@ class Player bool wait_for_playback_to_start_exclusively(double timeout = -1.0) { - if (!player_) { - throw std::runtime_error("Player is not initialized. Please use constructor with " - "storage and play options."); - } return player_->wait_for_playback_to_start(std::chrono::duration(timeout)); } @@ -278,138 +252,57 @@ class Player bool wait_for_playback_to_finish_exclusively(double timeout = -1.0) { - if (!player_) { - throw std::runtime_error("Player is not initialized. Please use constructor with " - "storage and play options."); - } return player_->wait_for_playback_to_finish(std::chrono::duration(timeout)); } void stop() { - if (!player_) { - throw std::runtime_error("Player is not initialized. Please use constructor with " - "storage and play options."); - } player_->stop(); } void pause() { - if (!player_) { - throw std::runtime_error("Player is not initialized. Please use constructor with " - "storage and play options."); - } player_->pause(); } void resume() { - if (!player_) { - throw std::runtime_error("Player is not initialized. Please use constructor with " - "storage and play options."); - } player_->resume(); } [[nodiscard]] bool is_paused() const { - if (!player_) { - throw std::runtime_error("Player is not initialized. Please use constructor with " - "storage and play options."); - } return player_->is_paused(); } [[nodiscard]] int64_t get_starting_time() const { - if (!player_) { - throw std::runtime_error("Player is not initialized. Please use constructor with " - "storage and play options."); - } return player_->get_starting_time(); } [[nodiscard]] int64_t get_playback_duration() const { - if (!player_) { - throw std::runtime_error("Player is not initialized. Please use constructor with " - "storage and play options."); - } return player_->get_playback_duration(); } bool play_next() { - if (!player_) { - throw std::runtime_error("Player is not initialized. Please use constructor with " - "storage and play options."); - } return player_->play_next(); } size_t burst(size_t num_messages) { - if (!player_) { - throw std::runtime_error("Player is not initialized. Please use constructor with " - "storage and play options."); - } return player_->burst(num_messages); } void seek(int64_t time_point) { - if (!player_) { - throw std::runtime_error("Player is not initialized. Please use constructor with " - "storage and play options."); - } player_->seek(static_cast(time_point)); } - static void cancel() - { - PyErr_WarnEx(PyExc_DeprecationWarning, - "Player.cancel() is deprecated. Please use Player.stop() instead.", 1); - exit_ = true; - wait_for_exit_cv_.notify_all(); - } - - // TODO(christophebedard): remove this method after a deprecation period - void play( - const rosbag2_storage::StorageOptions & storage_options, - PlayOptions & play_options) - { - player_ = std::make_shared(storage_options, play_options); - play_impl(false); - } - - // TODO(christophebedard): remove this method after a deprecation period - void play( - const std::vector & storage_options, - PlayOptions & play_options) - { - player_ = std::make_shared(storage_options, play_options); - play_impl(false); - } - - // TODO(christophebedard): remove this method after a deprecation period - void burst( - const rosbag2_storage::StorageOptions & storage_options, - PlayOptions & play_options, - size_t num_messages) - { - player_ = std::make_shared(storage_options, play_options); - play_impl(true, num_messages); - } - protected: bool wait_for_async(double timeout, const std::function & wait_for_function) { - if (!player_) { - throw std::runtime_error("Player is not initialized. Please use constructor with " - "storage and play options."); - } - const double wait_period = (timeout < 0.0) ? SIGNAL_CHECK_INTERVAL : std::min(timeout, SIGNAL_CHECK_INTERVAL); @@ -448,99 +341,7 @@ class Player return finished; } - static void signal_handler(int sig_num) - { - if (sig_num == SIGINT || sig_num == SIGTERM) { - deferred_sig_number_ = sig_num; - rosbag2_py::Player::cancel(); - } - } - - static void install_signal_handlers() - { - deferred_sig_number_ = -1; - old_sigterm_handler_ = std::signal(SIGTERM, &rosbag2_py::Player::signal_handler); - old_sigint_handler_ = std::signal(SIGINT, &rosbag2_py::Player::signal_handler); - } - - static void uninstall_signal_handlers() - { - if (old_sigterm_handler_ != SIG_ERR) { - std::signal(SIGTERM, old_sigterm_handler_); - old_sigterm_handler_ = SIG_ERR; - } - if (old_sigint_handler_ != SIG_ERR) { - std::signal(SIGINT, old_sigint_handler_); - old_sigint_handler_ = SIG_ERR; - } - deferred_sig_number_ = -1; - } - - static void process_deferred_signal() - { - auto call_signal_handler = [](const SignalHandlerType & signal_handler, int sig_num) { - if (signal_handler != SIG_ERR && signal_handler != SIG_IGN && signal_handler != SIG_DFL) { - signal_handler(sig_num); - } - }; - - if (deferred_sig_number_ == SIGINT) { - call_signal_handler(old_sigint_handler_, deferred_sig_number_); - } else if (deferred_sig_number_ == SIGTERM) { - call_signal_handler(old_sigterm_handler_, deferred_sig_number_); - } - } - - void play_impl(bool burst = false, size_t burst_num_messages = 0) - { - if (!player_) { - throw std::runtime_error("Player is not initialized. Please use constructor with " - "storage and play options."); - } - install_signal_handlers(); - try { - exit_ = false; - start_spin(); - player_->play(); - - auto wait_for_exit_thread = std::thread( - [this]() { - std::unique_lock lock(wait_for_exit_mutex_); - wait_for_exit_cv_.wait(lock, [] {return rosbag2_py::Player::exit_.load();}); - player_->stop(); - }); - { - // Release the GIL for long-running play, so that calling Python code - // can use other threads - py::gil_scoped_release release; - if (burst) { - player_->burst(burst_num_messages); - } - player_->wait_for_playback_to_finish(); - } - - rosbag2_py::Player::cancel(); // Need to trigger exit from wait_for_exit_thread - if (wait_for_exit_thread.joinable()) { - wait_for_exit_thread.join(); - } - - stop_spin(); - } catch (...) { - process_deferred_signal(); - uninstall_signal_handlers(); - throw; - } - process_deferred_signal(); - uninstall_signal_handlers(); - } - - static std::atomic_bool exit_; - static std::condition_variable wait_for_exit_cv_; - static SignalHandlerType old_sigint_handler_; - static SignalHandlerType old_sigterm_handler_; - static int deferred_sig_number_; static constexpr double SIGNAL_CHECK_INTERVAL = 0.1; - std::mutex wait_for_exit_mutex_; std::shared_ptr player_; std::mutex spin_thread_mutex_; @@ -548,27 +349,9 @@ class Player std::thread spin_thread_; }; -Player::SignalHandlerType Player::old_sigint_handler_ {SIG_ERR}; -Player::SignalHandlerType Player::old_sigterm_handler_ {SIG_ERR}; -int Player::deferred_sig_number_{-1}; -std::atomic_bool Player::exit_{false}; -std::condition_variable Player::wait_for_exit_cv_{}; - class Recorder { public: - using SignalHandlerType = void (*)(int); - - // TODO(christophebedard): remove this constructor after a deprecation period - // along with the `if (!recorder_)` checks in the methods - explicit Recorder(const std::string & log_level = "info") - { - Arguments arguments({"--ros-args", "--log-level", log_level}); - rclcpp::init(arguments.argc(), arguments.argv()); - // Intentionally not initializing recorder_ here, because default constructor is intended to be - // used for the composable node only since it will call Recorder::record() inside. - } - Recorder( const rosbag2_storage::StorageOptions & storage_options, RecordOptions & record_options, @@ -580,16 +363,6 @@ class Recorder rclcpp::init(arguments.argc(), arguments.argv(), rclcpp::InitOptions(), rclcpp::SignalHandlerOptions::None); - if (!record_options.rmw_serialization_format.empty() && - record_options.output_serialization_format.empty()) - { - record_options.output_serialization_format = record_options.rmw_serialization_format; - PyErr_WarnEx(PyExc_DeprecationWarning, - "The rmw_serialization_format option is deprecated and will be removed in a " - "future release.\nPlease use output_serialization_format instead.", - 1 - ); - } if (record_options.output_serialization_format.empty()) { record_options.output_serialization_format = std::string(rmw_get_serialization_format()); } @@ -608,10 +381,6 @@ class Recorder void start_spin() { std::lock_guard lock(spin_thread_mutex_); - if (!recorder_) { - throw std::runtime_error("Recorder is not initialized. Please use constructor with " - "storage and record options."); - } if (exec_) { // We already have an executor spinning return; @@ -643,179 +412,36 @@ class Recorder void record() { - if (!recorder_) { - throw std::runtime_error("Recorder is not initialized. Please use constructor with " - "storage and record options."); - } recorder_->record(); } void stop() { - if (!recorder_) { - throw std::runtime_error("Recorder is not initialized. Please use constructor with " - "storage and record options."); - } recorder_->stop(); } void pause() { - if (!recorder_) { - throw std::runtime_error("Recorder is not initialized. Please use constructor with " - "storage and record options."); - } recorder_->pause(); } void resume() { - if (!recorder_) { - throw std::runtime_error("Recorder is not initialized. Please use constructor with " - "storage and record options."); - } recorder_->resume(); } bool is_paused() { - if (!recorder_) { - throw std::runtime_error("Recorder is not initialized. Please use constructor with " - "storage and record options."); - } return recorder_->is_paused(); } - // TODO(christophebedard): remove this method after a deprecation period - void record( - const rosbag2_storage::StorageOptions & storage_options, - RecordOptions & record_options, - const std::string & node_name) - { - if (!record_options.rmw_serialization_format.empty() && - record_options.output_serialization_format.empty()) - { - record_options.output_serialization_format = record_options.rmw_serialization_format; - PyErr_WarnEx(PyExc_DeprecationWarning, - "The rmw_serialization_format option is deprecated and will be removed in a " - "future release.\nPlease use output_serialization_format instead.", - 1 - ); - } - if (record_options.output_serialization_format.empty()) { - record_options.output_serialization_format = std::string(rmw_get_serialization_format()); - } - auto writer = rosbag2_transport::ReaderWriterFactory::make_writer(record_options); - - recorder_ = std::make_shared( - std::move(writer), storage_options, record_options, node_name); - - record_impl(); - } - - void record_impl() - { - if (!recorder_) { - throw std::runtime_error("Recorder is not initialized. Please use constructor with " - "storage and record options."); - } - install_signal_handlers(); - try { - exit_ = false; - recorder_->record(); - // Run exec->spin() in a separate thread, because we need to call exec->cancel() after - // recorder->stop() to be able to send notifications about bag split and close. - start_spin(); - { - // Release the GIL for long-running record, so that calling Python code - // can use other threads - py::gil_scoped_release release; - std::unique_lock lock(wait_for_exit_mutex_); - wait_for_exit_cv_.wait(lock, [] {return rosbag2_py::Recorder::exit_.load();}); - recorder_->stop(); - } - - stop_spin(); - } catch (...) { - process_deferred_signal(); - uninstall_signal_handlers(); - throw; - } - process_deferred_signal(); - uninstall_signal_handlers(); - } - - static void cancel() - { - PyErr_WarnEx(PyExc_DeprecationWarning, - "Recorder.cancel() is deprecated. Please use Recorder.stop() instead.", 1); - exit_ = true; - wait_for_exit_cv_.notify_all(); - } - protected: - static void signal_handler(int sig_num) - { - if (sig_num == SIGINT || sig_num == SIGTERM) { - deferred_sig_number_ = sig_num; - rosbag2_py::Recorder::cancel(); - } - } - - static void install_signal_handlers() - { - deferred_sig_number_ = -1; - old_sigterm_handler_ = std::signal(SIGTERM, &rosbag2_py::Recorder::signal_handler); - old_sigint_handler_ = std::signal(SIGINT, &rosbag2_py::Recorder::signal_handler); - } - - static void uninstall_signal_handlers() - { - if (old_sigterm_handler_ != SIG_ERR) { - std::signal(SIGTERM, old_sigterm_handler_); - old_sigterm_handler_ = SIG_ERR; - } - if (old_sigint_handler_ != SIG_ERR) { - std::signal(SIGINT, old_sigint_handler_); - old_sigint_handler_ = SIG_ERR; - } - deferred_sig_number_ = -1; - } - - static void process_deferred_signal() - { - auto call_signal_handler = [](const SignalHandlerType & signal_handler, int sig_num) { - if (signal_handler != SIG_ERR && signal_handler != SIG_IGN && signal_handler != SIG_DFL) { - signal_handler(sig_num); - } - }; - - if (deferred_sig_number_ == SIGINT) { - call_signal_handler(old_sigint_handler_, deferred_sig_number_); - } else if (deferred_sig_number_ == SIGTERM) { - call_signal_handler(old_sigterm_handler_, deferred_sig_number_); - } - } - - static std::atomic_bool exit_; - static std::condition_variable wait_for_exit_cv_; - static SignalHandlerType old_sigint_handler_; - static SignalHandlerType old_sigterm_handler_; - static int deferred_sig_number_; - std::mutex wait_for_exit_mutex_; - std::shared_ptr recorder_; std::mutex spin_thread_mutex_; std::unique_ptr exec_{nullptr}; std::thread spin_thread_; }; -Recorder::SignalHandlerType Recorder::old_sigint_handler_ {SIG_ERR}; -Recorder::SignalHandlerType Recorder::old_sigterm_handler_ {SIG_ERR}; -int Recorder::deferred_sig_number_{-1}; -std::atomic_bool Recorder::exit_{false}; -std::condition_variable Recorder::wait_for_exit_cv_{}; - // Return a RecordOptions struct with defaults set for rewriting bags. rosbag2_transport::RecordOptions bag_rewrite_default_record_options() { @@ -980,7 +606,6 @@ PYBIND11_MODULE(_transport, m) { .def_readwrite("static_topics_uri", &RecordOptions::static_topics_uri) .def_readwrite("topic_types", &RecordOptions::topic_types) .def_readwrite("exclude_topic_types", &RecordOptions::exclude_topic_types) - .def_readwrite("rmw_serialization_format", &RecordOptions::rmw_serialization_format) .def_readwrite("input_serialization_format", &RecordOptions::input_serialization_format) .def_readwrite("output_serialization_format", &RecordOptions::output_serialization_format) .def_readwrite("topic_polling_interval", &RecordOptions::topic_polling_interval) @@ -1016,24 +641,6 @@ PYBIND11_MODULE(_transport, m) { ; py::class_(m, "Player") - // TODO(christophebedard): remove this constructor after a deprecation period - // Deprecated default constructor - .def(py::init([]() - { - PyErr_WarnEx(PyExc_DeprecationWarning, "Player() is deprecated. Use the constructor with " - "full configuration parameters instead.", 1); - return new rosbag2_py::Player(); - }), "Deprecated: Use constructor with full options.") - - // TODO(christophebedard): remove this constructor after a deprecation period - // Deprecated constructor with log_level - .def(py::init([](const std::string & log_level) - { - PyErr_WarnEx(PyExc_DeprecationWarning, "Player(log_level) is deprecated. Use the " - "constructor with full configuration parameters instead.", 1); - return new rosbag2_py::Player(log_level); - }), py::arg("log_level"), "Deprecated: Use constructor with full options.") - // Recommended constructor with storage and play options .def(py::init(), @@ -1198,37 +805,7 @@ PYBIND11_MODULE(_transport, m) { time_point (int): Time point in ROS playback timeline, in nanoseconds. )pbdoc") - // TODO(christophebedard): remove this method after a deprecation period - // Deprecated play method with storage and play options - .def("play", - [](rosbag2_py::Player & self, const rosbag2_storage::StorageOptions & storage_options, - PlayOptions & play_options) - { - PyErr_WarnEx(PyExc_DeprecationWarning, "Player.play(storage_options, play_options) is " - "deprecated. Use the parameterless play() instead.", 1); - return self.play(storage_options, play_options); - }, - py::arg("storage_options"), - py::arg("play_options"), - "Deprecated: use play() with preconfigured options instead.") - - // TODO(christophebedard): remove this method after a deprecation period - // Deprecated play method with multiple storage options - .def("play", - [](rosbag2_py::Player & self, - const std::vector & storage_options, - PlayOptions & play_options) - { - PyErr_WarnEx(PyExc_DeprecationWarning, "Player.play(storage_options_list, play_options) is " - "deprecated. Use the parameterless play() instead.", 1); - return self.play(storage_options, play_options); - }, - py::arg("storage_options"), - py::arg("play_options"), - "Deprecated: use play() with preconfigured options instead.") - - // Recommended burst playback method - .def("burst", py::overload_cast(&rosbag2_py::Player::burst), py::arg("num_messages"), + .def("burst", &rosbag2_py::Player::burst, py::arg("num_messages"), R"pbdoc( Play a burst of messages. @@ -1239,51 +816,9 @@ PYBIND11_MODULE(_transport, m) { Returns: size_t: Number of messages played in this burst. )pbdoc") - - // TODO(christophebedard): remove this method after a deprecation period - // Deprecated burst method with storage and play options - .def("burst", - [](rosbag2_py::Player & self, const rosbag2_storage::StorageOptions & storage_options, - PlayOptions & play_options, size_t num_messages) - { - PyErr_WarnEx(PyExc_DeprecationWarning, - "Player.burst(storage_options, play_options, num_messages) is deprecated. " - "Use burst(num_messages) with preconfigured options instead.", 1); - return self.burst(storage_options, play_options, num_messages); - }, - py::arg("storage_options"), - py::arg("play_options"), - py::arg("num_messages"), - "Deprecated: use burst(num_messages) with preconfigured options instead.") - - .def_static("cancel", &rosbag2_py::Player::cancel, - R"pbdoc( - Cancel the ongoing playback session. - - This is a static method and will affect any running Players globally. - Deprecated: use Player.stop() instead. - )pbdoc") ; py::class_(m, "Recorder") - // TODO(christophebedard): remove this constructor after a deprecation period - // Deprecated default constructor - .def(py::init([]() - { - PyErr_WarnEx(PyExc_DeprecationWarning, "Recorder() is deprecated. Use the constructor with " - "full configuration parameters instead.", 1); - return new rosbag2_py::Recorder(); - }), "Deprecated: Use constructor with full options.") - - // TODO(christophebedard): remove this constructor after a deprecation period - // Deprecated constructor with string argument - .def(py::init([](const std::string & arg) - { - PyErr_WarnEx(PyExc_DeprecationWarning, "Recorder(log_level) is deprecated. Use the" - " constructor with full configuration parameters instead.", 1); - return new rosbag2_py::Recorder(arg); - }), py::arg("arg"), "Deprecated: Use constructor with full options.") - // Recommended constructor with storage and record options .def( py::init(&rosbag2_py::Recorder::record), + .def("record", &rosbag2_py::Recorder::record, R"pbdoc( Start recording based on the internal Recorder configuration. @@ -1342,29 +877,6 @@ PYBIND11_MODULE(_transport, m) { "is_paused", &rosbag2_py::Recorder::is_paused, "Whether the recording is currently paused.") - - // TODO(christophebedard): remove this method after a deprecation period - // (deprecated) record method - .def("record", - [](rosbag2_py::Recorder & self, const rosbag2_storage::StorageOptions & storage_options, - RecordOptions & record_options, const std::string & node_name) - { - PyErr_WarnEx(PyExc_DeprecationWarning, "Recorder.record(storage_options, record_options, " - "node_name) is deprecated. Use the parameterless record() instead.", 1); - return self.record(storage_options, record_options, node_name); - }, - py::arg("storage_options"), - py::arg("record_options"), - py::arg("node_name") = "rosbag2_recorder", - "Deprecated: use record() with preconfigured options instead.") - - .def_static("cancel", &rosbag2_py::Recorder::cancel, - R"pbdoc( - Cancel the ongoing recording session. - - This is a static method and will affect any running Recorders globally. - Deprecated: use Recorder.stop() instead. - )pbdoc") ; m.def( diff --git a/rosbag2_py/test/test_transport.py b/rosbag2_py/test/test_transport.py index 5314da2104..a52a1331bf 100644 --- a/rosbag2_py/test/test_transport.py +++ b/rosbag2_py/test/test_transport.py @@ -15,7 +15,6 @@ import datetime import os from pathlib import Path -import re import signal import threading @@ -32,7 +31,6 @@ RESOURCES_PATH = Path(os.environ['ROSBAG2_PY_TEST_RESOURCES_DIR']) -PLAYBACK_UNTIL_TIMESTAMP_REGEX_STRING = r'\[rosbag2_player]: Playback until timestamp: -1' def test_options_qos_conversion(): @@ -248,142 +246,6 @@ def test_recorder_api(tmp_path, storage_id): assert '/test_recorder_api_node/topic' in bag_topics, str(bag_topics) -def test_player_unconfigured(): - # Test that using a constructor without full configuration raises an error when trying to use - # the player - player = rosbag2_py.Player() - with pytest.raises(RuntimeError): - player.start_spin() - with pytest.raises(RuntimeError): - player.play() - with pytest.raises(RuntimeError): - player.wait_for_playback_to_start() - with pytest.raises(RuntimeError): - player.wait_for_playback_to_finish() - with pytest.raises(RuntimeError): - player.stop() - with pytest.raises(RuntimeError): - player.pause() - with pytest.raises(RuntimeError): - player.resume() - with pytest.raises(RuntimeError): - player.is_paused() - with pytest.raises(RuntimeError): - player.play_next() - with pytest.raises(RuntimeError): - player.burst(1) - with pytest.raises(RuntimeError): - player.seek(0) - - -def test_recorder_unconfigured(): - # Test that using a constructor without full configuration raises an error when trying to use - # the recorder - recorder = rosbag2_py.Recorder() - with pytest.raises(RuntimeError): - recorder.start_spin() - with pytest.raises(RuntimeError): - recorder.record() - with pytest.raises(RuntimeError): - recorder.stop() - with pytest.raises(RuntimeError): - recorder.pause() - with pytest.raises(RuntimeError): - recorder.resume() - with pytest.raises(RuntimeError): - recorder.is_paused() - - -@pytest.mark.parametrize('storage_id', TESTED_STORAGE_IDS) -def test_record_cancel(tmp_path, storage_id): - bag_path = tmp_path / 'test_record_cancel' - storage_options, converter_options = get_rosbag_options(str(bag_path), storage_id) - - record_options = rosbag2_py.RecordOptions() - record_options.all_topics = True - record_options.is_discovery_disabled = False - record_options.topic_polling_interval = datetime.timedelta(milliseconds=100) - - recorder = rosbag2_py.Recorder() - - ctx = rclpy.Context() - ctx.init() - record_thread = threading.Thread( - target=recorder.record, - args=(storage_options, record_options), - daemon=True) - record_thread.start() - - node = rclpy.create_node('test_record_cancel', context=ctx) - executor = rclpy.executors.SingleThreadedExecutor(context=ctx) - executor.add_node(node) - pub = node.create_publisher(String, 'chatter', 10) - - i = 0 - msg = String() - - while rclpy.ok() and i < 10: - msg.data = 'Hello World: {0}'.format(i) - i += 1 - pub.publish(msg) - - recorder.cancel() - - metadata_io = rosbag2_py.MetadataIo() - assert wait_for(lambda: metadata_io.metadata_file_exists(str(bag_path)), - timeout=rclpy.duration.Duration(seconds=3)) - record_thread.join() - - metadata = metadata_io.read_metadata(str(bag_path)) - assert len(metadata.relative_file_paths) - storage_path = bag_path / metadata.relative_file_paths[0] - assert wait_for(lambda: storage_path.is_file(), - timeout=rclpy.duration.Duration(seconds=3)) - - -@pytest.mark.parametrize('storage_id', TESTED_STORAGE_IDS) -def test_play_cancel(storage_id, capfd): - bag_path = str(RESOURCES_PATH / storage_id / 'talker') - assert os.path.exists(bag_path), 'Could not find test bag file: ' + bag_path - - storage_options, converter_options = get_rosbag_options(bag_path, storage_id) - - play_options = rosbag2_py.PlayOptions() - play_options.loop = True - play_options.start_paused = True - - player = rosbag2_py.Player(storage_options, play_options) - - player_thread = threading.Thread( - target=player.play, - args=(storage_options, play_options), - daemon=True) - player_thread.start() - - def check_playback_start_output(cap_streams): - out, err = capfd.readouterr() - cap_streams['err'] += err - cap_streams['out'] += out - expected_string_regex = re.compile(PLAYBACK_UNTIL_TIMESTAMP_REGEX_STRING) - matches = expected_string_regex.search(cap_streams['err']) - return matches is not None - - captured_streams = {'out': '', 'err': ''} - - if not wait_for(lambda: check_playback_start_output(captured_streams), - timeout=rclpy.duration.Duration(seconds=5)): - with capfd.disabled(): - print('\nCaptured stdout:', captured_streams['out']) - print('\nCaptured stderr:', captured_streams['err']) - player.cancel() - player_thread.join() - assert False - - player.cancel() - player_thread.join(3) - assert not player_thread.is_alive() - - @pytest.mark.parametrize('storage_id', TESTED_STORAGE_IDS) def test_play_process_sigint_in_python_handler(storage_id): bag_path = str(RESOURCES_PATH / storage_id / 'talker') diff --git a/rosbag2_storage/include/rosbag2_storage/storage_interfaces/base_write_interface.hpp b/rosbag2_storage/include/rosbag2_storage/storage_interfaces/base_write_interface.hpp index d6ecdfd4c3..186864a056 100644 --- a/rosbag2_storage/include/rosbag2_storage/storage_interfaces/base_write_interface.hpp +++ b/rosbag2_storage/include/rosbag2_storage/storage_interfaces/base_write_interface.hpp @@ -40,17 +40,6 @@ class ROSBAG2_STORAGE_PUBLIC BaseWriteInterface /// \brief Default destructor. virtual ~BaseWriteInterface() = default; - /// \brief Writes one serialized message to the storage. - /// \throws std::runtime_error if the storage is not open, or if create_topic(..) was not called - /// previously for the topic associated with the message being written. - [[deprecated("Use write_message(std::shared_ptr msg) instead.")]] - virtual void write(std::shared_ptr msg) = 0; - - // This method is deprecated, use - // std::vector write_messages(const SerializedBagMessages & messages) instead. - [[deprecated("Use write_messages(const SerializedBagMessages & messages) instead.")]] - virtual void write(const std::vector> & msg) = 0; - /// \brief Writes one serialized message to the storage. /// \param msg - The serialized message to write. /// \return Returns true if the message was written successfully, false otherwise. diff --git a/rosbag2_storage/test/rosbag2_storage/test_plugin.cpp b/rosbag2_storage/test/rosbag2_storage/test_plugin.cpp index 96275ebea1..d5f090afe3 100644 --- a/rosbag2_storage/test/rosbag2_storage/test_plugin.cpp +++ b/rosbag2_storage/test/rosbag2_storage/test_plugin.cpp @@ -82,19 +82,6 @@ void TestPlugin::remove_topic(const rosbag2_storage::TopicMetadata & topic) std::cout << "Removed topic with name =" << topic.name << " and type =" << topic.type << ".\n"; } -void TestPlugin::write(const std::shared_ptr msg) -{ - (void) msg; - std::cout << "\nwriting\n"; -} - -void TestPlugin::write( - const std::vector> & msg) -{ - (void) msg; - std::cout << "\nwriting multiple\n"; -} - bool TestPlugin::write_message( const std::shared_ptr msg) { diff --git a/rosbag2_storage/test/rosbag2_storage/test_plugin.hpp b/rosbag2_storage/test/rosbag2_storage/test_plugin.hpp index 4ffab612b2..badf64ed90 100644 --- a/rosbag2_storage/test/rosbag2_storage/test_plugin.hpp +++ b/rosbag2_storage/test/rosbag2_storage/test_plugin.hpp @@ -47,11 +47,6 @@ class TestPlugin : public rosbag2_storage::storage_interfaces::ReadWriteInterfac std::shared_ptr read_next() override; - void write(std::shared_ptr msg) override; - - void write( - const std::vector> & msg) override; - bool write_message(std::shared_ptr msg) override; std::vector diff --git a/rosbag2_storage_mcap/src/mcap_storage.cpp b/rosbag2_storage_mcap/src/mcap_storage.cpp index 1b6ee569fc..2e9c810d47 100644 --- a/rosbag2_storage_mcap/src/mcap_storage.cpp +++ b/rosbag2_storage_mcap/src/mcap_storage.cpp @@ -201,9 +201,6 @@ class ROSBAG2_STORAGE_MCAP_PUBLIC MCAPStorage uint64_t get_minimum_split_file_size() const override; /** BaseWriteInterface **/ - void write(std::shared_ptr msg) override; - void write( - const std::vector> & msg) override; bool write_message(std::shared_ptr msg) override; std::vector write_messages( const rosbag2_storage::SerializedBagMessages & messages) override; @@ -846,28 +843,6 @@ uint64_t MCAPStorage::get_minimum_split_file_size() const } /** BaseWriteInterface **/ -void MCAPStorage::write(std::shared_ptr msg) -{ - if (!write_message(msg)) { - throw std::runtime_error{std::string{"Message on topic '"} + msg->topic_name + "' of size '" + - std::to_string(msg->serialized_data->buffer_length) + - "' bytes failed to write to MCAP file. It will be lost."}; - } -} - -void MCAPStorage::write( - const std::vector> & msgs) -{ - std::lock_guard lock(mcap_storage_mutex_); - for (const auto & msg : msgs) { - if (!write_lock_free(msg)) { - throw std::runtime_error{std::string{"Message on topic '"} + msg->topic_name + "' of size '" + - std::to_string(msg->serialized_data->buffer_length) + - "' bytes failed to write to MCAP file. It will be lost."}; - } - } -} - bool MCAPStorage::write_message(std::shared_ptr msg) { std::lock_guard lock(mcap_storage_mutex_); diff --git a/rosbag2_storage_sqlite3/include/rosbag2_storage_sqlite3/sqlite_storage.hpp b/rosbag2_storage_sqlite3/include/rosbag2_storage_sqlite3/sqlite_storage.hpp index 5e3440b4ad..31a7ea8d9d 100644 --- a/rosbag2_storage_sqlite3/include/rosbag2_storage_sqlite3/sqlite_storage.hpp +++ b/rosbag2_storage_sqlite3/include/rosbag2_storage_sqlite3/sqlite_storage.hpp @@ -63,12 +63,6 @@ class ROSBAG2_STORAGE_DEFAULT_PLUGINS_PUBLIC SqliteStorage const rosbag2_storage::TopicMetadata & topic, const rosbag2_storage::MessageDefinition & message_definition) override; - void write(std::shared_ptr message) override; - - void write( - const std::vector> & messages) - override; - bool write_message(std::shared_ptr message) override; std::vector diff --git a/rosbag2_storage_sqlite3/src/rosbag2_storage_sqlite3/sqlite_storage.cpp b/rosbag2_storage_sqlite3/src/rosbag2_storage_sqlite3/sqlite_storage.cpp index 18e3d525d2..4aaa7f6444 100644 --- a/rosbag2_storage_sqlite3/src/rosbag2_storage_sqlite3/sqlite_storage.cpp +++ b/rosbag2_storage_sqlite3/src/rosbag2_storage_sqlite3/sqlite_storage.cpp @@ -280,11 +280,6 @@ void SqliteStorage::commit_transaction() active_transaction_ = false; } -void SqliteStorage::write(std::shared_ptr message) -{ - (void)write_message(message); -} - bool SqliteStorage::write_message(std::shared_ptr message) { @@ -335,12 +330,6 @@ bool SqliteStorage::write_locked( return true; } -void SqliteStorage::write( - const std::vector> & messages) -{ - (void)write_messages(messages); -} - std::vector SqliteStorage::write_messages(const rosbag2_storage::SerializedBagMessages & messages) { diff --git a/rosbag2_tests/test/rosbag2_tests/test_rosbag2_cpp_get_service_info.cpp b/rosbag2_tests/test/rosbag2_tests/test_rosbag2_cpp_get_service_info.cpp index 48cb070042..9a4f2a89da 100644 --- a/rosbag2_tests/test/rosbag2_tests/test_rosbag2_cpp_get_service_info.cpp +++ b/rosbag2_tests/test/rosbag2_tests/test_rosbag2_cpp_get_service_info.cpp @@ -210,7 +210,7 @@ TEST_P(Rosbag2CPPGetServiceInfoTest, get_service_info_for_bag_with_services_only storage_options.storage_id = storage_id; storage_options.uri = bag_path_str; rosbag2_transport::RecordOptions record_options = - {false, true, false, false, {}, {}, {}, {}, {"/rosout"}, {}, {}, {}, {}, {}, "cdr", 100ms}; + {false, true, false, false, {}, {}, {}, {}, {"/rosout"}, {}, {}, {}, {}, "cdr", 100ms}; auto recorder = std::make_shared( std::move(writer), storage_options, record_options); recorder->record(); @@ -296,7 +296,7 @@ TEST_P(Rosbag2CPPGetServiceInfoTest, get_service_info_for_bag_with_topics_and_se storage_options.storage_id = storage_id; storage_options.uri = bag_path_str; rosbag2_transport::RecordOptions record_options = - {true, true, false, false, {}, {}, {}, {}, {"/rosout"}, {}, {}, {}, {}, {}, "cdr", 100ms}; + {true, true, false, false, {}, {}, {}, {}, {"/rosout"}, {}, {}, {}, {}, "cdr", 100ms}; auto recorder = std::make_shared( std::move(writer), storage_options, record_options); recorder->record(); diff --git a/rosbag2_transport/include/rosbag2_transport/player.hpp b/rosbag2_transport/include/rosbag2_transport/player.hpp index 9c0a445169..a24c30e8e9 100644 --- a/rosbag2_transport/include/rosbag2_transport/player.hpp +++ b/rosbag2_transport/include/rosbag2_transport/player.hpp @@ -379,13 +379,6 @@ class Player : public rclcpp::Node ROSBAG2_TRANSPORT_PUBLIC size_t get_number_of_registered_on_play_msg_post_callbacks(); - /// \brief Getter for the first of the currently stored storage options - /// \return Copy of the first item in the StorageOptions vector - // TODO(morlov): Remove this method in Rolling after Lyrical release - [[deprecated("Use rosabg2_transport::Player::get_all_storage_options() instead")]] - ROSBAG2_TRANSPORT_PUBLIC - rosbag2_storage::StorageOptions get_storage_options(); - /// \brief Getter for the currently stored storage options /// \return Copy of the currently stored storage options ROSBAG2_TRANSPORT_PUBLIC diff --git a/rosbag2_transport/include/rosbag2_transport/record_options.hpp b/rosbag2_transport/include/rosbag2_transport/record_options.hpp index 7ee770fe29..faa924941a 100644 --- a/rosbag2_transport/include/rosbag2_transport/record_options.hpp +++ b/rosbag2_transport/include/rosbag2_transport/record_options.hpp @@ -42,8 +42,6 @@ struct RecordOptions std::vector exclude_topic_types; std::vector exclude_service_events; // service event topics list std::vector exclude_actions; // actions name list - // rmw_serialization_format deprecated. Use output_serialization_format instead - std::string rmw_serialization_format; std::string input_serialization_format; std::string output_serialization_format; std::chrono::milliseconds topic_polling_interval{100}; diff --git a/rosbag2_transport/src/rosbag2_transport/bag_rewrite.cpp b/rosbag2_transport/src/rosbag2_transport/bag_rewrite.cpp index 2ff5c89d3e..c4cf3e04c6 100644 --- a/rosbag2_transport/src/rosbag2_transport/bag_rewrite.cpp +++ b/rosbag2_transport/src/rosbag2_transport/bag_rewrite.cpp @@ -131,10 +131,6 @@ setup_topic_filtering( auto filtered_topics_and_types = topic_filter.filter_topics(input_topics); std::string output_serialization_format = record_options.output_serialization_format; - // Fall back to the deprecated rmw_serialization_format if output format is unspecified - if (!record_options.rmw_serialization_format.empty() && output_serialization_format.empty()) { - output_serialization_format = record_options.rmw_serialization_format; - } // Done filtering - set up writer for (const auto & [topic_name, topic_type] : filtered_topics_and_types) { diff --git a/rosbag2_transport/src/rosbag2_transport/config_options_from_node_params.cpp b/rosbag2_transport/src/rosbag2_transport/config_options_from_node_params.cpp index c8c6a18b81..1bc914455d 100644 --- a/rosbag2_transport/src/rosbag2_transport/config_options_from_node_params.cpp +++ b/rosbag2_transport/src/rosbag2_transport/config_options_from_node_params.cpp @@ -327,9 +327,6 @@ RecordOptions get_record_options_from_node_params(rclcpp::Node & node) record_options.exclude_actions = node.declare_parameter>( "record.exclude_actions", std::vector()); - record_options.rmw_serialization_format = - node.declare_parameter("record.rmw_serialization_format", "cdr"); - record_options.input_serialization_format = node.declare_parameter("record.input_serialization_format", "cdr"); record_options.output_serialization_format = diff --git a/rosbag2_transport/src/rosbag2_transport/player.cpp b/rosbag2_transport/src/rosbag2_transport/player.cpp index b15ca4dd78..1dd777914a 100644 --- a/rosbag2_transport/src/rosbag2_transport/player.cpp +++ b/rosbag2_transport/src/rosbag2_transport/player.cpp @@ -240,10 +240,6 @@ class PlayerImpl /// \return Number of registered on_play_msg_post_callbacks size_t get_number_of_registered_on_play_msg_post_callbacks(); - /// \brief Getter for the first of the currently stored storage options - /// \return Copy of the first of the currently stored storage options - rosbag2_storage::StorageOptions get_storage_options(); - /// \brief Getter for the currently stored storage options /// \return Copy of the currently stored storage options std::vector get_all_storage_options(); @@ -2272,15 +2268,6 @@ void PlayerImpl::publish_clock_update(const rclcpp::Time & time) } } -rosbag2_storage::StorageOptions PlayerImpl::get_storage_options() -{ - auto all_storage_options = get_all_storage_options(); - if (all_storage_options.empty()) { - throw std::runtime_error("Storage options not available."); - } - return all_storage_options[0]; -} - std::vector PlayerImpl::get_all_storage_options() { return readers_->get_all_storage_options(); @@ -2541,11 +2528,6 @@ size_t Player::get_number_of_registered_on_play_msg_post_callbacks() return pimpl_->get_number_of_registered_on_play_msg_post_callbacks(); } -rosbag2_storage::StorageOptions Player::get_storage_options() -{ - return pimpl_->get_storage_options(); -} - std::vector Player::get_all_storage_options() { return pimpl_->get_all_storage_options(); diff --git a/rosbag2_transport/src/rosbag2_transport/record_options.cpp b/rosbag2_transport/src/rosbag2_transport/record_options.cpp index 8cea5dc65c..76c4d046a8 100644 --- a/rosbag2_transport/src/rosbag2_transport/record_options.cpp +++ b/rosbag2_transport/src/rosbag2_transport/record_options.cpp @@ -33,7 +33,7 @@ Node convert::encode( auto & [all_topics, all_services, all_actions, is_discovery_disabled, topics, topic_types, services, actions, exclude_topics, exclude_topic_types, exclude_service_events, exclude_actions, - rmw_serialization_format, input_serialization_format, output_serialization_format, + input_serialization_format, output_serialization_format, topic_polling_interval, regex, exclude_regex, node_prefix, compression_mode, compression_format, compression_queue_size, compression_threads, compression_threads_priority, topic_qos_profile_overrides, @@ -54,7 +54,6 @@ Node convert::encode( node["exclude_topic_types"] = exclude_topic_types; node["exclude_services"] = exclude_service_events; node["exclude_actions"] = exclude_actions; - node["rmw_serialization_format"] = rmw_serialization_format; node["input_serialization_format"] = input_serialization_format; node["output_serialization_format"] = output_serialization_format; node["topic_polling_interval"] = topic_polling_interval; @@ -91,7 +90,7 @@ bool convert::decode( auto & [all_topics, all_services, all_actions, is_discovery_disabled, topics, topic_types, services, actions, exclude_topics, exclude_topic_types, exclude_service_events, exclude_actions, - rmw_serialization_format, input_serialization_format, output_serialization_format, + input_serialization_format, output_serialization_format, topic_polling_interval, regex, exclude_regex, node_prefix, compression_mode, compression_format, compression_queue_size, compression_threads, compression_threads_priority, topic_qos_profile_overrides, @@ -120,8 +119,6 @@ bool convert::decode( optional_assign>(node, "exclude_topic_types", exclude_topic_types); optional_assign>(node, "exclude_services", exclude_service_events); optional_assign>(node, "exclude_actions", exclude_actions); - optional_assign( - node, "rmw_serialization_format", rmw_serialization_format); optional_assign( node, "input_serialization_format", input_serialization_format); optional_assign( diff --git a/rosbag2_transport/src/rosbag2_transport/recorder.cpp b/rosbag2_transport/src/rosbag2_transport/recorder.cpp index 982bd3e9f4..e9aea107b1 100644 --- a/rosbag2_transport/src/rosbag2_transport/recorder.cpp +++ b/rosbag2_transport/src/rosbag2_transport/recorder.cpp @@ -687,14 +687,6 @@ bool RecorderImpl::record(const std::string & uri) RCLCPP_INFO(node->get_logger(), "Starting recording to '%s'", storage_options_.uri.c_str()); // Check serialization format options - if (!record_options_.rmw_serialization_format.empty() && - record_options_.output_serialization_format.empty()) - { - RCLCPP_WARN(node->get_logger(), - "The rmw_serialization_format option is deprecated and will be removed in a future release.\n" - "Please use output_serialization_format instead."); - record_options_.output_serialization_format = record_options_.rmw_serialization_format; - } if (record_options_.input_serialization_format.empty()) { record_options_.input_serialization_format = rmw_get_serialization_format(); RCLCPP_WARN(node->get_logger(), diff --git a/rosbag2_transport/test/rosbag2_transport/test_composable_recorder.cpp b/rosbag2_transport/test/rosbag2_transport/test_composable_recorder.cpp index 65ed626252..2012850b3e 100644 --- a/rosbag2_transport/test/rosbag2_transport/test_composable_recorder.cpp +++ b/rosbag2_transport/test/rosbag2_transport/test_composable_recorder.cpp @@ -225,7 +225,6 @@ TEST_P(ComposableRecorderTests, recorder_can_parse_parameters_from_file) { EXPECT_EQ(record_options.exclude_topic_types, exclude_topic_types); std::vector services {"/service/_service_event", "/other_service/_service_event"}; EXPECT_EQ(record_options.services, services); - EXPECT_EQ(record_options.rmw_serialization_format, "cdr"); EXPECT_EQ(record_options.input_serialization_format, "cdr"); EXPECT_EQ(record_options.output_serialization_format, "cdr"); EXPECT_TRUE(record_options.topic_polling_interval == 0.01s); diff --git a/rosbag2_transport/test/rosbag2_transport/test_keyboard_controls.cpp b/rosbag2_transport/test/rosbag2_transport/test_keyboard_controls.cpp index 942da51eda..7644e9edad 100644 --- a/rosbag2_transport/test/rosbag2_transport/test_keyboard_controls.cpp +++ b/rosbag2_transport/test/rosbag2_transport/test_keyboard_controls.cpp @@ -194,7 +194,7 @@ TEST_F(RecordIntegrationTestFixture, test_keyboard_controls) auto keyboard_handler = std::make_shared(); rosbag2_transport::RecordOptions record_options = - {true, false, false, false, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, "rmw_format", 100ms}; + {true, false, false, false, {}, {}, {}, {}, {}, {}, {}, {}, {}, "rmw_format", 100ms}; record_options.start_paused = true; auto recorder = std::make_shared( diff --git a/rosbag2_transport/test/rosbag2_transport/test_record.cpp b/rosbag2_transport/test/rosbag2_transport/test_record.cpp index b5b5422294..aba8d52d88 100644 --- a/rosbag2_transport/test/rosbag2_transport/test_record.cpp +++ b/rosbag2_transport/test/rosbag2_transport/test_record.cpp @@ -58,7 +58,7 @@ TEST_F(RecordIntegrationTestFixture, published_messages_from_multiple_topics_are rosbag2_transport::RecordOptions record_options = {false, false, false, false, {string_topic, array_topic}, - {}, {}, {}, {}, {}, {}, {}, {}, "rmw_format", "rmw_format", 50ms}; + {}, {}, {}, {}, {}, {}, {}, "rmw_format", "rmw_format", 50ms}; auto recorder = std::make_shared( std::move(writer_), storage_options_, record_options); recorder->record(); @@ -177,7 +177,7 @@ TEST_F(RecordIntegrationTestFixture, can_record_again_after_stop) rosbag2_transport::RecordOptions record_options = { - false, false, false, false, {test_topic}, {}, {}, {}, {}, {}, {}, {}, {}, "rmw_format", + false, false, false, false, {test_topic}, {}, {}, {}, {}, {}, {}, {}, "rmw_format", "rmw_format", 50ms }; auto recorder = std::make_shared( @@ -288,7 +288,7 @@ TEST_F(RecordIntegrationTestFixture, qos_is_stored_in_metadata) pub_manager.setup_publisher(topic, string_message, 2); rosbag2_transport::RecordOptions record_options = - {false, false, false, false, {topic}, {}, {}, {}, {}, {}, {}, {}, {}, {}, "rmw_format", 100ms}; + {false, false, false, false, {topic}, {}, {}, {}, {}, {}, {}, {}, {}, "rmw_format", 100ms}; auto recorder = std::make_shared( std::move(writer_), storage_options_, record_options); recorder->record(); @@ -353,7 +353,7 @@ TEST_F(RecordIntegrationTestFixture, records_sensor_data) pub_manager.setup_publisher(topic, string_message, 2, rclcpp::SensorDataQoS()); rosbag2_transport::RecordOptions record_options = - {false, false, false, false, {topic}, {}, {}, {}, {}, {}, {}, {}, {}, {}, "rmw_format", 100ms}; + {false, false, false, false, {topic}, {}, {}, {}, {}, {}, {}, {}, {}, "rmw_format", 100ms}; auto recorder = std::make_shared( std::move(writer_), storage_options_, record_options); recorder->record(); @@ -397,7 +397,7 @@ TEST_F(RecordIntegrationTestFixture, receives_latched_messages) pub_manager.run_publishers(); rosbag2_transport::RecordOptions record_options = - {false, false, false, false, {topic}, {}, {}, {}, {}, {}, {}, {}, {}, {}, "rmw_format", 100ms}; + {false, false, false, false, {topic}, {}, {}, {}, {}, {}, {}, {}, {}, "rmw_format", 100ms}; auto recorder = std::make_shared( std::move(writer_), storage_options_, record_options); recorder->record(); @@ -435,7 +435,6 @@ TEST_F(RecordIntegrationTestFixture, repeat_transient_local_topics_register_requ rosbag2_transport::RecordOptions record_options; record_options.topics = {topic}; - record_options.rmw_serialization_format = "rmw_format"; record_options.topic_polling_interval = 100ms; record_options.repeat_transient_local_messages[topic] = 3; @@ -477,7 +476,6 @@ TEST_F(RecordIntegrationTestFixture, repeat_all_transient_local_auto_detects_mix rosbag2_transport::RecordOptions record_options; record_options.topics = {topic}; - record_options.rmw_serialization_format = "rmw_format"; record_options.topic_polling_interval = 100ms; record_options.repeat_all_transient_local_depth = expected_depth; @@ -515,7 +513,6 @@ TEST_F(RecordIntegrationTestFixture, repeat_all_transient_local_skips_volatile_o rosbag2_transport::RecordOptions record_options; record_options.topics = {topic}; - record_options.rmw_serialization_format = "rmw_format"; record_options.topic_polling_interval = 100ms; record_options.repeat_all_transient_local_depth = 5; @@ -561,7 +558,7 @@ TEST_F(RecordIntegrationTestFixture, mixed_qos_subscribes) { topic, profile_transient_local); rosbag2_transport::RecordOptions record_options = - {false, false, false, false, {topic}, {}, {}, {}, {}, {}, {}, {}, {}, {}, "rmw_format", 100ms}; + {false, false, false, false, {topic}, {}, {}, {}, {}, {}, {}, {}, {}, "rmw_format", 100ms}; auto recorder = std::make_shared( std::move(writer_), storage_options_, record_options); recorder->record(); @@ -610,7 +607,7 @@ TEST_F(RecordIntegrationTestFixture, duration_and_noncompatibility_policies_mixe auto publisher_liveliness = create_pub(profile_liveliness); rosbag2_transport::RecordOptions record_options = - {false, false, false, false, {topic}, {}, {}, {}, {}, {}, {}, {}, {}, {}, "rmw_format", 100ms}; + {false, false, false, false, {topic}, {}, {}, {}, {}, {}, {}, {}, {}, "rmw_format", 100ms}; auto recorder = std::make_shared( std::move(writer_), storage_options_, record_options); recorder->record(); @@ -653,7 +650,7 @@ TEST_F(RecordIntegrationTestFixture, write_split_callback_is_called) rosbag2_transport::RecordOptions record_options = { - false, false, false, false, {string_topic}, {}, {}, {}, {}, {}, {}, {}, {}, {}, "rmw_format", + false, false, false, false, {string_topic}, {}, {}, {}, {}, {}, {}, {}, {}, "rmw_format", 10ms }; auto recorder = std::make_shared( diff --git a/rosbag2_transport/test/rosbag2_transport/test_record_all.cpp b/rosbag2_transport/test/rosbag2_transport/test_record_all.cpp index 3a830ee85d..b83d617c5c 100644 --- a/rosbag2_transport/test/rosbag2_transport/test_record_all.cpp +++ b/rosbag2_transport/test/rosbag2_transport/test_record_all.cpp @@ -62,7 +62,7 @@ TEST_F(RecordIntegrationTestFixture, published_messages_from_multiple_topics_are RecorderEventNotifier::get_default_write_split_topic_name(), RecorderEventNotifier::get_default_messages_lost_topic_name(), }, - {}, {}, {}, {}, {}, "rmw_format", 100ms + {}, {}, {}, {}, "rmw_format", 100ms }; auto recorder = std::make_shared( std::move(writer_), storage_options_, record_options); @@ -111,7 +111,7 @@ TEST_F(RecordIntegrationTestFixture, published_messages_from_multiple_services_a "test_service_2"); rosbag2_transport::RecordOptions record_options = - {false, true, false, false, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, "rmw_format", 100ms}; + {false, true, false, false, {}, {}, {}, {}, {}, {}, {}, {}, {}, "rmw_format", 100ms}; auto recorder = std::make_shared( std::move(writer_), storage_options_, record_options); recorder->record(); @@ -159,7 +159,7 @@ TEST_F(RecordIntegrationTestFixture, published_messages_from_multiple_actions_ar "test_action_2"); rosbag2_transport::RecordOptions record_options = - {false, false, true, false, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, "rmw_format", 100ms}; + {false, false, true, false, {}, {}, {}, {}, {}, {}, {}, {}, {}, "rmw_format", 100ms}; auto recorder = std::make_shared( std::move(writer_), storage_options_, record_options); recorder->record(); @@ -229,7 +229,7 @@ TEST_F(RecordIntegrationTestFixture, published_messages_from_topic_service_actio RecorderEventNotifier::get_default_write_split_topic_name(), RecorderEventNotifier::get_default_messages_lost_topic_name(), }, - {}, {}, {}, {}, {}, "rmw_format", 100ms + {}, {}, {}, {}, "rmw_format", 100ms }; auto recorder = std::make_shared( std::move(writer_), storage_options_, record_options); @@ -292,7 +292,7 @@ TEST_F(RecordIntegrationTestFixture, cancel_event_messages_from_action_are_recor RecorderEventNotifier::get_default_write_split_topic_name(), RecorderEventNotifier::get_default_messages_lost_topic_name(), }, - {}, {}, {}, {}, {}, "rmw_format", 100ms + {}, {}, {}, {}, "rmw_format", 100ms }; auto recorder = std::make_shared( std::move(writer_), storage_options_, record_options); diff --git a/rosbag2_transport/test/rosbag2_transport/test_record_all_ignore_leaf_topics.cpp b/rosbag2_transport/test/rosbag2_transport/test_record_all_ignore_leaf_topics.cpp index e7bc10af8c..cd7390d5a6 100644 --- a/rosbag2_transport/test/rosbag2_transport/test_record_all_ignore_leaf_topics.cpp +++ b/rosbag2_transport/test/rosbag2_transport/test_record_all_ignore_leaf_topics.cpp @@ -52,7 +52,7 @@ TEST_F(RecordIntegrationTestFixture, published_messages_from_two_topics_ignore_l pub_manager.setup_publisher(string_topic, string_message, 2); rosbag2_transport::RecordOptions record_options = - {true, false, false, false, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, "rmw_format", 100ms}; + {true, false, false, false, {}, {}, {}, {}, {}, {}, {}, {}, {}, "rmw_format", 100ms}; record_options.ignore_leaf_topics = true; auto recorder = std::make_shared( std::move(writer_), storage_options_, record_options); diff --git a/rosbag2_transport/test/rosbag2_transport/test_record_all_include_unpublished_topics.cpp b/rosbag2_transport/test/rosbag2_transport/test_record_all_include_unpublished_topics.cpp index b6fd44dc85..69b3c3f635 100644 --- a/rosbag2_transport/test/rosbag2_transport/test_record_all_include_unpublished_topics.cpp +++ b/rosbag2_transport/test/rosbag2_transport/test_record_all_include_unpublished_topics.cpp @@ -36,7 +36,7 @@ TEST_F(RecordIntegrationTestFixture, record_all_include_unpublished_false_ignore string_topic, 10, [](test_msgs::msg::Strings::ConstSharedPtr) {}); rosbag2_transport::RecordOptions record_options = - {true, false, false, false, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, "rmw_format", 100ms}; + {true, false, false, false, {}, {}, {}, {}, {}, {}, {}, {}, {}, "rmw_format", 100ms}; record_options.include_unpublished_topics = false; auto recorder = std::make_shared(writer_, storage_options_, record_options); recorder->record(); @@ -55,7 +55,7 @@ TEST_F(RecordIntegrationTestFixture, record_all_include_unpublished_true_include string_topic, 10, [](test_msgs::msg::Strings::ConstSharedPtr) {}); rosbag2_transport::RecordOptions record_options = - {true, false, false, false, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, "rmw_format", 100ms}; + {true, false, false, false, {}, {}, {}, {}, {}, {}, {}, {}, {}, "rmw_format", 100ms}; record_options.include_unpublished_topics = true; auto recorder = std::make_shared(writer_, storage_options_, record_options); recorder->record(); @@ -76,7 +76,7 @@ TEST_F( string_topic, 10, [](test_msgs::msg::Strings::ConstSharedPtr) {}); rosbag2_transport::RecordOptions record_options = - {true, false, false, false, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, "rmw_format", 100ms}; + {true, false, false, false, {}, {}, {}, {}, {}, {}, {}, {}, {}, "rmw_format", 100ms}; record_options.include_unpublished_topics = false; auto recorder = std::make_shared(writer_, storage_options_, record_options); recorder->record(); diff --git a/rosbag2_transport/test/rosbag2_transport/test_record_all_no_discovery.cpp b/rosbag2_transport/test/rosbag2_transport/test_record_all_no_discovery.cpp index f8f37d1cf4..04a864c78c 100644 --- a/rosbag2_transport/test/rosbag2_transport/test_record_all_no_discovery.cpp +++ b/rosbag2_transport/test/rosbag2_transport/test_record_all_no_discovery.cpp @@ -38,7 +38,7 @@ TEST_F(RecordIntegrationTestFixture, record_all_without_discovery_ignores_later_ string_message->string_value = "Hello World"; rosbag2_transport::RecordOptions record_options = - {true, false, false, true, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, "rmw_format", 100ms}; + {true, false, false, true, {}, {}, {}, {}, {}, {}, {}, {}, {}, "rmw_format", 100ms}; auto recorder = std::make_shared( std::move(writer_), storage_options_, record_options); recorder->record(); diff --git a/rosbag2_transport/test/rosbag2_transport/test_record_all_use_sim_time.cpp b/rosbag2_transport/test/rosbag2_transport/test_record_all_use_sim_time.cpp index ecbc87c524..d6a8f56b72 100644 --- a/rosbag2_transport/test/rosbag2_transport/test_record_all_use_sim_time.cpp +++ b/rosbag2_transport/test/rosbag2_transport/test_record_all_use_sim_time.cpp @@ -97,7 +97,7 @@ TEST_F(RecordIntegrationTestFixture, record_all_with_sim_time) rosbag2_transport::RecordOptions record_options = { - false, false, false, false, {string_topic, clock_topic}, {}, {}, {}, {}, {}, {}, {}, {}, {}, + false, false, false, false, {string_topic, clock_topic}, {}, {}, {}, {}, {}, {}, {}, {}, "rmw_format", 100ms }; record_options.use_sim_time = true; diff --git a/rosbag2_transport/test/rosbag2_transport/test_record_options.cpp b/rosbag2_transport/test/rosbag2_transport/test_record_options.cpp index fd4aec070c..bf8965c05e 100644 --- a/rosbag2_transport/test/rosbag2_transport/test_record_options.cpp +++ b/rosbag2_transport/test/rosbag2_transport/test_record_options.cpp @@ -35,7 +35,6 @@ TEST(record_options, test_yaml_serialization_deserialization) original.exclude_topic_types = {"exclude_type1", "exclude_type2"}; original.exclude_service_events = {"exclude_service1", "exclude_service2"}; original.exclude_actions = {"exclude_action1", "exclude_action2"}; - original.rmw_serialization_format = "cdr"; original.input_serialization_format = "cdr"; original.output_serialization_format = "cdr"; original.topic_polling_interval = std::chrono::milliseconds{200}; @@ -79,7 +78,6 @@ TEST(record_options, test_yaml_serialization_deserialization) CHECK(exclude_topic_types); CHECK(exclude_service_events); CHECK(exclude_actions); - CHECK(rmw_serialization_format); CHECK(input_serialization_format); CHECK(output_serialization_format); CHECK(topic_polling_interval); diff --git a/rosbag2_transport/test/rosbag2_transport/test_record_regex.cpp b/rosbag2_transport/test/rosbag2_transport/test_record_regex.cpp index d48f4c80a4..d35878f89c 100644 --- a/rosbag2_transport/test/rosbag2_transport/test_record_regex.cpp +++ b/rosbag2_transport/test/rosbag2_transport/test_record_regex.cpp @@ -65,7 +65,7 @@ TEST_F(RecordIntegrationTestFixture, regex_topics_recording) ASSERT_FALSE(std::regex_match(b4, re)); rosbag2_transport::RecordOptions record_options = - {false, false, false, false, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, "rmw_format", 10ms}; + {false, false, false, false, {}, {}, {}, {}, {}, {}, {}, {}, {}, "rmw_format", 10ms}; record_options.regex = regex; // TODO(karsten1987) Refactor this into publication manager @@ -138,7 +138,7 @@ TEST_F(RecordIntegrationTestFixture, regex_and_exclude_regex_topic_recording) ASSERT_TRUE(std::regex_match(e1, exclude)); rosbag2_transport::RecordOptions record_options = - {false, false, false, false, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, "rmw_format", 10ms}; + {false, false, false, false, {}, {}, {}, {}, {}, {}, {}, {}, {}, "rmw_format", 10ms}; record_options.regex = regex; record_options.exclude_regex = topics_regex_to_exclude; @@ -214,7 +214,7 @@ TEST_F(RecordIntegrationTestFixture, regex_and_exclude_topic_topic_recording) ASSERT_TRUE(e1 == topics_exclude); rosbag2_transport::RecordOptions record_options = - {false, false, false, false, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, "rmw_format", 10ms}; + {false, false, false, false, {}, {}, {}, {}, {}, {}, {}, {}, {}, "rmw_format", 10ms}; record_options.regex = regex; record_options.exclude_topics.emplace_back(topics_exclude); @@ -277,7 +277,7 @@ TEST_F(RecordIntegrationTestFixture, regex_and_exclude_regex_service_recording) std::string b2 = "/namespace_before/not_nice"; rosbag2_transport::RecordOptions record_options = - {false, false, false, false, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, "rmw_format", 10ms}; + {false, false, false, false, {}, {}, {}, {}, {}, {}, {}, {}, {}, "rmw_format", 10ms}; record_options.regex = regex; record_options.exclude_regex = services_regex_to_exclude; @@ -359,7 +359,7 @@ TEST_F(RecordIntegrationTestFixture, regex_and_exclude_service_service_recording std::string b2 = "/namespace_before/not_nice"; rosbag2_transport::RecordOptions record_options = - {false, false, false, false, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, "rmw_format", 10ms}; + {false, false, false, false, {}, {}, {}, {}, {}, {}, {}, {}, {}, "rmw_format", 10ms}; record_options.regex = regex; record_options.exclude_service_events.emplace_back(services_exclude); @@ -441,7 +441,7 @@ TEST_F(RecordIntegrationTestFixture, regex_and_exclude_regex_action_recording) std::string b2 = "/namespace_before/not_nice"; rosbag2_transport::RecordOptions record_options = - {false, false, false, false, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, "rmw_format", 10ms}; + {false, false, false, false, {}, {}, {}, {}, {}, {}, {}, {}, {}, "rmw_format", 10ms}; record_options.regex = regex; record_options.exclude_regex = actions_regex_to_exclude; @@ -541,7 +541,7 @@ TEST_F(RecordIntegrationTestFixture, regex_and_exclude_actions_action_recording) std::string b2 = "/namespace_before/not_nice"; rosbag2_transport::RecordOptions record_options = - {false, false, false, false, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, "rmw_format", 10ms}; + {false, false, false, false, {}, {}, {}, {}, {}, {}, {}, {}, {}, "rmw_format", 10ms}; record_options.regex = regex; record_options.exclude_actions = action_exclude; diff --git a/rosbag2_transport/test/rosbag2_transport/test_record_services.cpp b/rosbag2_transport/test/rosbag2_transport/test_record_services.cpp index 967a4b5451..3a4b77336b 100644 --- a/rosbag2_transport/test/rosbag2_transport/test_record_services.cpp +++ b/rosbag2_transport/test/rosbag2_transport/test_record_services.cpp @@ -113,7 +113,6 @@ class RecordSrvsTest : public RecordIntegrationTestFixture rosbag2_transport::RecordOptions record_options; record_options.is_discovery_disabled = is_discovery_disabled_; record_options.topics = record_topics; - record_options.rmw_serialization_format = "rmw_format"; record_options.topic_polling_interval = 100ms; record_options.use_sim_time = use_sim_time_; storage_options_.snapshot_mode = snapshot_mode_; diff --git a/rosbag2_transport/test/rosbag2_transport/test_record_static_topics.cpp b/rosbag2_transport/test/rosbag2_transport/test_record_static_topics.cpp index 3d62e173e3..e47d1d8398 100644 --- a/rosbag2_transport/test/rosbag2_transport/test_record_static_topics.cpp +++ b/rosbag2_transport/test/rosbag2_transport/test_record_static_topics.cpp @@ -29,7 +29,7 @@ namespace fs = std::filesystem; TEST_F(RecordIntegrationTestFixture, recorder_can_read_static_topics_list) { rosbag2_transport::RecordOptions record_options = rosbag2_transport::RecordOptions{ - true, true, true, true, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, "rmw_format", 100ms}; + true, true, true, true, {}, {}, {}, {}, {}, {}, {}, {}, {}, "rmw_format", 100ms}; // _SRC_RESOURCES_DIR_PATH defined in CMakeLists.txt record_options.static_topics_uri = @@ -55,7 +55,7 @@ TEST_F(RecordIntegrationTestFixture, exclude_applies_above_static_topics_list) rosbag2_test_common::PublicationManager pub_manager; pub_manager.setup_publisher("/topic_name1", string_message, 5); rosbag2_transport::RecordOptions record_options = rosbag2_transport::RecordOptions{ - false, false, false, true, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, "rmw_format", 100ms}; + false, false, false, true, {}, {}, {}, {}, {}, {}, {}, {}, {}, "rmw_format", 100ms}; record_options.include_unpublished_topics = true; record_options.exclude_regex = ".*_name2.*";