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

Add Gridfinity Refined thumbscrew option #226

Conversation

andrewguy
Copy link
Contributor

I've implemented the thumbscrew option from Gridfinity Refined (https://www.printables.com/model/413761-gridfinity-refined), enabling an optional screw hole in the middle of each base unit.

I did this for my own benefit, but I noticed there was an open issue for this (#195).

I'm very new to OpenSCAD (only been a couple of days), so I may not have the best approach here, but it seems to work.

I've pulled in Ryan Colyer's threads-scad library to handle this - https://github.com/rcolyer/threads-scad. Included as a top-level threads.scad file. Not sure if this is how you want to handle library dependencies?

These thumbscrew holes print well on a Bambu Lab A1 mini with Bambu PLA Basic, and the thumbscrew file provided by Gridfinity Refined work well (engages well, easy to insert and minimal slack).

gridfinity_thumbscrew

@EmperorArthur
Copy link
Collaborator

We need to discuss how we want to handle libraries.

There are four major options I can think of:

  • Require the user to manually have it. -- Obviously bad idea.
  • Include it in something like an "External" folder. -- Library updates involve copying files.
  • Use git submodules. -- Need to add extra instructions for people pulling down code.
  • Do not allow 3rd party libraries.

If 3rd party libraries are allowed, we will need to make sure they are compatible license wise.

So far I have deliberately stayed away from libraries, but if we do decide to use them, then I suggest we use BSOL or BSOL2. Either would replace both this threading library, and a significant amount of helper code.

Changes Needed

Regardless, there will need to be changes to this PR. Even if it is just putting the library in a separate folder, and making a note of where to get the latest version.

@EmperorArthur EmperorArthur self-requested a review October 4, 2024 05:02
Copy link
Collaborator

@EmperorArthur EmperorArthur left a comment

Choose a reason for hiding this comment

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

I like the idea, but there are a few tweaks I'd like to see.

The big thing is we need to determine how libraries are handled.

threads.scad Outdated Show resolved Hide resolved
gridfinity-rebuilt-utility.scad Outdated Show resolved Hide resolved
gridfinity-rebuilt-utility.scad Show resolved Hide resolved
gridfinity-rebuilt-utility.scad Show resolved Hide resolved
gridfinity-rebuilt-utility.scad Show resolved Hide resolved
@andrewguy
Copy link
Contributor Author

Thanks @EmperorArthur for the feedback.

I've refactored into a call to ScrewHole(), although this introduces a few more 'magic numbers' that come from the threads.scad code (i.e. 1.01*outer_diam + 1.25*tolerance, presuming this introduces a bit of slack for threading the screw). Worth it I think.

I've moved the threads.scad library into /external/threads-scad/. In terms of licence, threads.scad is released under a CC0-1.0 license, which means it should be fine to include here.

In terms of handling external libraries, the only other option which would allow this pull request to go through would be to use Git submodules. I'd be concerned that submodules would lead to more trouble than they're worth (e.g. people need to use --recurse-submodule if doing a git clone, which often isn't obvious).

If the decision is made to not use external libraries, obviously this PR is a non-starter, which is fine.

@EmperorArthur
Copy link
Collaborator

@andrewguy You'll want to set up pre-commit. It will fix the automatic checks failing. OpenSCAD doesn't have an option to trim white-space on empty lines, but adds it there by default. :(

Copy link
Collaborator

@EmperorArthur EmperorArthur left a comment

Choose a reason for hiding this comment

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

Personally I'm more of a fan of git submodules, but for something like this that doesn't change often I don't see a problem with it. I also brought up the license as a future thing, not something to worry about right now.

Approving this, since my fix to #225 will have conflicts with this code, and I don't want to trash your hard work.

thumbscrew_height = 5;
thumbscrew_tolerance = 0.4;
thumbscrew_tooth_angle = 30;
thumbscrew_pitch = 1.5;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd prefer these in standard.scad, but will move them myself.

@EmperorArthur
Copy link
Collaborator

@andrewguy Since I'm not a maintainer I can't edit this pull request, and I'm not allowed to manually merge it.

Could you please fix the whites-pace issue so the checks pass. I'll merge this once that's done. You could also squash your commits, or I can have the merge do it.

@andrewguy
Copy link
Contributor Author

Whitespace is fixed and all the pre-commit checks pass. If you could squash on merge that would be great - that's my normal strategy, and I don't want to break something by trying to squash an existing branch. 😅

Thanks for the review and being happy to integrate this. Appreciate it.

@EmperorArthur EmperorArthur merged commit 25b988d into kennetek:main Oct 8, 2024
1 check passed
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.

2 participants