Auto discovery of new topics #63
Conversation
anhosi
force-pushed
the
feature/auto_discovery_of_new_topics
branch
from
224a8a2
to
d3cd7c6
Compare
|
CI:
|
|
The macOS slave seems to have a connection problem. |
|
Trying macOS again: |
|
The macOS build fails because of Clang warnings in non rosbag2 code. |
|
We added the option New round of CI:
|
| @@ -41,9 +41,6 @@ class PublisherManager | |||
| auto publisher_node = std::make_shared<rclcpp::Node>("publisher" + std::to_string(counter++)); | |||
| auto publisher = publisher_node->create_publisher<T>(topic_name); | |||
|
|
|||
| // We need to publish one message to set up the topic for discovery | |||
| publisher->publish(message); | |||
There was a problem hiding this comment.
is this just legacy or why is this code removed?
There was a problem hiding this comment.
With auto discovery we no longer need to advertise the topic before starting rosbag in the test.
| auto subscribe_to_topics = | ||
| [this, topics_to_record, topic_polling_interval, is_discovery_disabled] { | ||
| do { | ||
| auto all_topics_and_types_to_subscribe = topics_to_record.empty() ? |
There was a problem hiding this comment.
I think this is pretty complicated for what it is.
wouldn't it be easier to have this function called once as a blocking call to start the recording and then asynchronously call node_->get_topics_with_types and subscribe to the not-yet-subscribed once? I think this makes it easier to follow what's happening here.
By extracting the new topics, the call to is_every_topic_subscribed is then not necessary anymore, because we know which topics to subscribe.
There was a problem hiding this comment.
By extracting the new topics, the call to
is_every_topic_subscribedis then not necessary anymore, because we know which topics to subscribe.
I don't understand. In the --all case we do not know what topics to expect.
rosbag2_transport/src/rosbag2_transport/rosbag2_transport_python.cpp
Outdated
Show resolved
Hide resolved
|
New CI:
|
1 similar comment
|
New CI:
|
- this allows new topics to be discovered also after startup. - as a consquence, ros2 bag record won't throw any exception anymore if the specified topics are not available at startup - each time the recorder subscribes to a new topic, this will be logged to console
- make option available in RecordOptions - set to 100ms for now
- use set for subscribed topics instead of vector - perform the asynchronous call only when discovery is needed
- naming of methods - emphasize similarity between first subscription and discovery loop
anhosi
force-pushed
the
feature/auto_discovery_of_new_topics
branch
from
6c0d458
to
ee11257
Compare
|
New CI:
|
…s_touch_up small touch ups
|
Using
ros2 bag record: Poll regularly for new topics if not all topics have been found yet orros2 bag recordis used with the--alloption to subscribe to all topics.The polling interval is currently set to 100ms and not exposed to the user, but this can easily be amended if necessary.
CI: