View Issue Details

IDProjectCategoryView StatusLast Update
0001247FreeCADBugpublic2013-11-07 13:21
Reporterwandererfan Assigned Towmayer  
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Product Version0.13 
Target Version0.14 
Summary0001247: Part Conical Helix Height/Pitch Incorrect
DescriptionPart Toposhape::makeHelix makes conical helices with incorrect height & pitch. See HelixCylCone.fcst.
Current code uses same segment endpoint calculation for both cylindrical and conical helices.
See conHelFix.patch for new calculation for conical helix.
Additional InformationOS: Ubuntu 12.04.3 LTS
Platform: 64-bit
Version: 0.13.1830 (Git)
Branch: releases/FreeCAD-0-13
Hash: ec7636d7aaf2612e9b43cff5d6a424037d53e505
Python version: 2.7.3
Qt version: 4.8.1
Coin version: 3.1.3
SoQt version: 1.5.0
OCC version: 6.5.0
TagsNo tags attached.
FreeCAD Information

Activities

ulrich1a

2013-09-25 18:09

reporter   ~0003669

I am using the conical helix in my screw-maker macro. The actual pitch is just the length between two helix lines after a turn. I used sine and cosine calculations to get my helix defined right. Changing the functionality would break my macro.

wandererfan

2013-09-26 20:00

manager   ~0003684

The current code in makeHelix doesn't do what it is asked to do, it should be changed.

The change doesn't seem to make much difference to the macro(see png's). The top thread is a little shorter in the devel version.

Are taper threads an issue? I don't know of an ISO equivalent to US NPT spec so I didn't try one.

ulrich1a

2013-09-27 13:03

reporter   ~0003691

I have another view: The current gui is asking wrong, because it does not allow to construct a flat helix.
A flat helix would allow to construct things like heat exchangers, electrolyte condensors and other things. And to exclude this possibility makes FreeCAD less usefull. So I would like to see a gui and a changed code that allows an angle of 90° for the helix, which is not possible in the current gui.

wandererfan

2013-09-27 19:34

manager   ~0003694

That's a different issue. This report doesn't involve the gui at all. It's about the code in part/app/TopoShape.cpp not dealing properly with a curve on a conical surface.

see http://forum.freecadweb.org/viewtopic.php?f=3&t=4211&start=0 for partial Archimedean spiral via gui. It's partial because FC won't accept a start radius of zero.

ulrich1a

2013-09-28 17:14

reporter   ~0003699

The bug report is about the point, that the description of the function does not describe what the function does. The user expects something different. I was in the same situation. This is a valid point and should be handled as a bug.
There are three possible solutions:
1. Change the description of the function. In this case we have to exchange pitch with the distance between the helix lines and height by the length of the distance between helix lines multiplied by the number of helix turns. This description does meet the current implementation of the helix-function.

2. Change the original function interface, so it works now with the heigth and the pitch instead of the two parameters above. This is no good idea, because former versions of FreeCAD where advertised with the possibility of making constructions by python code. I will give an extreme constructed case, what could happen, if the python code is broken, due to a different interface of the python helix-function: Programmer P programs a parametric safety device, with a conical spring as key component. Manufacturer M buys this code and produces this safety devices with individual settings for each customer. Customer C dies, as he had bought a safety device after an FreeCAD update. Now the conical springs have a different length and the new configured safety devices do not work reliable any more. This would give FreeCAD a very bad reputation.

3. Make a different Helix-function, "Helix_B" for example. This function may have the height and the pitch as parameters. And use this function in the Part-Workbench. This would not break existing code and also solve your case.

My point is: The current implementation does allow a nearly flat helix with an angle of 89.9999°. I would like to see an implementation, which also allows an angle of 90°. Using the height as parameter will exclude per definition a flat helix. So why not making a new Helix-function "Helix_C" with distance between helix lines and number of turns as parameters. This is intuitive and would in principle also allow a flat helix. (I hope, the OCC-code does allow a flat helix.)

wmayer

2013-09-30 08:44

administrator   ~0003702

As far as I can see the patch from wandererfan is a correct fix and the current behaviour is wrong.

The Height at the moment doesn't give the height of the virtual cone you internally use but the length of the line you get by intersecting the cone's girthed area with the plane that includes the axis of the cone.

As an example create a cone with Radius1=0.00, Radius2=10.00, Height=10.00. So this corresponds to a helix with an angle of 45 deg, Radius=0.00 (which is currently not possible, so choose 0.01), Height=10.00. You can easily see that the helix' end point doesn't match with the bottom area of the cone.

> The current gui is asking wrong, because it does not allow to construct a flat helix.
IMO, this has nothing to do with the mentioned misbehaviour. The helix is created by projecting a line on a cone. But creating a flat helix is not possible because a cone with Height=0 is not a cone. Here we need a totally different procedure to create a flat helix.

> I hope, the OCC-code does allow a flat helix.
AFAIK it does not. But as a further step you can project the conical helix onto a plane.

ulrich1a

2013-09-30 13:19

reporter   ~0003703

I know that the current helix-function does not work as described. I figured that out, as I was programming the macro screw_maker. I am using
Height_angle = Height/cos(angle), Pitch_angle = Pitch/cos(angle) for the conical helix in order to get the height I was needing.

As the documentation of FreeCAD is often lagging behind functionality, I argue that the documentation is wrong and the function is right.

I just dont want to have changed the current version of the python-helix-function. As there is external code out in the field, that is using the current version of the python-helix-function. See for example here: http://www.freecadweb.org/wiki/index.php?title=Macro_screw_maker1_2
It is in general not a good idea to change the interface of a function, which is already in use by others, as it breaks the external code. Nobody can figure out what kind of errors are introduced in the software others made for the usage with FreeCAD. (See the hypothetical example of my post above.) M$ was partly so successfull as they did not changed odd behavior in order not to break existing software.
The solution to this is, make a new function were documentation and functionality are in sync. So old external software will function still correct and new external software can use the new function.

yorik

2013-11-04 13:45

administrator  

conHelFix.patch (Attachment missing)

yorik

2013-11-04 13:45

administrator  

HelixCylCone.fcstd (Attachment missing)

yorik

2013-11-04 13:45

administrator  

devm510.png (Attachment missing)

yorik

2013-11-04 13:46

administrator  

mstm510.png (Attachment missing)

wmayer

2013-11-07 12:52

administrator   ~0003865

Instead of having a new helix class it's better to introduce a new mode parameter with two values: maybe "old style" and "new style" or are the better expressions for it?

So, the behaviour will be that the default is "old style" to keep scripts running as is. In the GUI where we create a helix using the panel the "new style" can be set to create it correctly.

Related Changesets

FreeCAD: master d5757b70

2013-11-07 14:17:41

wmayer

Details Diff
+ fixes 0001247 Affected Issues
0001247
mod - src/Mod/Part/App/PrimitiveFeature.cpp Diff File
mod - src/Mod/Part/App/PrimitiveFeature.h Diff File
mod - src/Mod/Part/App/TopoShape.cpp Diff File
mod - src/Mod/Part/App/TopoShape.h Diff File
mod - src/Mod/Part/Gui/DlgPrimitives.cpp Diff File

Issue History

Date Modified Username Field Change
2013-09-22 16:12 wandererfan New Issue
2013-09-22 16:12 wandererfan File Added: HelixCylCone.fcstd
2013-09-22 16:13 wandererfan File Added: conHelFix.patch
2013-09-25 18:09 ulrich1a Note Added: 0003669
2013-09-26 19:47 wandererfan File Added: devm510.png
2013-09-26 19:47 wandererfan File Added: mstm510.png
2013-09-26 20:00 wandererfan Note Added: 0003684
2013-09-27 13:03 ulrich1a Note Added: 0003691
2013-09-27 19:34 wandererfan Note Added: 0003694
2013-09-28 17:14 ulrich1a Note Added: 0003699
2013-09-30 08:44 wmayer Note Added: 0003702
2013-09-30 13:19 ulrich1a Note Added: 0003703
2013-11-04 13:44 yorik File Deleted: HelixCylCone.fcstd
2013-11-04 13:44 yorik File Deleted: conHelFix.patch
2013-11-04 13:44 yorik File Deleted: devm510.png
2013-11-04 13:44 yorik File Deleted: mstm510.png
2013-11-04 13:45 yorik File Added: conHelFix.patch
2013-11-04 13:45 yorik File Added: HelixCylCone.fcstd
2013-11-04 13:45 yorik File Added: devm510.png
2013-11-04 13:46 yorik File Added: mstm510.png
2013-11-07 12:52 wmayer Note Added: 0003865
2013-11-07 13:01 wmayer Assigned To => wmayer
2013-11-07 13:01 wmayer Status new => assigned
2013-11-07 13:02 wmayer Target Version => 0.14
2013-11-07 13:21 wmayer Changeset attached => FreeCAD Master master d5757b70
2013-11-07 13:21 wmayer Status assigned => closed
2013-11-07 13:21 wmayer Resolution open => fixed