Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Design] Add design on recording and replay action #1928
base: rolling
Are you sure you want to change the base?
[Design] Add design on recording and replay action #1928
Changes from 2 commits
ea9bd92
d8246df
f989395
52f39c7
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would reverse the logic for the
--send-action-goal
parameter and send by default the real action's service requests instead of the recorded service events. As far as I understand, this would be a desired behavior in most cases.I also found that
--send-action-goal
is unclear about what it really means and difficult to explain in the comments.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is currently a parameter for play service request. If we reverse the logic for the --send-action-goal parameter, users will notice that the logic for services and actions is different.
How about the following description?
Send recorded action goal. If this command parameter is not set, replay the send_goal service event, get_result service event, cancel_service event, feedback topic, and status topic in the recorded order. You can use ros2 action echo to view it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmmm, i do not remember exactly how we get to decide this default behavior...
probably user needs to explicitly specify the send the actual request to the service because service server needs to be running in ROS 2 network. compared to the topic playback, this is kinda different expectation for user. the same logic goes to the action playback, the action server needs to be running when user specifies.
@Barry-Xu-2018
--send-action-goal
sounds like only sending the goal request, but it just replays all the data stored in rosbag database right? including result request and cancel request.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The naming is hard!
Now I am questioning myself if do really need one more extra parameter
--send-action-goal
?It seems that
--publish-service-requests
is doing exactly what--send-action-goal
is supposed to do.i.e. transform action service events to the real service requests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fujitatomoya
Based on my idea, if the '--send-action-goal' is set, rosbag2 will replay the action goal requests in the order they were recorded. User need to launch action server.
When this option is not set, it will replay all recorded action data (in addition to the send_goal event, it also includes the timing of cancel_goal and get_result, as well as feedback and status data. Users can view this through ros2 action echo).
@MichaelOrlov
Yes. The essence of these two parameters is the same: whether to replay request or event messages. I used the new parameter '--send-action-goal' with another consideration in mind, which is to separately control the playback for services and actions. For example, service data is replayed with event messages, while action data can replay goal requests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i am inclined to take one option with
--publish-service-requests
including actions, but i will leave this to you and @MichaelOrlov