Add STEP Import for Assemblies #1779
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1779 +/- ##
========================================
Coverage 95.66% 95.67%
========================================
Files 28 29 +1
Lines 7431 7538 +107
Branches 1122 1140 +18
========================================
+ Hits 7109 7212 +103
- Misses 193 194 +1
- Partials 129 132 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@adam-urbanczyk In 4045c58 you set the We need to decide on the proper way to fix this. |
|
Hey, I do not understand the issue. You can always do this: Do you mean that you need to modify |
|
Correct, mypy complains. I can make that change to make |
…king through indirect lookup
|
@adam-urbanczyk The gold color of the v-slot parts above is because there is no color defined for those components, and @lorenzncode @adam-urbanczyk Can you provide the steps to reproduce the location issue with the constrainted door assembly? I cannot reproduce, so we are following different processes somehow. The code below has all the components in the correct location when I show it using import cadquery as cq
from cadquery.occ_impl.exporters.assembly import exportStepMeta
from cadquery.vis import show
# Parameters
H = 400
W = 200
D = 350
PROFILE = cq.importers.importDXF("vslot-2020_1.dxf").wires()
SLOT_D = 6
PANEL_T = 3
HANDLE_D = 20
HANDLE_L = 50
HANDLE_W = 4
def make_vslot(l):
return PROFILE.toPending().extrude(l)
def make_connector():
rv = (
cq.Workplane()
.box(20, 20, 20)
.faces("<X")
.workplane()
.cboreHole(6, 15, 18)
.faces("<Z")
.workplane(centerOption="CenterOfMass")
.cboreHole(6, 15, 18)
)
# tag mating faces
rv.faces(">X").tag("X").end()
rv.faces(">Z").tag("Z").end()
return rv
def make_panel(w, h, t, cutout):
rv = (
cq.Workplane("XZ")
.rect(w, h)
.extrude(t)
.faces(">Y")
.vertices()
.rect(2 * cutout, 2 * cutout)
.cutThruAll()
.faces("<Y")
.workplane()
.pushPoints([(-w / 3, HANDLE_L / 2), (-w / 3, -HANDLE_L / 2)])
.hole(3)
)
# tag mating edges
rv.faces(">Y").edges("%CIRCLE").edges(">Z").tag("hole1")
rv.faces(">Y").edges("%CIRCLE").edges("<Z").tag("hole2")
return rv
def make_handle(w, h, r):
pts = ((0, 0), (w, 0), (w, h), (0, h))
path = cq.Workplane().polyline(pts)
rv = (
cq.Workplane("YZ")
.rect(r, r)
.sweep(path, transition="round")
.tag("solid")
.faces("<X")
.workplane()
.faces("<X", tag="solid")
.hole(r / 1.5)
)
# tag mating faces
rv.faces("<X").faces(">Y").tag("mate1")
rv.faces("<X").faces("<Y").tag("mate2")
return rv
# define the elements
door = (
cq.Assembly()
.add(make_vslot(H), name="left")
.add(make_vslot(H), name="right")
.add(make_vslot(W), name="top")
.add(make_vslot(W), name="bottom")
.add(make_connector(), name="con_tl", color=cq.Color("black"))
.add(make_connector(), name="con_tr", color=cq.Color("black"))
.add(make_connector(), name="con_bl", color=cq.Color("black"))
.add(make_connector(), name="con_br", color=cq.Color("black"))
.add(
make_panel(W + 2 * SLOT_D, H + 2 * SLOT_D, PANEL_T, SLOT_D),
name="panel",
color=cq.Color(0, 0, 1, 0.2),
)
.add(
make_handle(HANDLE_D, HANDLE_L, HANDLE_W),
name="handle",
color=cq.Color("yellow"),
)
)
# define the constraints
(
door
# left profile
.constrain("left@faces@<Z", "con_bl?Z", "Plane")
.constrain("left@faces@<X", "con_bl?X", "Axis")
.constrain("left@faces@>Z", "con_tl?Z", "Plane")
.constrain("left@faces@<X", "con_tl?X", "Axis")
# top
.constrain("top@faces@<Z", "con_tl?X", "Plane")
.constrain("top@faces@<Y", "con_tl@faces@>Y", "Axis")
# bottom
.constrain("bottom@faces@<Y", "con_bl@faces@>Y", "Axis")
.constrain("bottom@faces@>Z", "con_bl?X", "Plane")
# right connectors
.constrain("top@faces@>Z", "con_tr@faces@>X", "Plane")
.constrain("bottom@faces@<Z", "con_br@faces@>X", "Plane")
.constrain("left@faces@>Z", "con_tr?Z", "Axis")
.constrain("left@faces@<Z", "con_br?Z", "Axis")
# right profile
.constrain("right@faces@>Z", "con_tr@faces@>Z", "Plane")
.constrain("right@faces@<X", "left@faces@<X", "Axis")
# panel
.constrain("left@faces@>X[-4]", "panel@faces@<X", "Plane")
.constrain("left@faces@>Z", "panel@faces@>Z", "Axis")
# handle
.constrain("panel?hole1", "handle?mate1", "Plane")
.constrain("panel?hole2", "handle?mate2", "Point")
)
# solve
door.solve()
success = exportStepMeta(door, "door.step")
if success:
imported_assy = cq.Assembly.importStep("door.step")
show(imported_assy) |
Try door.export("door.step")STEP is exported in the assembly tests. I'm getting a run the pytest, export step (still uses old save method) This fails with ValueError: from cadquery import Assembly
obj = Assembly.importStep("./tmp/assembly10/chassis0_assy.step") |
|
Here is mine: before: I also found another issues, without copies the import throws. See last section of the snippet. #%% imports
from cadquery import Assembly, Location, Color
from cadquery.func import box, rect
from cadquery.vis import show
#%% prepare the model
def make_model(name: str, COPY: bool):
b = box(1,1,1)
assy = Assembly(name='test_assy')
assy.add(box(1,2,5), color=Color('green'))
for v in rect(10,10).vertices():
assy.add(
b.copy() if COPY else b,
loc=Location(v.Center()),
color=Color('red')
)
# show(assy)
assy.export(name)
return assy
assy_copy = make_model("test_assy_copy.step", True)
assy = make_model("test_assy.step", False)
show(assy)
#%% import the assy with copies
assy_i = Assembly.importStep("test_assy_copy.step")
show(assy_i)
#%% import the assy without copies - this throws
assy_i = Assembly.importStep("test_assy.step")
show(assy_i) |
|
@adam-urbanczyk @lorenzncode The parent location was not being applied correctly when the original assembly.export method is used. I've fixed that now and pulled in the srgb changes from master, so I think the location and color issues should be fixed. |
|
Do you plan to fix the second issue I found? |
|
@adam-urbanczyk Yes, I'll see what I can figure out. |
|
@adam-urbanczyk The
In the screenshot: Two variations of the name, one with the |
|
This tool (eDrawings) can import the problematic STEP without copies and the structure LGTM. I think that CQ should be able to handle this as well (and produce identical structure).
|
|
How do we do that without breaking things like constraints that use part names? We would have to remove the |
|
But subassy names are unique and parts don't have names in cq AFAIR. |
|
Additional observation: In [3]: assy_i.objects
Out[3]:
{'a9a08701-4ac0-11f0-a634-c86e08a4b6ef': <cadquery.assembly.Assembly at 0x1b1cf1dcbc0>,
'a7644761-4ac0-11f0-bcb0-c86e08a4b6ef_part': <cadquery.assembly.Assembly at 0x1b1d123ab10>,
'a76594bb-4ac0-11f0-9013-c86e08a4b6ef_part': <cadquery.assembly.Assembly at 0x1b1d1238440>,
'a7668644-4ac0-11f0-91c5-c86e08a4b6ef_part': <cadquery.assembly.Assembly at 0x1b1d123ac30>,
'a7668645-4ac0-11f0-9a6c-c86e08a4b6ef_part': <cadquery.assembly.Assembly at 0x1b1d123ad20>,
'a7668646-4ac0-11f0-abef-c86e08a4b6ef_part': <cadquery.assembly.Assembly at 0x1b1d123b680>}
In [4]: assy.objects
Out[4]:
{'test_assy': <cadquery.assembly.Assembly at 0x1b1d11fcc20>,
'a77ae412-4ac0-11f0-804d-c86e08a4b6ef': <cadquery.assembly.Assembly at 0x1b1d123a090>,
'a77ae413-4ac0-11f0-ab48-c86e08a4b6ef': <cadquery.assembly.Assembly at 0x1b1d123a480>,
'a77ae414-4ac0-11f0-b508-c86e08a4b6ef': <cadquery.assembly.Assembly at 0x1b1d123a510>,
'a77ae415-4ac0-11f0-b410-c86e08a4b6ef': <cadquery.assembly.Assembly at 0x1b1d123a5d0>,
'a77ae416-4ac0-11f0-b4a6-c86e08a4b6ef': <cadquery.assembly.Assembly at 0x1b1d123a690>}It seems that the name is taken from the wrong level of the tree. |
|
It turns out that the
I need to create another PR to fix nesting and name handling in I think there are still issues lurking with having duplicate names, but I will fix the name handling in this PR and make sure the nesting works properly. Then I can worry about the unique naming issues. |
|
I see that you are working on #1858, but wouldn't it be better to fix the issue here iso adding (before the release) more functionality? I'd propose to add a heuristic that detect nodes with one child and |
|
The structure looks the same now on the test case that used to fail. The first screenshot is the one that was exported, then the second is that STEP imported, then re-exported.
|
|
OK, I'll take one final look. |
|
|
||
| # Process the color for the shape, which could be of different types | ||
| color = Quantity_Color() | ||
| cq_color = cq.Color(0.50, 0.50, 0.50) |
There was a problem hiding this comment.
What are the magic parameters?
There was a problem hiding this comment.
They are the RGB values that most of the CQ visualization methods seem to be using as a default with assemblies (gray/silver). The values are not standardized as cadquery.vis.show seems to use a gold color IIRC. CQ-editor uses the gray color though.
There was a problem hiding this comment.
OK, default color should be None. See:
class Assembly(object):
"""Nested assembly of Workplane and Shape objects defining their relative positions."""
loc: Location
name: str
color: Optional[Color]
metadata: Dict[str, Any]
obj: AssemblyObjects
parent: Optional["Assembly"]
children: List["Assembly"]
objects: Dict[str, "Assembly"]
constraints: List[Constraint]| loc=cq.Location(parent_location), | ||
| ) | ||
| else: | ||
| assy.add(cq.Shape.cast(shape), name=name, color=cq_color) |
There was a problem hiding this comment.
Why isn't this covered. Is parent location not optional?
| # Pass down the location or other context as needed | ||
| # Add the parent location if it exists | ||
| if parent_location is not None: | ||
| loc = parent_location * loc |
There was a problem hiding this comment.
Isn't this suspicious. Assy applies locations in a relative sense. It shouldn't be needed to construct absolute locations.
There was a problem hiding this comment.
I'll have to check, but I think I added this because of nested sub-assemblies
There was a problem hiding this comment.
Confirmed. If the parent location is not passed through the call stack, nested subassemblies will break. I added the test_nested_subassembly_step_import test to illustrate this. If you disable parent location passing and run that test, it should fail.
There was a problem hiding this comment.
OK, but that is actually suspicious. When you construct an assy you do not need combine (i.e. multiply) the locations explicitly.
|
I am still getting a ValueError with a STEP from the tests. |
|
@jmwright any updates on this PR? |
|
@adam-urbanczyk Not yet. I got busy with some other things. |
The goal is to make it possible to round-trip assemblies to and from STEP without loss of data. This data can include: