Remove JumpHandler copy-implementation from PlayerClock/TimeControllerClock #935
Conversation
emersonknapp
force-pushed
the
emersonknapp/remove-player-clock-jump-handler
branch
from
baa8613
to
48585d0
Compare
emersonknapp
requested review from
MichaelOrlov
and removed request for
a team
|
Gist: https://gist.githubusercontent.com/emersonknapp/2853603ea13179bcaaa53f419dc1421b/raw/a61c989f67630c06e2a4da576e6fabeb14ae9b69/ros2.repos
|
…rClock This API was originally envisioned as a way to handle the `seek`/`jump` operation, but that feature ended up being implemented inline in the `Player`. This is not used now, and I do not anticipate using it in the future. Removing as housekeeping - if it is needed later it can always be re-added from history. Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
emersonknapp
force-pushed
the
emersonknapp/remove-player-clock-jump-handler
branch
from
48585d0
to
ec98835
Compare
|
delete-merged-branch
bot
deleted the
emersonknapp/remove-player-clock-jump-handler
branch
There was a problem hiding this comment.
@emersonknapp I didn't get from description of this task what does it means copy-implementation ? And why do you still need create_jump_callback(..) ?
| ROSBAG2_CPP_PUBLIC | ||
| virtual void remove_jump_callbacks(PlayerClock::JumpHandler::SharedPtr handler) = 0; | ||
| virtual rclcpp::JumpHandler::SharedPtr create_jump_callback( |
There was a problem hiding this comment.
@emersonknapp If you are removing jump callbacks from player_clock class, why do you need this API create_jump_callback(..) which is returning null_ptr?
There was a problem hiding this comment.
I meant "it's an implementation that was mostly copy-pasted from rclcpp". I will use the jump handler in the ROStime implementation #928 - i could have added this there but it will end up the same
This API was originally envisioned as a way to handle the
seek/jumpoperation, but that feature ended up being implemented inline in thePlayer. This is not used now, and I do not anticipate using it in the future. Removing as housekeeping - if it is needed later it can always be re-added from history.Depends on ros2/rclcpp#1842