collaborative-robotics / documentation

CRTK Documentation
https://crtk-robotics.readthedocs.io
15 stars 1 forks source link

PoseStamped instead of TransformStamped for Cartesian space positions #1

Closed kschwan closed 1 year ago

kschwan commented 2 years ago

Hi,

I would like to suggest a change to the CRTK specification. I know the following is a breaking change and so may not be accepted because of that.

I propose using the PoseStamped message type for representing "single" (i.e., not a heirarchy of transformations) measurements/commands in Cartesian space. My arguments for doing so:

My reason for asking this is that I was looking to implement the CRTK spec in our MOPS surgical system, and here I found a discrepancy.

Thanks for your consideration, Kim

adeguet1 commented 2 years ago

Agreed. One of our heavy ROS user made a similar comment a while back. This will break users's code since the payload will change. So if we get a consensus for this change (@pkazanzides @blake5634 @melodysu83), I'd like to keep this in the devel branch until we get to a proper release (2.1?). Should we synchronize this with the next round of Raven, dVRK, CRTK ROS2... releases?

pkazanzides commented 2 years ago

I am fine with that change.

melodysu83 commented 2 years ago

Thanks for monitoring the new GitHub issue! That makes a lot of sense to me.

best, Melody

-- Yun-Hsuan (Melody) Su Assistant Professor of Computer Science Mount Holyoke College

Email: @.*** Office: Clapp 222 Phone: (413) 538-3468

On Wed, Dec 1, 2021 at 6:36 PM Anton Deguet @.***> wrote:

Agreed. One of our heavy ROS user made a similar comment a while back. This will break users's code since the payload will change. So if we get a consensus for this change @.*** https://github.com/pkazanzides @blake5634 https://github.com/blake5634 @melodysu83 https://github.com/melodysu83), I'd like to keep this in the devel branch until we get to a proper release (2.1?). Should we synchronize this with the next round of Raven, dVRK, CRTK ROS2... releases?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/collaborative-robotics/documentation/issues/1#issuecomment-984155975, or unsubscribe https://github.com/notifications/unsubscribe-auth/AETGMMCWZFBANSJEQ47D4IDUO2WQXANCNFSM5JFUEA5Q . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

adeguet1 commented 2 years ago

I have pushed some code related to this issue to a few repositories:

Regarding the documentation (https://github.com/collaborative-robotics/documentation/wiki/Robot-API-motion). Should I:

As far as I am concerned, CRTK is still a draft and not released (not 1.0 yet) so I'd be fine just replacing TransformStamped by PoseStamped.