RigidJoint and RevoluteJoint don't align correctly

[bernhard-42]

https://discord.com/channels/964330484911972403/1074840524181217452/1183060124353433641

b1 = Box(10, 10, 1)
b2 = Box(5, 5, 1)
j1 = RigidJoint("j1", b1, Pos(-4, -4, 0.5))
j2 = RevoluteJoint("j2", b2, Axis((2.5, 2.5, 0), (0, -1, 0)))

j1.connect_to(j2, angle=30)
show(b1, b2, render_joints=True)

Three issues I don't get: 1) Revolute axis ignored when adding an angle 2) Absolute positions not the same after connect_to 3) Wrong order of joints when I think about connecting like a carpenter does Admittedly, the third one might be subjective, and is of minor importance. I can flip the order in my mind. But the first two are unclear to me

print(j1.parent.location * j1.relative_location)
print(j2.parent.location * j2.relative_axis.location)
# Location: (position=(-4.00, -4.00, 0.50), orientation=(-0.00, 0.00, -0.00))
# Location: (position=(-4.00, -4.00, 0.50), orientation=(-90.00, -60.00, -0.00))

[bernhard-42]

I added to RevoluteJoint, LinearJoint, CylindricalJoint

    @property
    def absolute_location(self):
        """Absolute location of joint"""
        return self.parent.location * self.relative_axis.location

and to RigidJoint, BallJoint

    @property
    def absolute_location(self):
        """Absolute location of joint"""
        return self.parent.location * self.relative_location

and replaced _connect_to in class Joint by

    def _connect_to(self, other: Joint, **kwargs):  # pragma: no cover
        """Connect Joint self by repositioning other"""
        if kwargs.get("angle") is not None:
            transform = Rotation(0, 0, kwargs["angle"])
        else:
            transform = Location()

        self.parent.location = (
            other.absolute_location * transform * self.absolute_location.inverse()
        ) * self.parent.location
        self.connected_to = other

    @property
    @abstractmethod
    def absolute_location(self) -> Location:  # pragma: no cover
        """A CAD object positioned in global space to illustrate the joint"""
        raise NotImplementedError

Then j2.connect_to(j1, angle=45) results in

and a following j2.connect_to(j1, angle=0) results in

What I'd like to demonstrate here is that working with the absolute locations for calculating the new location of j2 can be done very simply. And since self.parent is relocated to the global origin by self.absolute_location.inverse() the angle rotation can be easily applied and respects the direction of the axis (always automatically the z-axis)

[bernhard-42]

I think applying this pattern will both simplify and correct the joint handling.

Let's generalize the example:

loc1 = Location((1, 2, 3), (10, 20, 30))
loc2 = Location((16, 4, 0), (-30, -40, 50))

b1 = Box(10, 10, 1)
b2 = Box(5, 5, 1)
RigidJoint("j1", b1, Pos(-4, -4, 0.5))
RevoluteJoint("j2", b2, Axis((2.5, 2.5, 0), (0, -1, 0)))

b1 = loc1 * b1
b2 = loc2 * b2

show(b1, b2)

j2.connect_to(j1, angle=0) resuts in

[bernhard-42]

Something is currently already broken, I think:

Shouldn't the arm's edge aligned with the base edge?

[bernhard-42]

And a third issue is around LinearJoint, see https://discord.com/channels/@me/1083392680039555192/1185620998876303450

slider_arms are oriented the same in world coords, but not relative to the LinearJoint

[easonke]

For RigidJoint and RevoluteJoint alignment, I think the issue is in RevoluteJoint:relative_to, around below line where the transformation is calculated. self.relative_axis.location rotation other.relative_location.inverse()

I had a fix as below. I tried several different manual tests locally and it seems working as expected. But I am new to build123d for like just 3 days, so please take a close look. https://github.com/easonke/build123d/commit/389da70b70d9a47000fb40811f5a446ed5400d36

[bernhard-42]

For reference: https://github.com/gumyr/build123d/tree/joints-v2, an attempt to fix these issues

[zackyancey]

I had been struggling with this problem for a while before coming across this issue, the joints-v2 branch fixed it for me. @bernhard-42, are there any plans to merge that branch?

[gumyr]

I'm pretty sure that the Joints systems didn't have the current problems in the past and suffered from a change(s) that passes all of the tests (code coverage is at 100% IIRC) but doesn't cover all of the scenarios/math. Adding tests that currently fail is a high priority to me to ensure that the code stays functional going forward. So, if you have any scenarios that currently fail that could be a unittest, please share.

... and yes, I'm going to but this fix in my high priority list - so much to do.

[bernhard-42]

@zackyancey I am kind of already waiting a bit for the joints stuff being looked at 😉 But having my own open source project, I totally understand the issue of time and priorities ...

[zackyancey]

Here's what I was doing:

from build123d import *

A = Box(6, 4, 2)
B = Cone(2, 1, 2)
jA = RevoluteJoint("jA", A, Axis((3, 0, 1), (0, -1, 0)))
jB = RigidJoint("jB", B, Location((-2, 0, -1), (90, 0, -90)))

A.joints["jA"].connect_to(B.joints["jB"], angle=30)

print(B.location)

# Lazy version of assertAlmostEqual:
assert str(B.location) == "Location: (position=(4.23, -0.00, 2.87), orientation=(0.00, -30.00, 0.00))"

Hoping for this

Got this

I'm pretty sure this is basically the same code that's in this comment.

No worries, I get that there's a big todo list (brief appreciation aside since I've got the maintainers of two of my recent favorite open source projects here—build123d and ocp-vscode is basically my dream CAD environment, very quickly after finding this it's already easier to make complex (for me) parametric models than it ever was in F360/OpenSCAD. amazing work!)

[zackyancey]

I wrote up the failing case as a unit test, in case that's helpful (#613). I wasn't sure if I should put it on the joints branch or if there's a convention for expected failures, I can move/modify it if needed.

[gumyr]

This is very helpful - thank you!

[bernhard-42]

@gumyr Is there anything I can help to get the joints stuff prioritised (it is >5 months old in the meantime)? The joints-v2 branch is fully functional, with all tests passing (I only tweaked some of them, where the joints logic was broken before). If you review it and let me know what to fix I am happy to work on it.

[gumyr]

@bernhard-42 Sorry this is taking so long. I've got a fix for this that passes all of the existing tests - instead of this (in RevoluteJoint.relative_to:

        rotation = Location(
            Plane(
                origin=(0, 0, 0),
                x_dir=self.angle_reference.rotate(self.relative_axis, angle),
                z_dir=self.relative_axis.direction,
            )
        )
        return (
            self.relative_axis.location * rotation * other.relative_location.inverse()
        )

this:

        return (
            self.relative_axis.location
            * Rotation(0, 0, angle)
            * other.relative_location.inverse()
        )

but I'd like to extend the tests to try to get better coverage - 100% coverage by line numbers doesn't verify that the math is correct necessarily.

Here is a slightly modified version of your example from the top:

b1 = Box(10, 10, 1)
b2 = extrude(Trapezoid(5, 5, 70, 90), 1)
j1 = RigidJoint("j1", b1, Pos(-4, -4, 0.5))
j2 = RevoluteJoint("j2", b2, Axis((2.5, 2.5, 0), (0, -1, 0)))

j1.connect_to(j2, angle=30)

Before connect_to: After:

[bernhard-42]

No worries, I was just curious.

I would still recommend to look at my branch. I spent quite some time to simplify/harmonize the logic. And I took care that after joining the absolute location of both joints are identical. This was broken earlier => candidate for some tests?

Do you have a branch with your changes? I could try to break it and write further tests while doing so.

[bernhard-42]

This test breaks with the current dev code:

    def test_revolute_joint_absolute_locations(self):
        b1 = Box(10, 10, 1)
        b2 = Box(5, 5, 1)
        j1 = RigidJoint("j1", b1, Location((-4, -4, 0.5)))
        j2 = RevoluteJoint("j2", b2, Axis((2.5, 2.5, 0), (0, -1, 0)))

        j1.connect_to(j2, angle=0)

        a1 = j1.parent.location * j1.relative_location
        a2 = j2.parent.location * j2.relative_axis.location

        self.assertEqual(a1, a2)

btw. it would be good if Joints would have a location property that allows to write the test like:

    def test_revolute_joint_absolute_locations(self):
        b1 = Box(10, 10, 1)
        b2 = Box(5, 5, 1)
        j1 = RigidJoint("j1", b1, Location((-4, -4, 0.5)))
        j2 = RevoluteJoint("j2", b2, Axis((2.5, 2.5, 0), (0, -1, 0)))

        j1.connect_to(j2, angle=0)

        self.assertEqual(j1.location, j2.location)

[gumyr]

@bernhard-42 I did look at your branch. Thanks for all the work you've put into it but I was hoping to keep the existing design as I think it will be helpful for some of the future features I'm planning. It's super useful to have as a reference though.

I like the idea of a Joint location and I'll add the test you've provided.

[bernhard-42]

Your fix makes this test successful. However, I wanted to write a test with angle:

It has a bit of a surprising result:

a = Pos(0, 0, 3) * Box(1, 5, 5)

b1 = Box(10, 10, 1)
b2 = Box(5, 5, 1)
j1 = RigidJoint("j1", b1, Pos(-4, -4, 0.5))
j2 = RevoluteJoint("j2", b2, Axis((2.5, 2.5, 0), (0, -1, 0)))

angle = 30
j1.connect_to(j2, angle=angle)

a1 = j1.parent.location * j1.relative_location
a2 = j2.parent.location * j2.relative_axis.location

show(Rot(0, 0, angle) * a, b1, b2, render_joints=True)

returns

with angle = 0 they are oriented the same way.

So the rotation angle seems to behave differently for normal rotation (as expected counter clockwise) and when angle is provided

[bernhard-42]

I like the idea of a Joint location and I'll add the test you've provided.

And I think the absolute location assertion should be added as a default to every joint test

[bernhard-42]

This test also fails:

    def test_linear_revolute_joint(self):
        base = Box(10, 10, 2)
        arm = Box(2, 1, 2, align=(Align.CENTER, Align.CENTER, Align.MIN))

        base_top_edges = (
            base.edges().filter_by(Axis.X, tolerance=30).sort_by(Axis.Z)[-2:]
        )
        j1 = LinearJoint(
            "slot",
            base,
            axis=Edge.make_mid_way(*base_top_edges, 0.33).to_axis(),
            linear_range=(0, base_top_edges[0].length),
        )
        j2 = RevoluteJoint("pin", arm, axis=Axis.Z, angular_range=(0, 360))

        j1.connect_to(j2, position=6, angle=60)

        a1 = j1.parent.location * j1.relative_axis.location
        a2 = j2.parent.location * j2.relative_axis.location
        self.assertTupleAlmostEquals(list(a1.position), list(a2.position), 6)
        self.assertTupleAlmostEquals(list(a1.orientation), list(a2.orientation), 6)

btw. to be tolerant, I changed the assertion to assertTupleAlmostEqual

The main reason for thew failure is that the two joint axis are orthogonal after joining. While I understand that this is the result wanted, I feel it breaks the logic. I solved it by introducing a parameter called y_angle to rotate the aligned object around the y-axis. I think this is a bit of a design challenge ...

[bernhard-42]

Another thing you could take over is to show the x-axis in the symbol. I found that really helpful

[gumyr]

Your fix makes this test successful. However, I wanted to write a test with angle:

It has a bit of a surprising result:

a = Pos(0, 0, 3) * Box(1, 5, 5)

b1 = Box(10, 10, 1)
b2 = Box(5, 5, 1)
j1 = RigidJoint("j1", b1, Pos(-4, -4, 0.5))
j2 = RevoluteJoint("j2", b2, Axis((2.5, 2.5, 0), (0, -1, 0)))

angle = 30
j1.connect_to(j2, angle=angle)

a1 = j1.parent.location * j1.relative_location
a2 = j2.parent.location * j2.relative_axis.location

show(Rot(0, 0, angle) * a, b1, b2, render_joints=True)

returns

with angle = 0 they are oriented the same way.

So the rotation angle seems to behave differently for normal rotation (as expected counter clockwise) and when angle is provided

Interesting. The Axis might be in two different directions here so the angle or rotation is backwards? This is why I wanted more testing...

[gumyr]

Another thing you could take over is to show the x-axis in the symbol. I found that really helpful

Excellent idea

[gumyr]

def test_revolute_joint_absolute_locations(self):
        b1 = Box(10, 10, 1)
        b2 = Box(5, 5, 1)
        j1 = RigidJoint("j1", b1, Location((-4, -4, 0.5)))
        j2 = RevoluteJoint("j2", b2, Axis((2.5, 2.5, 0), (0, -1, 0)))

        j1.connect_to(j2, angle=0)

        self.assertEqual(j1.location, j2.location)

Got all the changes in (including adding a location property) but this test fails: AssertionError: (p=(-4.00, -4.00, 0.50), o=(-0.00, 0.00, -0.00)) != (p=(-4.00, -4.00, 0.50), o=(-0.00, 0.00, 0.00)) i.e -0.00 != 0.00 sigh .. now I have to look at location comparisons again :(

[gumyr]

The issue is finally fixed. In addition all the joints have a location property and I've enhanced the rotation based symbols to show the x direction.