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

🐛 Fix automatic pitch calculation in ScrewHole() #3

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

nobodyinperson
Copy link

As ScrewThread() is called in ScrewHole() with scaled values for
outer_diam, the ThreadPitch() function in ScrewThread() outputs
non-matching values. Calculating the pitch independently fixes it.

As `ScrewThread()` is called in `ScrewHole()` with scaled values for
`outer_diam`, the `ThreadPitch()` function in `ScrewThread()` outputs
non-matching values. Calculating the pitch independently fixes it.

- Fixes rcolyer#1
This is much less resource intensive when setting $fs to a smaller value
in preview mode.
@rcolyer
Copy link
Owner

rcolyer commented Dec 1, 2021

On the commit about screw_resolution, this sort of usage of $fs (as a dynamic scope variable) in used libraries is not properly defined, and will not work in this manner in the next OpenSCAD release. It could be done by replacing it with a function call, but this is not a good idea for this library without also overriding the default $fs variable in some manner, because the default $fs is 2, and will cause almost every thread of interest to be a catastrophic failure under the default usage of the library. One possible resolution to this is to replace screw_resolution with a function that returns 0.2 if $fs is 2 or greater, otherwise it returns $fs. But this has a slight weakness as well because it has become a convention to set $fs to 0.4 on the most common 0.4mm extruder FDM printers, whereas threads are always vertical and there is some benefit to matching them to the layer height.

Perhaps the right answer is to change the top line to default_screw_resolution and then move screw_resolution into ScrewThread with a line like:
screw_resolution = is_undef($screw_resolution) ? default_screw_resolution : $screw_resolution;

That would still yield full user control, while not conflicting with established usage patterns.

ellemenno added a commit to ellemenno/threads-scad that referenced this pull request Jun 19, 2022
ellemenno added a commit to ellemenno/threads-scad that referenced this pull request Jun 19, 2022
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.

Automatic pitch calculation in ScrewHole is broken
2 participants