-
Notifications
You must be signed in to change notification settings - Fork 126
APP-8776 Move geometry proto conversions to reference frame #5149
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
APP-8776 Move geometry proto conversions to reference frame #5149
Conversation
…6-move-pointcloud-to-spatialmath
…6-move-pointcloud-to-spatialmath
raybjork
left a comment
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.
Tests are failing but otherwise this seems good
I think the failures are from me pointing to my local version of the API, since the API changes PR hasn't been merged yet. I will run them on my local to verify, but assuming they pass we should be good. The next steps will be for me to double-check the SDKs to make sure the changes in API don't break anything there, then I will announce the breaking changes and merge everything to go out in the same release. |
…6-move-pointcloud-to-spatialmath
…6-move-pointcloud-to-spatialmath
…6-move-pointcloud-to-spatialmath
…6-move-pointcloud-to-spatialmath
…6-move-pointcloud-to-spatialmath
…6-move-pointcloud-to-spatialmath
|
@raybjork merged the API PR, all tests are passing with the actual changes. |
This PR relocates the proto conversions for geometries and point clouds to the
referenceframepackage, based on the changes in this API PR.The reason for moving proto conversions to
referenceframewas driven by the inclusion of the point cloud as a geometry type. Thereferenceframepackage is already dependent on both thespatialmathandpointcloud, so it did not introduce any cyclical import issues to handle the conversion logic. After some discussion, although this is not the ideal long-term location for this code, it makes the most sense for now.API PR: viamrobotics/api#695
Scope doc: https://docs.google.com/document/d/1ionvyBa7x3HZU_rwDPBvrAUrQjBrP6sJ10Z_xbBTue8/edit?tab=t.0#heading=h.tcicyojyqi6c