Skip to content

align rotation-matrix FFI to row-major convention#186

Open
Joseph Borodach (JosephBorodach) wants to merge 1 commit into
mainfrom
rotation-convention-fix
Open

align rotation-matrix FFI to row-major convention#186
Joseph Borodach (JosephBorodach) wants to merge 1 commit into
mainfrom
rotation-convention-fix

Conversation

@JosephBorodach

@JosephBorodach Joseph Borodach (JosephBorodach) commented Jul 10, 2026

Copy link
Copy Markdown
Member

Applying the analogous fix to what shipped in rdk#6193 (viamrobotics/rdk#6193).

Two motivations:

The behavior change to both rust_utils and the python sdk are safe bc we have no users of these APIs yet. Best to fix it before we do.

Question for rust-utils maintainers: what's the release cadence here? viam-python-sdk's postinstall.sh pulls rust-utils/releases/latest, so I'll need to know when this ships as a release to time the Python SDK follow-up

@viambot

viambot commented Jul 10, 2026

Copy link
Copy Markdown
Member

👋 Thanks for requesting a review from the team!

We aim to review PRs within one business day. If this is urgent
or blocking you, please reach out in #team-sdk and
we'll prioritize it.

elements: *const [f64; 9],
) -> *mut Rotation3<f64> {
null_pointer_check!(elements);
let matrix = Matrix3::from_vec(Vec::from(*elements));

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://docs.rs/nalgebra/latest/nalgebra/base/struct.Matrix.html#method.from_vec states

Creates a matrix with its elements filled with the components provided by a slice in column-major order

@nfranczak Nick Franczak (nfranczak) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

math checks out.
i defer approval to the sdk team.
please make sure that universal-robots still remains working as it uses rust-utils through FFI

@JosephBorodach

Copy link
Copy Markdown
Member Author

please make sure that universal-robots still remains working as it uses rust-utils through FFI

Yep already noted this in the rdk pr audit - universal-robots rust_utils.h only imports OV/Quaternion/Euler/AxisAngle FFI symbols, zero rotation-matrix functions. This PR only touches rotation-matrix symbols so its imports are untouched

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants