Adds PlaybackPlugin with ignition-gui 3 #376
Conversation
|
2f81bc1 --> Changes icon style for the buttons |
| // An ignition transport node. | ||
| ignition::transport::Node node_; | ||
|
|
||
| SimTimes time_status_; |
There was a problem hiding this comment.
timeStatus_ to keep the format?
There was a problem hiding this comment.
Nit: I believe you should remove the underscore at the end and add one in all function arguments
| // @brief Converts from ignition::msgs::Time to std::chrono::duration. | ||
| // @param[in] src Time to be converted | ||
| // @returns `src` converted to std::chrono::duration::nanoseconds. | ||
| std::chrono::nanoseconds TimeToChrono(const ignition::msgs::Time& src) { |
There was a problem hiding this comment.
Let's call it IgnitionTimeToChrono
| // @param[out] dst Duration | ||
| // @throws When `dst` is nullptr. | ||
| void ChronoToDuration(const std::chrono::nanoseconds& src, ignition::msgs::Duration* dst) { | ||
| DELPHYNE_DEMAND(dst != nullptr); |
There was a problem hiding this comment.
Throw instead of abort.
| // @throws When `dst` is nullptr. | ||
| void ChronoToDuration(const std::chrono::nanoseconds& src, ignition::msgs::Duration* dst) { | ||
| DELPHYNE_DEMAND(dst != nullptr); | ||
| std::chrono::seconds src_in_seconds = std::chrono::duration_cast<std::chrono::seconds>(src); |
There was a problem hiding this comment.
I think it can be const.
|
|
||
| PlaybackPlugin::PlaybackPlugin() : ignition::gui::Plugin() { qRegisterMetaType<ignition::msgs::PlaybackStatus>(); } | ||
|
|
||
| PlaybackPlugin::~PlaybackPlugin() {} |
There was a problem hiding this comment.
Can you make it default or implicit?
| import QtQuick.Controls.Material 2.1 | ||
| import QtQuick.Layouts 1.3 | ||
|
|
||
| Rectangle { |
There was a problem hiding this comment.
Code in this file should also be documented.
There was a problem hiding this comment.
I just added some minor comments. I don't know how deep it should be given that there isn't that much code here.
| # | ||
|
|
||
| """ | ||
| The replay tool. |
There was a problem hiding this comment.
Would you please a doctring explaining how it works?
| OUTPUT_NAME PlaybackPlugin | ||
| ) | ||
|
|
||
| target_link_libraries(PlaybackPlugin |
There was a problem hiding this comment.
Don't you need to link against ignition-plugin1::register ?
There was a problem hiding this comment.
Apparently. However It didn't complain about it.
Added.
| ARCHIVE DESTINATION lib | ||
| ) | ||
|
|
||
| include_directories( |
There was a problem hiding this comment.
This is already four times in this file. Can we just leave it once?
There was a problem hiding this comment.
You are right. Done.
| anchors.leftMargin : 10 | ||
| anchors.verticalCenter : rewindButton.verticalCenter | ||
| onClicked: { | ||
| if (isPlaying){ |
There was a problem hiding this comment.
I think that 2 spaces indentation is OK. Would you please adjust it?
Part of #332
PlaybackWidgetwhich uses ign-gui0.delphyne_replay2.mp4
layout2_for_playback.configin which thePlaybackPluginis added.In addition a
delphyne_gui/python/toolkit/replay2.pyscript was added for executing the replayer with the new visualizer and with thePlaybackPlugin.Note: In essence this app is equal to the
delphyne_gui/python/toolkit/replay.pyscript that runs the old visualizer withPlaybackWidget, the only difference is that during this script the plugin is injected to the default layout instead of directly using the layout created for that end.For executing the
replayapp:1 - Create a log file, e.g.:
2 - Execute
delphyne_replay2app: