-
Notifications
You must be signed in to change notification settings - Fork 100
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 #84 and #63 by float conversion int(float( version_string )) >= 18 #85
base: master
Are you sure you want to change the base?
Conversation
In this PoC only the involute gears work, and there is still a lot of cleanup pending. What does work, however, is that those gears now play nicely with PartDesign's concept of stacking features onto each other, i.e. that the result of a feature is the fusion of all previous ones. Special Thanks goes to DeepSOIC for his tutorial in the forum at [1] as well as this Part-o-Matic which showed me how this works in real live [2] [1]: https://forum.freecadweb.org/viewtopic.php?f=22&t=21097#p163340 [2]: https://github.com/DeepSOIC/Part-o-magic/blob/master/PartOMagic/Features/PartDesign/PDShapeFeature.py
There are still some issues when the generated Shape is not a solid, e.g. in the preview mode of the Crown Gear.
Previously, the `preview_mode` of the crown gear returned a compound of the base and the cut-outs. This caused problems in PD::Bodies where a single solid is requried. The solution in this commit changes the preview_mode to only output the base, not generating the cutout shapes at all. This is consistent with the involute gears having "simple=true" and saves again 0.5 Seconds processsing time on my system using defaults (15 teeth, 4 loft profiles). In addition, "preview = false" is also speed up by first collecting all cut-outs, and then passing them all at once to a single cut operation. This reduced the cutting time from 3.0 Seconds to 2.2 Seconds here. So preview now generats the shape immediately (0.0008s vs 0.5s) and the actual crown is generated in 2.7s instead of 3.5s (again, using the defaut parameters, measued via Python's time.perf_counter).
Right now, the tool tip is used as transaction name, which matches the wording "Create a new sketch". There doesn't seem to be consistency though FreeCAD's other workbenches, though.
Fix looooo#63 by change version check from 19 to 18 (required for Mac users according to issue).
Just changing the check from ">= 19" to ">= 18" may fall a bit short. According to this form discussion the change was made during the 0.19 release cycle. If now also 0.18 requires this check, it has to be due to the change being backported into a 0.18.x release appearing after the initial 0.19 release. This means, also the patch-level (i.e. |
Ok. Note that the current check fails if the versionstring contains a decimal. int('19.8') --> error hence the change from "int(version_string)" to "int(float(version_string))" is needed regardless of the RHS of the expression. Or just "float(version_string)" depend on whether or not you want ot check major or major.minor version. |
That looks very strange, indeed (where "strange" means "completely unexpected to me"). In which version did you observe this? What is printed if you write |
I encountered this issue in FreeCAD 0.18.4 on Ubuntu 20.04.4 LTS, and can confirm that coercing the App.Value()[1] str to a float before coercing it to an int solved the issue. I can't speak on the 18 vs. 19 issue though, as I left that part of the statement alone. Here's the error that I was getting before updating the code:
App.Version()[1] prints |
ping @looooo (just checking on this PR, any progress?) |
Fix #63 by change version check from 19 to 18 (required for Mac users according to issue).