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.

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

InPlane constraint #712

Merged
merged 10 commits into from
Apr 13, 2021
Merged

InPlane constraint #712

merged 10 commits into from
Apr 13, 2021

Conversation

marcus7070
Copy link
Member

I've added a constraint that positions a point within a 2D plane.

Proper docs are coming in a later PR, but I want to update SphinxCadquery to handle assemblies first.

@codecov
Copy link

codecov bot commented Apr 3, 2021

Codecov Report

Merging #712 (55535a0) into master (39e60f6) will increase coverage by 0.07%.
The diff coverage is 96.02%.

❗ Current head 55535a0 differs from pull request most recent head 8d9ffbc. Consider uploading reports for the commit 8d9ffbc to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #712      +/-   ##
==========================================
+ Coverage   94.51%   94.59%   +0.07%     
==========================================
  Files          32       32              
  Lines        7078     7215     +137     
  Branches      768      785      +17     
==========================================
+ Hits         6690     6825     +135     
- Misses        254      255       +1     
- Partials      134      135       +1     
Impacted Files Coverage Δ
cadquery/occ_impl/geom.py 89.08% <81.81%> (+0.12%) ⬆️
cadquery/occ_impl/solver.py 90.55% <86.36%> (+0.64%) ⬆️
cadquery/assembly.py 91.07% <93.75%> (+0.53%) ⬆️
tests/test_assembly.py 100.00% <100.00%> (ø)
tests/test_cadquery.py 99.18% <100.00%> (+<0.01%) ⬆️
tests/test_workplanes.py 99.30% <0.00%> (-0.70%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 39e60f6...8d9ffbc. Read the comment docs.

@marcus7070 marcus7070 marked this pull request as draft April 4, 2021 03:21
@marcus7070
Copy link
Member Author

Codecov pointed out I only test when the plane is locked and the point is free, I should test the other way around too.

@marcus7070
Copy link
Member Author

Codecov pointed out I only test when the plane is locked and the point is free, I should test the other way around too.

At the moment this constraint requires the first object to be a plane (or at least quack like one), and the second object to be a point. I started changing the code so the constraint could be specified either way around but I didn't like what I was writing. I would have to type check to figure out which argument is a plane, and in doing so prevent the use case of constraining the centre of one face to the plane formed by another face. It was also ugly code.

At the moment the responsibility is on the user to specify the plane as the first object and the point as the second.

I'll keep it in the back of my head and see if I can come up with a better way to write these non-symetrical constraints (plane/point vs the symetrical axis/axis and point/point constraints).

@marcus7070 marcus7070 added assembly enhancement New feature or request labels Apr 4, 2021
@marcus7070
Copy link
Member Author

BTW @jmwright, I think this constraint does what you expected "Plane" to do back in #479?

@marcus7070 marcus7070 marked this pull request as ready for review April 5, 2021 06:04
@jmwright
Copy link
Member

jmwright commented Apr 5, 2021

BTW @jmwright, I think this constraint does what you expected "Plane" to do back in #479?

Yes, this seems like what I was looking for. I'm not sure when I'll get a chance to do it, but I plan to go back to your spindle assembly and retry mounting the spindle base plate to the aluminum extrusion with this constraint in place.

https://raw.githubusercontent.com/marcus7070/spindle-assy-example/master/screenshot.png

Melding both the way I would do it in something like SolidWorks and CadQuery's existing constraints, I expect this is what I'll try.

  1. inPlane of "<Y" on extrusion and ">Y" on base plate
  2. inPlane of "<Z" on extrusion and "<Z" on base plate
  3. Use translate to create an offset for the base plate until it sort of looks like it's in the correct position away from the end of the extrusion
  4. Create an axial constraint to (hopefully) center the base plate in the X axis in relation to the extrusion - not sure if this will work

@jmwright
Copy link
Member

jmwright commented Apr 6, 2021

Got a chance to play around with this a little bit and I like it. I expect I will use this constraint constantly when working with assemblies in CQ. I created the following simplified assembly to accomplish what caused my confusion in #479

import cadquery as cq

block1 = cq.Workplane("XY").box(200, 100, 20)
block2 = cq.Workplane("XY").box(50, 100, 10)

assy = cq.Assembly()

assy.add(block1, name="block1", color=cq.Color(1.0, 0, 0, 1.0))
assy.add(block2, name="block2", color=cq.Color(0.0, 0, 1, 1.0))

assy.constrain("block1@faces@>Z", "block2@faces@<Z", "InPlane", param=0)
assy.constrain("block1@faces@>X", "block2@faces@>X", "InPlane", param=20)
assy.constrain("block1@faces@<X", "block2@faces@>X", "Axis")

assy.solve()

show_object(assy)

Results in:

Screenshot from 2021-04-06 09-01-21

I do have two questions though.

  • Why is param named that, instead of something like offset?
  • What if I want to use a negative param, say to have one part overhang another? In the example below I've arranged the Y face inPlane constraint in such a way that I "push" the block2 off the the other side, but what if I wanted to apply a negative offset to "pull" it to hang off the opposite edge? Or if I want two plates to face each other, but to be floating 5mm apart? This is probably a moot point because you should always be able to set the constraint up to achieve the desired effect, but I thought I'd bring it up anyway to be thorough.
import cadquery as cq

block1 = cq.Workplane("XY").box(200, 100, 20)
block2 = cq.Workplane("XY").box(50, 80, 10)

assy = cq.Assembly()

assy.add(block1, name="block1", color=cq.Color(1.0, 0, 0, 1.0))
assy.add(block2, name="block2", color=cq.Color(0.0, 0, 1, 1.0))

assy.constrain("block1@faces@>Z", "block2@faces@<Z", "InPlane", param=0)
assy.constrain("block1@faces@>X", "block2@faces@>X", "InPlane", param=20)
assy.constrain("block1@faces@>Y", "block2@faces@>Y", "InPlane", param=30)
#assy.constrain("block1@faces@<X", "block2@faces@>X", "Axis")

assy.solve()

show_object(assy)

Screenshot from 2021-04-06 09-09-54

Copy link
Member

@jmwright jmwright left a comment

Choose a reason for hiding this comment

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

Looks great, thanks @marcus7070 !

cadquery/assembly.py Outdated Show resolved Hide resolved
@marcus7070
Copy link
Member Author

  • What if I want to use a negative param, say to have one part overhang another? In the example below I've arranged the Y face inPlane constraint in such a way that I "push" the block2 off the the other side, but what if I wanted to apply a negative offset to "pull" it to hang off the opposite edge? Or if I want two plates to face each other, but to be floating 5mm apart? This is probably a moot point because you should always be able to set the constraint up to achieve the desired effect, but I thought I'd bring it up anyway to be thorough.

If you have a look at the function that is minimized:

return (
val - (m1.Transformed(t1).XYZ() - m2.Transformed(t2).XYZ()).Modulus()
) ** 2

(param winds up as val in that), the solver shuffles around the locations until that returned value is minimized (normally it winds up as 0). So when it comes to distance relationships it is an offset, but it doesn't have any inherent direction, so I think with pnt to pnt constraint you wind up with a sphere around the fixed pnt where the function would return 0.

So InPlane with a param != 0 would be minimised at two parallel planes, offset param from the argument. To make sure my part winds up in the plane I want, I position the part roughly (but definitely on the correct side) before solving (with a location argument in Assembly.add) and trust in the solver to hit the nearest minimum and not jump over it to the other side.

An alternative is to use the 5 argument Assembly.constrain (instead of 3 string arguments), where you can do things like part.vertices(">X and >Y and >Z").val().translate((5, 0, 0)).

@marcus7070
Copy link
Member Author

In case there is any misunderstanding I'll just make this clear.

assy.constrain("block1@faces@>Z", "block2@faces@<Z", "InPlane", param=0)
assy.constrain("block1@faces@>X", "block2@faces@>X", "InPlane", param=20)
assy.constrain("block1@faces@>Y", "block2@faces@>Y", "InPlane", param=30)

I'm planning to make this quite clear in the docs, but the InPlane relationship is between a plane and a point. I've just duck typed it so that the faces you are selecting on block2 can be represented as a point.

@adam-urbanczyk
Copy link
Member

Still need to read the code but three general things pop up.

  1. given the naming I'd expect first thing to be "point" and the second thing to be plane ( ... inPlane)
  2. wires seem not to be supported, though you can [often] construct a plane from a wire
  3. I'd propose to make the param sign sensitive - you can offset the plane by d*normal and optimize to zero

@marcus7070 marcus7070 marked this pull request as draft April 8, 2021 05:55
@marcus7070
Copy link
Member Author

marcus7070 commented Apr 8, 2021

  1. given the naming I'd expect first thing to be "point" and the second thing to be plane ( ... inPlane)

Sure, the order was always pretty arbitrary.

This was in the back of my mind while writing this PR: the order of arguments in a constraint is important, right? eg.

assy = cq.Assembly()

assy.add(cq.Workplane().box(1, 1, 1), name="box")
assy.add(cq.Workplane().sphere(1), name="sphere")

assy.constrain(
    "sphere",
    cq.Vertex.makeVertex(0, 0, 1),
    "box",
    cq.Vertex.makeVertex(0, 0, 0),
    "Point",
)

assy.solve()
show_object(assy)

screenshot2021-04-08-153430

But when you flip the constraint order to:

assy.constrain(
    "box",
    cq.Vertex.makeVertex(0, 0, 0),
    "sphere",
    cq.Vertex.makeVertex(0, 0, 1),
    "Point",
)

screenshot2021-04-08-153516

So a user could need the reverse of this constraint, ie. "PointOnPlane" and "PlaneOnPoint".

  1. wires seem not to be supported, though you can [often] construct a plane from a wire

Thanks, I had planned to do wires but forgot.

Should I extend Constraint._getAxis to also handle Wires? edit: decided not to do this

  1. I'd propose to make the param sign sensitive - you can offset the plane by d*normal and optimize to zero

Oh, so I can! That's going to be so much better, thanks! 🚀

@marcus7070
Copy link
Member Author

marcus7070 commented Apr 9, 2021

I now handle Wires and introduced tests for _getPlane.

I decided to make a plane from any Face (as opposed to checking for planar Faces first). Not 100% confident in that descision, let me know if you have any opinions.

Still to do:

  • Use signed param
  • Think about how to do the "PointOnPlane"/"PlaneOnPoint" without repeating code, and considering there will probably be more "non-symetrical" constraints coming along later.

@marcus7070
Copy link
Member Author

I said above:

This was in the back of my mind while writing this PR: the order of arguments in a constraint is important, right? eg.
...
So a user could need the reverse of this constraint, ie. "PointOnPlane" and "PlaneOnPoint".

I now think the order of arguments to constraints is not important.

What was happening in my examples was that I was starting with an empty assembly (assy = cq.Assembly()), so the first entity of the first constraint was set to locked by:

def solve(self) -> "Assembly":
"""
Solve the constraints.
"""
# get all entities and number them
ents = {}
i = 0
lock_ix = 0
for c in self.constraints:
for name in c.objects:
if name not in ents:
ents[name] = i
if name == self.name:
lock_ix = i
i += 1

If the user wants a part to be locked, then they should specify it like assy = cq.Assembly(part_to_be_locked, name="part"), and then the order of constraint arguments shouldn't matter and there is no need for a "PlaneOnPoint" constraint.

@marcus7070 marcus7070 marked this pull request as ready for review April 10, 2021 06:15
@marcus7070
Copy link
Member Author

I think I'm done now, so ready for you to look at @adam-urbanczyk. @jmwright you can check out the changes if you like.

@marcus7070 marcus7070 merged commit b0a8cb2 into CadQuery:master Apr 13, 2021
@marcus7070 marcus7070 deleted the more-constraints branch April 14, 2021 01:42
@adam-urbanczyk adam-urbanczyk mentioned this pull request Nov 3, 2021
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assembly enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants