Fix for #1: Countersink Head angle vs nominal size #12

Open
sw-tya wants to merge 3 commits from sw-tya/add_mesh_BoltFactory:swtya_CounterSinkHead into main

When changing the target branch, be careful to rebase the branch in your fork to match. See documentation.
First-time contributor

Here is my attempt at a fix to bring the contersink head modelling closer to the ISO specification with respect to the head angle.

Here is my attempt at a fix to bring the contersink head modelling closer to the ISO specification with respect to the head angle.
sw-tya added 1 commit 2024-08-19 01:26:43 +02:00
Loren Osborn reviewed 2024-08-19 07:35:43 +02:00
@ -793,0 +792,4 @@
# Between 20 and 22mm the head angle changes.
# Note, this is not suitable for cheating a rivet - due to a different angle, see ISO 1051.
# The input arg of HEIGHT is not passed from the GUI, it must be created here:
if SHANK_DIA < 21:
Contributor

I certainly applaud your research and initiative, but my concern is “the best thing about standards is there are so many to choose from.” Would there be a reason this couldn’t be an additional parameter in the GUI? I would love if the default value changed according to the shank diameter, but based on my own previous experimentation, I expect this may be a taller order. I expect other standards bodies will declare other counter sink angles in their own ecosystems parameterized off other cutoff values.

I certainly applaud your research and initiative, but my concern is “the best thing about standards is there are so many to choose from.” Would there be a reason this couldn’t be an additional parameter in the GUI? I would love if the default value changed according to the shank diameter, but based on my own previous experimentation, I expect this may be a taller order. I expect other standards bodies will declare other counter sink angles in their own ecosystems parameterized off other cutoff values.
Author
First-time contributor

Yep, I hear what you are saying....

"Would there be a reason this couldn’t"

Indeed that wold be best. Having had a look this it will touch some of the same lines already touched in #10 & #11. To do a complete job the new angle should make it into the presets too.

How would I do it:

  1. Update Create_CounterSink_Head() so the not used HEIGHT becomes the HEAD_ANGLE.
  2. My new lines get scrubbed and the HEAD_ANGLE is used to make the height.
  3. Define a new property "props.bf_CounterSink_Head_Angle" in the class add_mesh_bolt.
    3a) Make new property appear in the GUI draw() function adding "col.prop(self, 'bf_CounterSink_Head_Angle')"
  4. Pass that new property via Bolt_Mesh()
  5. For each of the preset files add the required angle property.

I understand GIT can manage this, but can I....
Less scary to wait until those other changes are on main. Then I can "rebase main" to avoid conflicts.

Yep, I hear what you are saying.... >> "Would there be a reason this couldn’t" Indeed that wold be best. Having had a look this it will touch some of the same lines already touched in #10 & #11. To do a complete job the new angle should make it into the presets too. How would I do it: 1) Update Create_CounterSink_Head() so the not used HEIGHT becomes the HEAD_ANGLE. 2) My new lines get scrubbed and the HEAD_ANGLE is used to make the height. 3) Define a new property "props.bf_CounterSink_Head_Angle" in the class add_mesh_bolt. 3a) Make new property appear in the GUI draw() function adding "col.prop(self, 'bf_CounterSink_Head_Angle')" 4) Pass that new property via Bolt_Mesh() 5) For each of the preset files add the required angle property. I understand GIT can manage this, but can I.... Less scary to wait until those other changes are on main. Then I can "rebase main" to avoid conflicts.
Author
First-time contributor

I would love if the default value changed according to the shank diameter, but based on my own previous experimentation, I expect this may be a taller order.

@linux_dr Did yo find this documentation page?: https://docs.blender.org/api/current/bpy.props.html

def update_func(self, context):
    print("my test function", self)
bpy.types.Scene.testprop = bpy.props.FloatProperty(update=update_func)
> I would love if the default value changed according to the shank diameter, but based on my own previous experimentation, I expect this may be a taller order. @linux_dr Did yo find this documentation page?: [https://docs.blender.org/api/current/bpy.props.html](https://docs.blender.org/api/current/bpy.props.html#update-example) ``` def update_func(self, context): print("my test function", self) bpy.types.Scene.testprop = bpy.props.FloatProperty(update=update_func) ```
Contributor

Yes… your game-plan above looks good… the big gotcha that I was fighting with is how to add transient state that the GUI builder can access but not display… (i .e. was the counter sink bevel a default angle the last time the GUI was rebuilt, so you can update the default based on the shank diameter). As far as a reasonable (if slightly extreme) range, perhaps a range of 30°-75° makes sense?

If you’re in a hurry, you could simply branch off swtya_combinedFixes_PR10_and_PR11 (git checkout swtya_combinedFixes_PR10_and_PR11 ;git checkout -b {new_branch_name_here} ), and cherry-pick your commits onto main when the other changes are merged, but, given your lack of familiarity with git it’s likely a better idea to wait for @nickberckley to merge #10 and #11 first.

Yes… your game-plan above looks good… the big gotcha that I was fighting with is how to add transient state that the GUI builder can access but not display… (i .e. was the counter sink bevel a default angle the last time the GUI was rebuilt, so you can update the default based on the shank diameter). As far as a reasonable (if slightly extreme) range, perhaps a range of 30°-75° makes sense? If you’re in a hurry, you could simply branch off `swtya_combinedFixes_PR10_and_PR11` (`git checkout swtya_combinedFixes_PR10_and_PR11 ;git checkout -b {new_branch_name_here}` ), and cherry-pick your commits onto `main` when the other changes are merged, but, given your lack of familiarity with `git` it’s likely a better idea to wait for @nickberckley to merge #10 and #11 first.
Author
First-time contributor

I made changes and tried to update this but can't:

/add_mesh_BoltFactory$ git push me swtya_CounterSinkHead
To projects.blender.org:sw-tya/add_mesh_BoltFactory.git
! [rejected] swtya_CounterSinkHead -> swtya_CounterSinkHead (non-fast-forward)
error: failed to push some refs to 'projects.blender.org:sw-tya/add_mesh_BoltFactory.git'

I've tried the command lines above. That asked me for a password which did not work = Authentication failure.
My new changes conflict with lines of my first attempt, so I resolved those too.
So far longer trying random command lines with git than editing the code.

I've now deleted all my git files (kept a backup)
With a newly created branch using: git fetch me swtya_CounterSinkHead:swtya_CounterSinkHead
Do I put my changes straight in or rebase to head of main first?

I made changes and tried to update this but can't: `/add_mesh_BoltFactory$ git push me swtya_CounterSinkHead` `To projects.blender.org:sw-tya/add_mesh_BoltFactory.git` ` ! [rejected] swtya_CounterSinkHead -> swtya_CounterSinkHead (non-fast-forward)` `error: failed to push some refs to 'projects.blender.org:sw-tya/add_mesh_BoltFactory.git'` I've tried the command lines above. That asked me for a password which did not work = Authentication failure. My new changes conflict with lines of my first attempt, so I resolved those too. So far longer trying random command lines with git than editing the code. I've now deleted all my git files (kept a backup) With a newly created branch using: git fetch me swtya_CounterSinkHead:swtya_CounterSinkHead Do I put my changes straight in or rebase to head of main first?
Contributor

I made changes and tried to update this but can't:

/add_mesh_BoltFactory$ git push me swtya_CounterSinkHead
To projects.blender.org:sw-tya/add_mesh_BoltFactory.git
! [rejected] swtya_CounterSinkHead -> swtya_CounterSinkHead (non-fast-forward)
error: failed to push some refs to 'projects.blender.org:sw-tya/add_mesh_BoltFactory.git'

I suspect when you git pulled The updated main branch, this did an implicit “rebase” of your branch, which git considers “rewriting history” and an “unsafe operation”. When you attempted to push your updated branch, the system noticed the conflicting histories and rejected the change. Typically adding -f or --force to your push command should override this safeguard. Unless the repository is specifically configured to block forced pushes, this should be a safeguard only and not a permissions issue.

I hope that helps.

Updated: iPhone’s keyboard tries very hard to prevent you from typing double hyphen… so the original version of this response had a wide-hyphen by mistake.

> I made changes and tried to update this but can't: > > `/add_mesh_BoltFactory$ git push me swtya_CounterSinkHead` > `To projects.blender.org:sw-tya/add_mesh_BoltFactory.git` > ` ! [rejected] swtya_CounterSinkHead -> swtya_CounterSinkHead (non-fast-forward)` > `error: failed to push some refs to 'projects.blender.org:sw-tya/add_mesh_BoltFactory.git'` I suspect when you `git pull`ed The updated `main` branch, this did an implicit “rebase” of your branch, which git considers “rewriting history” and an “unsafe operation”. When you attempted to push your updated branch, the system noticed the conflicting histories and rejected the change. Typically adding `-f` or `--force` to your push command should override this safeguard. Unless the repository is specifically configured to block forced pushes, this should be a safeguard only and not a permissions issue. I hope that helps. **Updated:** iPhone’s keyboard tries very hard to prevent you from typing double hyphen… so the original version of this response had a wide-hyphen by mistake.
sw-tya added 2 commits 2024-08-21 23:49:35 +02:00
Author
First-time contributor

Thanks it did help.
From my clean start I was able to first do git merge origin/main then reinstate my edits before doing the
git commit -a -m "Fix for #1: Countersink Head angle via GUI"
git push me swtya_CounterSinkHead
Which this time worked as I expected.
Having the intermediate commit I was not expecting.

As you will see from the code I elected not to try and be cleaver and make the angle auto update. It would probably be annoying depending on the order of data entry. I had started with the IntProperty, but changed to FloatProperty so that it could accept the unit type of rotation. The base blender unit is radians, which you will see in the preset file and maths of the head.

Again thank you for the patience while I learn git.

Thanks it did help. From my clean start I was able to first do `git merge origin/main` then reinstate my edits before doing the `git commit -a -m "Fix for #1: Countersink Head angle via GUI"` `git push me swtya_CounterSinkHead` Which this time worked as I expected. Having the intermediate commit I was not expecting. As you will see from the code I elected not to try and be cleaver and make the angle auto update. It would probably be annoying depending on the order of data entry. I had started with the IntProperty, but changed to FloatProperty so that it could accept the unit type of rotation. The base blender unit is radians, which you will see in the preset file and maths of the head. Again thank you for the patience while I learn git.
sw-tya changed title from WIP: Fix for #1: Contersink Head angle vs nominal size to WIP: Fix for #1: Countersink Head angle vs nominal size 2024-08-22 00:05:43 +02:00

btw if something is ready to be merged tag me as reviewer so I get notified

btw if something is ready to be merged tag me as reviewer so I get notified
Author
First-time contributor

Hi @nickberckley , I have completed the coding & believe it to be correct and functional.
It would be great to have a review before I ask for it merged in - hence the WIP.
Thanks

Hi @nickberckley , I have completed the coding & believe it to be correct and functional. It would be great to have a review before I ask for it merged in - hence the WIP. Thanks
sw-tya changed title from WIP: Fix for #1: Countersink Head angle vs nominal size to Fix for #1: Countersink Head angle vs nominal size 2024-11-14 21:02:45 +01:00
Author
First-time contributor

Hi @nickberckley There has been no negative feedback, so I have removed the WIP.

Hi @nickberckley There has been no negative feedback, so I have removed the WIP.
This pull request can be merged automatically.
This branch is out-of-date with the base branch
You are not authorized to merge this pull request.

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u swtya_CounterSinkHead:sw-tya-swtya_CounterSinkHead
git checkout sw-tya-swtya_CounterSinkHead
Sign in to join this conversation.
No reviewers
No Label
No Milestone
No project
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: extensions/add_mesh_BoltFactory#12
No description provided.