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.

Sign up for GitHub

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

Jump to bottom

add rotation around origin for Rotation class #245

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

add rotation around origin for Rotation class #245

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
f80d8d2
add rotation around origin
ben-ray Sep 17, 2024
2e6a36f
Merge branch 'main' into ray/add_rotation_around_origin
ben-ray Sep 17, 2024
d732159
fix lint
ben-ray Sep 17, 2024
d2bb5ea
Merge branch 'ray/add_rotation_around_origin' of https://github.com/b…
ben-ray Sep 17, 2024
c5d065a
fix lint
ben-ray Sep 17, 2024
98af122
l i n t
ben-ray Sep 17, 2024
4c29c88
update 4x4 to 3x3 matrix, add sep tests file
ben-ray Sep 18, 2024
af901f1
implement offset origin for 3x3
ben-ray Sep 18, 2024
1cd78c8
l i n t
ben-ray Sep 18, 2024
3bd687d
indent
ben-ray Sep 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 25 additions & 14 deletions pylabrobot/resources/rotation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,36 +1,47 @@
import math

from pylabrobot.resources.coordinate import Coordinate
from pylabrobot.utils.linalg import matrix_multiply_3x3


class Rotation:
""" Represents a 3D rotation. """

def __init__(self, x: float = 0, y: float = 0, z: float = 0):
self.x = x # around x-axis, roll
self.y = y # around y-axis, pitch
self.z = z # around z-axis, yaw
self.x = x
self.y = y
self.z = z

def get_rotation_matrix(self):
# Create rotation matrices for each axis
Rz = ([
def get_rotation_and_translation(self, origin: Coordinate = Coordinate.zero()):
# rotation matrices for each axis
Comment on lines +13 to +14
Copy link
Member

Choose a reason for hiding this comment

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

I was thinking about storing the origin as an attribute on the class itself

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good idea

Rz = [
[math.cos(math.radians(self.z)), -math.sin(math.radians(self.z)), 0],
[math.sin(math.radians(self.z)), math.cos(math.radians(self.z)), 0],
[0, 0, 1]
])
Ry = ([
]
Ry = [
[math.cos(math.radians(self.y)), 0, math.sin(math.radians(self.y))],
[0, 1, 0],
[-math.sin(math.radians(self.y)), 0, math.cos(math.radians(self.y))]
])
Rx = ([
]
Rx = [
[1, 0, 0],
[0, math.cos(math.radians(self.x)), -math.sin(math.radians(self.x))],
[0, math.sin(math.radians(self.x)), math.cos(math.radians(self.x))]
])
]
# Combine rotations: The order of multiplication matters and defines the behavior significantly.
# This is a common order: Rz * Ry * Rx
return matrix_multiply_3x3(matrix_multiply_3x3(Rz, Ry), Rx)
rotation_matrix = matrix_multiply_3x3(matrix_multiply_3x3(Rz, Ry), Rx)

origin_vector = origin.vector()
rotated_origin = [
sum(rotation_matrix[i][j] * origin_vector[j] for j in range(3)) for i in range(3)
]
translation = [
rotated_origin[0] - origin.x,
rotated_origin[1] - origin.y,
rotated_origin[2] - origin.z
]

return rotation_matrix, translation
Comment on lines +43 to +44
Copy link
Member

Choose a reason for hiding this comment

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

we need to return the rotation because otherwise it would break get_absolute_rotation and other functions elsewhere

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm idk how to do this without a 4x4 matrix or a second translation matrix in addition to the rotation matrix

4x4 would just add the translation as a column, but would then have to be implemented in get_absolute_rotation and others?

rotation_matrix_4x4 = [
            [rotation_matrix_3x3[0][0], rotation_matrix_3x3[0][1], rotation_matrix_3x3[0][2], translation.x],
            [rotation_matrix_3x3[1][0], rotation_matrix_3x3[1][1], rotation_matrix_3x3[1][2], translation.y],
            [rotation_matrix_3x3[2][0], rotation_matrix_3x3[2][1], rotation_matrix_3x3[2][2], translation.z],
            [0, 0, 0, 1]  # Last row for homogeneous transformation
        ]

Copy link
Member

Choose a reason for hiding this comment

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

i think this method should just return the rotation matrix, no translation. translation can be applied when actually using the rotation matrix (in get_absolute_location i think)


def __str__(self) -> str:
return f"Rotation(x={self.x}, y={self.y}, z={self.z})"
Expand Down
27 changes: 27 additions & 0 deletions pylabrobot/resources/rotation_tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import unittest
from pylabrobot.resources.coordinate import Coordinate
from pylabrobot.resources.rotation import Rotation

class TestRotation(unittest.TestCase):
""" Tests for the Rotation class. """
def test_get_rotation_and_translation(self):
rotation = Rotation(x=0, y=0, z=90)
origin = Coordinate(x=1, y=0, z=1)

rotation_matrix, translation = rotation.get_rotation_and_translation(origin)

expected_rotation_matrix = [
[0, -1, 0],
[1, 0, 0],
[0, 0, 1]
]
expected_translation = [-1, 1, 0]

for i in range(3):
for j in range(3):
self.assertAlmostEqual(rotation_matrix[i][j], expected_rotation_matrix[i][j], places=4)
for i in range(3):
self.assertAlmostEqual(translation[i], expected_translation[i], places=4)

if __name__ == '__main__':
unittest.main()
Loading