Skip to content

Conversation

@bweissinger
Copy link
Contributor

Adds basic functionality for creating slots. It uses the current point as the center point for the slot. Length is the overall end to end length of the slot, with direction set by angle in degrees. Addresses #158

@codecov
Copy link

codecov bot commented Sep 4, 2019

Codecov Report

Merging #186 into master will increase coverage by 0.03%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #186      +/-   ##
==========================================
+ Coverage   94.82%   94.86%   +0.03%     
==========================================
  Files          19       19              
  Lines        4116     4144      +28     
==========================================
+ Hits         3903     3931      +28     
  Misses        213      213
Impacted Files Coverage Δ
cadquery/cq.py 93.2% <100%> (+0.13%) ⬆️
tests/TestCadQuery.py 98.94% <100%> (+0.01%) ⬆️

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 b0b9959...c378d49. Read the comment docs.

@adam-urbanczyk
Copy link
Member

Thanks a lot @bweissinger ! Some minor comments in the review.

@bweissinger
Copy link
Contributor Author

@adam-urbanczyk @jmwright Added changes to address noted issues.

@jmwright
Copy link
Member

jmwright commented Sep 6, 2019

@bweissinger Looks good. Thanks for implementing this.

@adam-urbanczyk
Copy link
Member

Thanks for you work @bweissinger . Before we merge it, I think we need to still discuss the naming. I realized that slot could imply 3D feature. Do you know how other CAD packages name such a 2D thing?

@bweissinger
Copy link
Contributor Author

Thanks for you work @bweissinger . Before we merge it, I think we need to still discuss the naming. I realized that slot could imply 3D feature. Do you know how other CAD packages name such a 2D thing?

Good catch.

As far as actual function names, the only package I have any idea on would be Freecad. Looking at the Freecad console, the slot sketch command seems to be named Sketcher_CreateSlot.

In my experience, slot usually refers to a 2D sketch feature. If a CAD package does use 3D slots, they don't seem to have a 2D slot features. A notable exception is Solid Works. It has both, but the 3D feature is actually located under Hole Wizard. I think it just depends on the function/industry the software is oriented towards. For instance, Catia has (as far as I know) only a 3D slot function, and it actually uses a path and a profile to cut along that path.

Unfortunately, the above programs all have a GUI to provide context. They could have both 2D and 3D, and name them both slot without much issue. However, 2D sketch slots tend to be named after the way they are defined:

  • Center to Center Slot
  • Center Point Slot
  • Arc Slot
  • ...

With everything above in mind, a couple of possibilities includes:

  • sketchSlot
  • centerPointSlot

@adam-urbanczyk
Copy link
Member

@jmwright any thoughts on the name?

@jmwright
Copy link
Member

@adam-urbanczyk I'm not sure. sketchSlot would be a break from how we name 2D operations, but it's probably the most accurate, and I think this is a little bit of a different case than we've had before with 2D operations. If it's named sketchSlot or something like slot2D, the user could still call some sort of 3D *Slot operation later that creates the 3D version, cut from the selected face.

@HLevering
Copy link
Contributor

@jmwright I think in AutoCad there is a similar feature called "RoundedRect", but where there is one more degree of freedom namely the height of the rect. Our "slot" would be a special case of a rounded rect, where H = 2 * R. Maybe be it makes sense to generalize our slot to a rounded rect, which can be parameterized by width, height, edge_radius and where height defaults to 2 times egde_radius when not given by the user.

@adam-urbanczyk
Copy link
Member

I looked at the newest FreeCAD and in the sketcher UI I can see a Create Slot tool. One day we might want to expose the OCC 3D slot feature so I'm in favor of renaming this to slot2D.

@Hatatister you can create a rounded rectangle by using rect, extruding it and filleting the edges. This is not possible for a slot (due to OCC kernel limitations).

@bweissinger
Copy link
Contributor Author

Slot2D sounds good to me as well.

@adam-urbanczyk
Copy link
Member

I think it is ready to be merged. Anything to add @jmwright @dcowden ?

@dcowden
Copy link
Member

dcowden commented Sep 16, 2019

@adam-urbanczyk looks great to me!

@jmwright
Copy link
Member

+1

@adam-urbanczyk adam-urbanczyk merged commit bb70734 into CadQuery:master Sep 17, 2019
@adam-urbanczyk
Copy link
Member

Thanks for your contribution @bweissinger !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants