Skip to content

Commit 95f78b6

Browse files
Bugfix for incorrect playback rate changes when pressing buttons (#1513)
- Playback rate expected to be changed by 10% with each increase/decrease step. - Use +0.1 and -0.1 in decrease/increase rate formula instead of multiply by factor of the 1.1 and 0.9 respectively. Signed-off-by: Michael Orlov <[email protected]>
1 parent 0fcc839 commit 95f78b6

File tree

2 files changed

+10
-4
lines changed

2 files changed

+10
-4
lines changed

rosbag2_transport/src/rosbag2_transport/player.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -1061,12 +1061,12 @@ void PlayerImpl::add_keyboard_callbacks()
10611061
);
10621062
add_key_callback(
10631063
play_options_.increase_rate_key,
1064-
[this]() {owner_->set_rate(get_rate() * 1.1);},
1064+
[this]() {owner_->set_rate(get_rate() + 0.1);},
10651065
"Increase Rate 10%"
10661066
);
10671067
add_key_callback(
10681068
play_options_.decrease_rate_key,
1069-
[this]() {owner_->set_rate(get_rate() * 0.9);},
1069+
[this]() {owner_->set_rate(get_rate() - 0.1);},
10701070
"Decrease Rate 10%"
10711071
);
10721072
}

rosbag2_transport/test/rosbag2_transport/test_keyboard_controls.cpp

+8-2
Original file line numberDiff line numberDiff line change
@@ -157,8 +157,14 @@ TEST_F(RosBag2PlayTestFixture, test_keyboard_controls)
157157
keyboard_handler->simulate_key_press(play_options_.pause_resume_toggle_key);
158158
EXPECT_THAT(player->is_paused(), true);
159159

160-
keyboard_handler->simulate_key_press(play_options_.increase_rate_key);
160+
EXPECT_DOUBLE_EQ(player->get_rate(), 1.0);
161161
keyboard_handler->simulate_key_press(play_options_.decrease_rate_key);
162+
// Each increase/decrease shall change playback rate value by 10%
163+
EXPECT_DOUBLE_EQ(player->get_rate(), 0.9);
164+
keyboard_handler->simulate_key_press(play_options_.increase_rate_key);
165+
EXPECT_DOUBLE_EQ(player->get_rate(), 1.0);
166+
keyboard_handler->simulate_key_press(play_options_.increase_rate_key);
167+
EXPECT_DOUBLE_EQ(player->get_rate(), 1.1);
162168

163169
// start playback asynchronously in a separate thread
164170
player->play();
@@ -175,7 +181,7 @@ TEST_F(RosBag2PlayTestFixture, test_keyboard_controls)
175181
EXPECT_THAT(player->num_paused, 1);
176182
EXPECT_THAT(player->num_resumed, 1);
177183
EXPECT_THAT(player->num_played_next, 1);
178-
EXPECT_THAT(player->num_set_rate, 2);
184+
EXPECT_THAT(player->num_set_rate, 3);
179185
}
180186

181187
TEST_F(RecordIntegrationTestFixture, test_keyboard_controls)

0 commit comments

Comments
 (0)