-
Notifications
You must be signed in to change notification settings - Fork 7
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
Return ROS time values #41
Conversation
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'm wondering if we need to support getting time as int
or float
? Like, why can't we just return embag.RosTime
and the client can call to_sec
or to_nsec
to get long/int or float. This way we won't have to pass around that ros_time_py_type
. Does rospy support just getting time/duration as primitive? May be this is for backward compatibility?
@kashish-jain Its mostly just about providing nice functionality for clients so that they don't have to worry about writing the conversions/serializations themselves and dealing with performance hits from deeply nested structures. IMO passing the arg around isn't too big of a deal. rospy doesn't provide any to_dict functionality, so theres no precedence there. |
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.
🚢
334bad3
to
5534a25
Compare
Fixes: DATA-627
Description of Changes
This switches the python interface to providing timestamps as ROS times, similar to how the actual rospy interface works.
Test Plan
Unit tests.