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

Added nut_sides parameter to allow creation of non-cylinder nut #83

Merged
merged 2 commits into from
Jan 27, 2024

Conversation

linkian209
Copy link
Contributor

Added a new parameter to the nut module that allows the creation of non cylinder nuts.

nut("M30", turns=3, Douter=46, nut_sides=6);

Would yield this:
image

@adrianschlatter
Copy link
Owner

Hi @linkian209

Thanks for your PR. Do I understand correctly that the nut_sides argument does exactly the same thing as the fn argument?

@linkian209
Copy link
Contributor Author

Do I understand correctly that the nut_sides argument does exactly the same thing as the fn argument?

It is the $fn argument specifically for the outer cyclinder. I wanted to make a hex nut and found that this was a fairly simple way to add this functionality to the library.

@adrianschlatter
Copy link
Owner

Ok, I see. I have rebased your PR on top of the develop branch so that the PR does not re-introduce bugs I had already fixed. I‘m currently on a train, but when I find time I‘d like to test whether we can set the default value of nut_sides to something invalid. Otherwise, we cannot set nut_sides to 120 and fn to something else.

Also, I want to test whether the change of order of the arguments will cause problems.

* nut_sides must be last argument
* default value = 0. With 120 as default, we cannot set nut_sides to 120
  while simultaneously setting fn to something different from 120.
@adrianschlatter
Copy link
Owner

adrianschlatter commented Jan 27, 2024

Ok, default value of 0 works for nut_sides. Also, we are not allowed to change the order of arguments for backwards compatibility reasons but it is ok to add a new argument at the end.

I am going to merge this. Thanks for contributing to threadlib!

@adrianschlatter adrianschlatter merged commit 3830919 into adrianschlatter:develop Jan 27, 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