Skip to content
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

[pr2eus] Add header timestamp to actiongoal topic in play-sound #495

Merged

Conversation

tkmtnt7000
Copy link
Member

Now, when we execute something like (send *ri* :speak-jp "こんにちは"), sound action goal topic has no headertimestamp.
I fixed it in this PR.

# before this PR
$ rostopic echo /robotsound_jp/goal
header: 
  seq: 0
  stamp: 
    secs: 0
    nsecs:         0
  frame_id: ''
goal_id: 
  stamp: 
    secs: 0
    nsecs:         0
  id: "1686130902134708426_/fetch_eus_interface_1686130898541295371_2753725_robotsound_jp_0"
goal: 
  sound_request: 
    sound: -3
    command: 1
    volume: 1.0
    arg: "\u3053\u3053\u304Cdock\u3067\u3059"
    arg2: "ja"

@tkmtnt7000 tkmtnt7000 added the bug label Jun 7, 2023
@tkmtnt7000 tkmtnt7000 force-pushed the PR-add-timestamp-for-play-sound branch from 513663c to d84d634 Compare June 7, 2023 09:57
@pazeshun
Copy link
Collaborator

pazeshun commented Jun 7, 2023

In JointTrajectoryAction, timestamp of action goal message means when to start trajectory.
And if goal timestamp + time_from_start of some trajectory points are older than current time, these points are not executed.
This is sometimes troublesome especially when muliple PCs having independent clocks exist, so we sometimes feel setting timestamp 0 is better because it means immediate execution:
start-jsk/rtmros_common#1052 (comment)

I'm not sure about sound action (and timestamp added by this PR is action goal topic's one, not goal message's one), but please check carefully.

@k-okada
Copy link
Member

k-okada commented Jun 8, 2023

@pazeshun I think ROS controller uses goal.trajectory.header.stamp, not goal.header

http://wiki.ros.org/pr2_controllers/Tutorials/Moving%20the%20arm%20using%20the%20Joint%20Trajectory%20Action#CA-11505f3470a9a076683bc46f9e917c9b05155c64_37

https://github.com/ros-controls/ros_controllers/blob/dce87855e1adf3a66271db8243d58561da9f2cdb/joint_trajectory_controller/include/joint_trajectory_controller/joint_trajectory_controller_impl.h#L610-L612

on pr2eus, we already set start time for both goal.trajectory.header.stamp and goal.header

(send goal :header :stamp st)
(cond
((equal (class goal) control_msgs::SingleJointPositionActionGoal)
(let* ((joint (car joints))
(id (position joint (send robot :joint-list)))
(pos (deg2rad (elt (elt (car trajpoints) 0) id)))
)
(send goal :goal :position pos)
(send goal :goal :max_velocity 5)
(send self :spin-once)
)
)
(t
(send goal :goal :trajectory :joint_names joint-names)
(send goal :goal :trajectory :header :stamp st)

@tkmtnt7000
Copy link
Member Author

I see. Thanks for your advice.

Now as long as I have checked, it seems that soundplay_node.py does not use timestamp of action goal topic, so I think this PR's change does not immediately have a side effect such as what you points out.
https://github.com/ros-drivers/audio_common/blob/3a748b223885a2f6035668c784e006e5426c84ef/sound_play/scripts/soundplay_node.py

FYI:
What I originally wanted to do was to avoid that the timestamp would be 1970/1/1 if the timestamp was left at 0 when using the function to remember the robot's speech and recall it later. (as part of our recent efforts around database_talker)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants