Code review of feature script to create a wood joint

josh_crowson

I am working on learning FS.  I would appreciate any criticisms of the following code.  Feel free to say this would be faster, would have made the coding process easier... Anything will be appreciated.

Here is the code.  The code creates a joints between parts.  The user needs to select faces to have tabs, parts to be slotted, material thickness, and how many tabs are needed.  The code the calculates dimensions for the tabs and evenly spaces them out.  There is an offset in the code, but since I am writing this for my machine, it should not need to be changed.  The script could easily be altered to have the offset user defined.

FeatureScript 543;
import(path : "onshape/std/geometry.fs", version : "543.0");

annotation { "Feature Type Name" : "Rabbit Joint" }
export const rabbitJoint = defineFeature(function(context is Context, id is Id, definition is map)
    precondition
    {
        annotation { "Name" : "Tab Faces", "Filter" : EntityType.FACE, "MaxNumberOfPicks" : 100 }
        definition.tabFaces is Query;
        annotation { "Name" : "Long Edge", "Filter" : EntityType.EDGE, "MaxNumberOfPicks" : 1 }
        definition.longEdge is Query;
        annotation { "Name" : "Slot Bodies", "Filter" : EntityType.BODY, "MaxNumberOfPicks" : 100 }
        definition.slotBodies is Query;
        annotation { "Name" : "Tab Count" }
        isInteger(definition.tabCount, POSITIVE_COUNT_BOUNDS);
        annotation { "Name" : "Material Thickness" }
        isLength(definition.thickness, LENGTH_BOUNDS);


    }
    {
        var count = 0;
        const tabDepth = 0.5;
        const offsetDistance = 0.08 * inch;
        const facesList = evaluateQuery(context, definition.tabFaces);
        var offsets = qNothing();
        for (var face in facesList) //Iterates for every tab face selected.
        {
            count = count + 1;
            const plane = evPlane(context, { "face" : face });
            const sk = newSketchOnPlane(context, id + toString(count) + "sk", { "sketchPlane" : plane });
            //The following line computes the width based on area.  This feels like the worst portion, but I could not find a smarter way
            const width = evArea(context, { "entities" : face }) / definition.thickness;
            const lineUnrounded = evLine(context, { "edge" : definition.longEdge })["direction"];
            const line = [round(lineUnrounded[0]), round(lineUnrounded[1]), round(lineUnrounded[2])]; //rounds off the line so that I can determine if it is vertical/horizontal
            const spaces = 2 * definition.tabCount + 1;
            const spaceDist = width / spaces;
            var corner = vector(0, 0);
            var spaceVector = vector(spaceDist, definition.thickness);
            var offsetVector = vector(2 * spaceDist, 0 * inch);
            // The following if/else if statements create the corner, space, and offset vectors based on the orientation of the line
            if (line == [-1, 0, 0])
            {
                spaceVector = vector(spaceDist, definition.thickness);
                corner = vector(-width / 2, -definition.thickness / 2) + vector(spaceDist, 0 * inch);
                offsetVector = vector(2 * spaceDist, 0 * inch);
            }
            else if (line == [1, 0, 0])
            {
                spaceVector = vector(definition.thickness, spaceDist);
                corner = vector(-definition.thickness / 2, -width / 2) + vector(0 * inch, spaceDist);
                offsetVector = vector(0 * inch, 2 * spaceDist);
            }
            else if (line == [0, 1, 0])
            {
                spaceVector = vector(spaceDist, definition.thickness);
                corner = vector(-width / 2, -definition.thickness / 2) + vector(spaceDist, 0 * inch);
                offsetVector = vector(2 * spaceDist, 0 * inch);
            }
            else if (line == [0, -1, 0])
            {
                spaceVector = vector(spaceDist, definition.thickness);
                corner = vector(width / 2, definition.thickness / 2) + vector(spaceDist, 0 * inch);
                offsetVector = offsetVector(2 * spaceDist, 0 * inch);
            }
            else if (line == [0, 0, -1])
            {
                spaceVector = vector(definition.thickness, spaceDist);
                corner = vector(-definition.thickness / 2, -width / 2) + vector(0 * inch, spaceDist);
                offsetVector = vector(0 * inch, 2 * spaceDist);
            }
            else if (line == [0, 0, 1])
            {
                // not tested
                spaceVector = vector(definition.thickness, spaceDist);
                corner = vector(-definition.thickness / 2, -width / 2) + vector(0 * inch, spaceDist);
                offsetVector = vector(0 * inch, 2 * spaceDist);
            }

            for (var i = 0; i < definition.tabCount; i += 1)
            {
                skRectangle(sk, "rectangle1" ~ (count + i), { "firstCorner" : corner, "secondCorner" : corner + spaceVector });
                corner = corner + offsetVector;
            }

            skSolve(sk);
            // extrude the tabs
            extrude(context, id + toString(count) + "extrudeTabs", {
                        "entities" : qSketchRegion(id + toString(count) + "sk", true),
                        "endBound" : BoundingType.BLIND,
                        "depth" : definition.thickness * tabDepth,
                    });
            // extrude the offsets to be subtracted from the slot parts
            extrude(context, id + toString(count) + "extrudeForOffsets", {
                        "entities" : qSketchRegion(id + toString(count) + "sk", true),
                        "endBound" : BoundingType.BLIND,
                        "depth" : definition.thickness * tabDepth,
                    });
            // Boolean union tabs and tab parts
            opBoolean(context, id + toString(count) + "boolean", {
                        "tools" : qUnion([qOwnerBody(face), qCreatedBy(id + toString(count) + "extrudeTabs", EntityType.BODY)]),
                        "operationType" : BooleanOperationType.UNION,
                        "keepTools" : true
                    });
            //offset the tab offsets
            opOffsetFace(context, id + toString(count) + "offsetFace1", {
                        "moveFaces" : qCreatedBy(id + toString(count) + "extrudeForOffsets", EntityType.FACE),
                        "offsetDistance" : offsetDistance
                    });
            // subtract offsets from slot faces
            offsets = qUnion([offsets, qCreatedBy(id + toString(count) + "extrudeForOffsets", EntityType.BODY)]);
            opBoolean(context, id + toString(count) + "boolean2", {
                        "tools" : offsets,
                        "targets" : qUnion([definition.slotBodies]),
                        "operationType" : BooleanOperationType.SUBTRACTION,
                        "keepTools" : false
                    });
        }
    });

lana

@josh_crowson
Thank you for trying your hand in FeatureScript. Your code looks good over all. Couple suggestions below.
You can simplify your task by controlling sketch plane parametrization.

const facePlane = evPlane(context, { "face" : face });
const edgeLine = evLine(context, { "edge" : definition.longEdge }); 
const startPoint = evPoint(context, { "vertex" : definition.startVertex}); // you might want to control start vertex
const skPlane = plane(startPoint, facePlane.normal, edgeLine.direction);
const sk = newSketchOnPlane(context, id + toString(count) + "sk", { "sketchPlane" : skPlane });

In this case in sketch plane coordinates startPoint will correspond to [0, 0] * inch, edgeLine.direction will correspond to [1, 0].
I don't think you can implement this for multiple face selection, because longEdge has to be parallel to the face and startVertex should belong to the face for functionality to make sense.

another small thing:
For performance reasons you might want to use opExtrude instead of extrude and instead of calling extrude twice to create identical geometry, use opPattern() with 1 instance and identityTransform() to make a copy (see example call in boolean.fs). In fact in this case it makes sense to call booleanBodies() with offset and keepTools.

josh_crowson

Thank you.  It does work with multiple face selection for all faces whose longer edge has the same length and direction vector.  
I tried getting opExtrude to work, but I am getting "@opExtrude: Expected vector (a non-empty array), value is undefined".  When I was writing the script I could not figure out booleanBodies() ether.

Here is a link  where I am using the script.  I call the script 3 separate times.
https://cad.onshape.com/documents/8b1842e8e78d7edeae1bcdc2/w/5d4631b24248d8c0b0248470/e/c3a48875412c50fb03be6b13

lana

I see, as long as you know the restriction on input, it will work.

I suspect, opExtrude was complaining about "direction" vector. With FS op-functions, it is a good idea to let the IDE to  generate the default call, that'll give you an idea of all the parameters expected.

opExtrude(context, id + "extrude1", {
               "entities" : entities,
               "direction" : evOwnerSketchPlane(context, {"entity" : entities}).normal,
               "endBound" : BoundingType.BLIND,
               "endDepth" : 1 * inch
       });

For features you can try adding the feature you want in part studio , then show Code of part studio and use the corresponding call as your template. It will need a lot of cleanup

booleanBodies(context, id + "F4cf3QKyKQOv2VS_2", { "operationType" : BooleanOperationType.SUBTRACTION, "tools" : qUnion([vckvWgxKnvUWxr_query]), "targets" : qUnion([HcNyApbtKOMxlF_query]), "offset" : true, "offsetAll" : true, "entitiesToOffset" : qUnion([]), "offsetDistance" : { 'value' : try(2.5 * millimeter), 'expression' : "2.5 mm" }.value, "oppositeDirection" : false, "reFillet" : false, "keepTools" : true });

josh_crowson

Is there a better way to determine the length and width of a face than this line of code?

//The following line computes the width based on area.  This feels like the worst portion, but I could not find a smarter way
const width = evArea(context, { "entities" : face }) / definition.thickness;

Jake_Rosenfeld

If you can assembly a Query for the line in question, you can just use

evLength(context, { "entities" : lineQuery } );