Skip to content

Commit e1ba8b8

Browse files
[humble] Bugfix for incorrect MCAPStorage::seek(timestamp) when timestamp is current (backport #2157) (#2161)
* Bugfix for MCAPStorage::seek(time) advance in time is current (#2157) - The seek(timestamp) shall adhere >= condition even when target timestamp is equal to the current timestamp in the storage. Signed-off-by: Michael Orlov <[email protected]> (cherry picked from commit 2738d77) # Conflicts: # rosbag2_transport/test/rosbag2_transport/test_play_seek.cpp * Address merge conflicts Signed-off-by: Michael Orlov <[email protected]> --------- Signed-off-by: Michael Orlov <[email protected]> Co-authored-by: Michael Orlov <[email protected]>
1 parent 0db8a54 commit e1ba8b8

File tree

2 files changed

+32
-3
lines changed

2 files changed

+32
-3
lines changed

rosbag2_storage_mcap/src/mcap_storage.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -654,9 +654,7 @@ void MCAPStorage::reset_filter()
654654

655655
void MCAPStorage::seek(const rcutils_time_point_value_t & time_stamp)
656656
{
657-
if (time_stamp != last_read_time_point_) {
658-
last_read_message_offset_ = std::nullopt;
659-
}
657+
last_read_message_offset_ = std::nullopt;
660658
last_read_time_point_ = time_stamp;
661659
reset_iterator();
662660
}

rosbag2_transport/test/rosbag2_transport/test_play_seek.cpp

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,37 @@ TEST_P(RosBag2PlaySeekTestFixture, seek_with_timestamp_later_than_in_last_messag
180180
ASSERT_THAT(replayed_topic1, SizeIs(expected_number_of_messages));
181181
}
182182

183+
TEST_P(RosBag2PlaySeekTestFixture, reader_can_correctly_do_seek) {
184+
reader_->open(storage_options_);
185+
const auto first_msg_timestamp = start_time_ms_ * 1000000;
186+
const auto second_msg_timestamp = (start_time_ms_ + message_spacing_ms_) * 1000000;
187+
const auto third_msg_timestamp = (start_time_ms_ + message_spacing_ms_ * 2) * 1000000;
188+
ASSERT_TRUE(reader_->has_next());
189+
auto msg = reader_->read_next();
190+
EXPECT_EQ(msg->time_stamp, first_msg_timestamp); // First message timestamp
191+
192+
// Jump on third message (1200 ms)
193+
reader_->seek(third_msg_timestamp);
194+
ASSERT_TRUE(reader_->has_next());
195+
msg = reader_->read_next();
196+
EXPECT_EQ(msg->time_stamp, third_msg_timestamp);
197+
198+
// Jump back on second message (1100 ms)
199+
reader_->seek(second_msg_timestamp);
200+
ASSERT_TRUE(reader_->has_next());
201+
msg = reader_->read_next();
202+
EXPECT_EQ(msg->time_stamp, second_msg_timestamp);
203+
ASSERT_TRUE(reader_->has_next());
204+
msg = reader_->read_next();
205+
EXPECT_EQ(msg->time_stamp, third_msg_timestamp);
206+
207+
// Jump on third message (1200 ms) where timestamp is exactly the same as previous read message
208+
reader_->seek(third_msg_timestamp);
209+
ASSERT_TRUE(reader_->has_next());
210+
msg = reader_->read_next();
211+
EXPECT_EQ(msg->time_stamp, third_msg_timestamp);
212+
}
213+
183214
TEST_P(RosBag2PlaySeekTestFixture, seek_forward) {
184215
const size_t expected_number_of_messages = num_msgs_in_bag_ - 1;
185216
auto player = std::make_shared<MockPlayer>(std::move(reader_), storage_options_, play_options_);

0 commit comments

Comments
 (0)