From 1027926be95f9d6c13e37dbd5abac96ff62e5ece Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20B=C3=A4hr?= Date: Sun, 10 Nov 2024 17:39:36 +0100 Subject: [PATCH] CAM: Configure Helix via CutMode, not Direction (#14314) To harmonize the various CAM operations, the helical drill shall also be configured using the cut mode (Climb/Conventional) just like Pocket or FaceMill. This means, the path direction (CW/CCW) is now a hidden read-only output property, calculated from the side (Inside/OutSide) and the here newly introduced cut mode. The spindle direction is not yet taken into account and is always assumed to be "Forward". The GUI strings are kept compatible with what RP#14364 introduced becasue of the v1.0 release string freeze. They need revision once back on the main branch. --- src/Mod/CAM/CAMTests/TestPathHelix.py | 72 ++++++++++++++++--- .../Gui/Resources/panels/PageOpHelixEdit.ui | 6 +- src/Mod/CAM/Path/Op/Gui/Helix.py | 10 +-- src/Mod/CAM/Path/Op/Helix.py | 72 +++++++++++++++++++ 4 files changed, 144 insertions(+), 16 deletions(-) diff --git a/src/Mod/CAM/CAMTests/TestPathHelix.py b/src/Mod/CAM/CAMTests/TestPathHelix.py index 5efc50aca2..f1561851d3 100644 --- a/src/Mod/CAM/CAMTests/TestPathHelix.py +++ b/src/Mod/CAM/CAMTests/TestPathHelix.py @@ -132,6 +132,33 @@ class TestPathHelix(PathTestUtils.PathTestBase): round(pos.Length / 10, 0), proxy.holeDiameter(op, model, sub) ) + def testPathDirection(self): + """Verify that the generated paths obays the given parameters""" + helix = PathHelix.Create("Helix") + + def check(start_side, cut_mode, expected_direction): + with self.subTest(f"({start_side}, {cut_mode}) => {expected_direction}"): + helix.StartSide = start_side + helix.CutMode = cut_mode + + self.assertSuccessfulRecompute(self.doc, helix) + + self.assertEqual( + helix.Direction, + expected_direction, + msg=f"Direction was not correctly determined", + ) + self.assertPathDirection( + helix.Path, + expected_direction, + msg=f"Path with wrong direction generated", + ) + + check("Inside", "Conventional", "CW") + check("Outside", "Climb", "CW") + check("Inside", "Climb", "CCW") + check("Outside", "Conventional", "CCW") + def testRecomputeHelixFromV021(self): """Verify that we can still open and recompute a Helix created with older FreeCAD""" self.tearDown() @@ -139,39 +166,68 @@ class TestPathHelix(PathTestUtils.PathTestBase): created_with = f"created with {self.doc.getProgramVersion()}" def check(helix, direction, start_side, cut_mode): - with self.subTest(f"{helix.Name}: {direction}, {start_side}, {cut_mode}"): + with self.subTest(f"{helix.Name}: ({direction}, {start_side}) => {cut_mode}"): # no recompute yet, i.e. check original as precondition + self.assertPathDirection( + helix.Path, + direction, + msg=f"Path direction does not match fixture for {helix.Name} {created_with}", + ) self.assertEqual( helix.Direction, direction, - msg=f"Direction does not match fixture for helix {created_with}", + msg=f"Direction does not match fixture for {helix.Name} {created_with}", ) self.assertEqual( helix.StartSide, start_side, - msg=f"StartSide does not match fixture for helix {created_with}", + msg=f"StartSide does not match fixture for {helix.Name} {created_with}", ) # now see whether we can recompute the object from the old document helix.enforceRecompute() self.assertSuccessfulRecompute( - self.doc, helix, msg=f"Cannot recompute helix {created_with}" + self.doc, helix, msg=f"Cannot recompute {helix.Name} {created_with}" ) self.assertEqual( helix.Direction, direction, - msg=f"Direction changed after recomputing helix {created_with}", + msg=f"Direction changed after recomputing {helix.Name} {created_with}", ) self.assertEqual( helix.StartSide, start_side, - msg=f"StartSide changed after recomputing helix {created_with}", + msg=f"StartSide changed after recomputing {helix.Name} {created_with}", + ) + self.assertEqual( + helix.CutMode, + cut_mode, + msg=f"CutMode not correctly derived for {helix.Name} {created_with}", + ) + self.assertPathDirection( + helix.Path, + direction, + msg=f"Path with wrong direction generated for {helix.Name} {created_with}", ) - # self.assertEqual(helix.CutMode, cut_mode, - # msg=f"CutMode not correctly derived for helix {created_with}") # object names and expected values defined in the fixture check(self.doc.Helix, "CW", "Inside", "Conventional") check(self.doc.Helix001, "CW", "Outside", "Climb") check(self.doc.Helix002, "CCW", "Inside", "Climb") check(self.doc.Helix003, "CCW", "Outside", "Conventional") + + def assertPathDirection(self, path, expected_direction, msg=None): + """Asserts that the given path goes into the expected direction. + + For the general case we'd need to check the sign of the second derivative, + but as we know we work on a helix here, we can take a short cut and just + look at the G2/G3 arc commands. + """ + has_g2 = any(filter(lambda cmd: cmd.Name == "G2", path.Commands)) + has_g3 = any(filter(lambda cmd: cmd.Name == "G3", path.Commands)) + if has_g2 and not has_g3: + self.assertEqual("CW", expected_direction, msg) + elif has_g3 and not has_g2: + self.assertEqual("CCW", expected_direction, msg) + else: + raise NotImplementedError("Cannot determine direction for arbitrary paths") diff --git a/src/Mod/CAM/Gui/Resources/panels/PageOpHelixEdit.ui b/src/Mod/CAM/Gui/Resources/panels/PageOpHelixEdit.ui index cdc8f842d8..cf07cebfe7 100644 --- a/src/Mod/CAM/Gui/Resources/panels/PageOpHelixEdit.ui +++ b/src/Mod/CAM/Gui/Resources/panels/PageOpHelixEdit.ui @@ -89,18 +89,18 @@ - + The direction for the helix, clockwise or counterclockwise. - CW + Climb - CCW + Conventional diff --git a/src/Mod/CAM/Path/Op/Gui/Helix.py b/src/Mod/CAM/Path/Op/Gui/Helix.py index 99edc2f321..c06166b076 100644 --- a/src/Mod/CAM/Path/Op/Gui/Helix.py +++ b/src/Mod/CAM/Path/Op/Gui/Helix.py @@ -52,7 +52,7 @@ class TaskPanelOpPage(PathCircularHoleBaseGui.TaskPanelOpPage): """getForm() ... return UI""" form = FreeCADGui.PySideUic.loadUi(":/panels/PageOpHelixEdit.ui") - comboToPropertyMap = [("startSide", "StartSide"), ("direction", "Direction")] + comboToPropertyMap = [("startSide", "StartSide"), ("cutMode", "CutMode")] enumTups = PathHelix.ObjectHelix.helixOpPropertyEnumerations(dataType="raw") @@ -62,8 +62,8 @@ class TaskPanelOpPage(PathCircularHoleBaseGui.TaskPanelOpPage): def getFields(self, obj): """getFields(obj) ... transfers values from UI to obj's properties""" Path.Log.track() - if obj.Direction != str(self.form.direction.currentData()): - obj.Direction = str(self.form.direction.currentData()) + if obj.CutMode != str(self.form.cutMode.currentData()): + obj.CutMode = str(self.form.cutMode.currentData()) if obj.StartSide != str(self.form.startSide.currentData()): obj.StartSide = str(self.form.startSide.currentData()) if obj.StepOver != self.form.stepOverPercent.value(): @@ -78,7 +78,7 @@ class TaskPanelOpPage(PathCircularHoleBaseGui.TaskPanelOpPage): Path.Log.track() self.form.stepOverPercent.setValue(obj.StepOver) - self.selectInComboBox(obj.Direction, self.form.direction) + self.selectInComboBox(obj.CutMode, self.form.cutMode) self.selectInComboBox(obj.StartSide, self.form.startSide) self.setupToolController(obj, self.form.toolController) @@ -94,7 +94,7 @@ class TaskPanelOpPage(PathCircularHoleBaseGui.TaskPanelOpPage): signals.append(self.form.stepOverPercent.editingFinished) signals.append(self.form.extraOffset.editingFinished) - signals.append(self.form.direction.currentIndexChanged) + signals.append(self.form.cutMode.currentIndexChanged) signals.append(self.form.startSide.currentIndexChanged) signals.append(self.form.toolController.currentIndexChanged) signals.append(self.form.coolantController.currentIndexChanged) diff --git a/src/Mod/CAM/Path/Op/Helix.py b/src/Mod/CAM/Path/Op/Helix.py index a283124989..9714284816 100644 --- a/src/Mod/CAM/Path/Op/Helix.py +++ b/src/Mod/CAM/Path/Op/Helix.py @@ -51,6 +51,36 @@ else: translate = FreeCAD.Qt.translate +def _caclulatePathDirection(mode, side): + """Calculates the path direction from cut mode and cut side""" + # NB: at the time of writing, we need py3.8 compat, thus not using py3.10 pattern machting + if mode == "Conventional" and side == "Inside": + return "CW" + elif mode == "Conventional" and side == "Outside": + return "CCW" + elif mode == "Climb" and side == "Inside": + return "CCW" + elif mode == "Climb" and side == "Outside": + return "CW" + else: + raise ValueError(f"No mapping for '{mode}'/'{side}'") + + +def _caclulateCutMode(direction, side): + """Calculates the cut mode from path direction and cut side""" + # NB: at the time of writing, we need py3.8 compat, thus not using py3.10 pattern machting + if direction == "CW" and side == "Inside": + return "Conventional" + elif direction == "CW" and side == "Outside": + return "Climb" + elif direction == "CCW" and side == "Inside": + return "Climb" + elif direction == "CCW" and side == "Outside": + return "Conventional" + else: + raise ValueError(f"No mapping for '{direction}'/'{side}'") + + class ObjectHelix(PathCircularHoleBase.ObjectOp): """Proxy class for Helix operations.""" @@ -75,6 +105,10 @@ class ObjectHelix(PathCircularHoleBase.ObjectOp): (translate("PathProfile", "Outside"), "Outside"), (translate("PathProfile", "Inside"), "Inside"), ], # side of profile that cutter is on in relation to direction of profile + "CutMode": [ + (translate("CAM_Helix", "Climb"), "Climb"), + (translate("CAM_Helix", "Conventional"), "Conventional"), + ], # whether the tool "rolls" with or against the feed direction along the profile } if dataType == "raw": @@ -106,6 +140,8 @@ class ObjectHelix(PathCircularHoleBase.ObjectOp): "The direction of the circular cuts, ClockWise (CW), or CounterClockWise (CCW)", ), ) + obj.setEditorMode("Direction", ["ReadOnly", "Hidden"]) + obj.setPropertyStatus("Direction", ["ReadOnly", "Output"]) obj.addProperty( "App::PropertyEnumeration", @@ -114,6 +150,17 @@ class ObjectHelix(PathCircularHoleBase.ObjectOp): QT_TRANSLATE_NOOP("App::Property", "Start cutting from the inside or outside"), ) + # TODO: revise property description once v1.0 release string freeze is lifted + obj.addProperty( + "App::PropertyEnumeration", + "CutMode", + "Helix Drill", + QT_TRANSLATE_NOOP( + "App::Property", + "The direction of the circular cuts, ClockWise (Climb), or CounterClockWise (Conventional)", + ), + ) + obj.addProperty( "App::PropertyPercent", "StepOver", @@ -163,9 +210,34 @@ class ObjectHelix(PathCircularHoleBase.ObjectOp): ), ) + if not hasattr(obj, "CutMode"): + # TODO: consolidate the duplicate definitions from opOnDocumentRestored and + # initCircularHoleOperation once back on the main line + obj.addProperty( + "App::PropertyEnumeration", + "CutMode", + "Helix Drill", + QT_TRANSLATE_NOOP( + "App::Property", + "The direction of the circular cuts, ClockWise (Climb), or CounterClockWise (Conventional)", + ), + ) + obj.CutMode = ["Climb", "Conventional"] + if obj.Direction in ["Climb", "Conventional"]: + # For some month, late in the v1.0 release cycle, we had the cut mode assigned + # to the direction (see PR#14364). Let's fix files created in this time as well. + new_dir = "CW" if obj.Direction == "Climb" else "CCW" + obj.Direction = ["CW", "CCW"] + obj.Direction = new_dir + obj.CutMode = _caclulateCutMode(obj.Direction, obj.StartSide) + obj.setEditorMode("Direction", ["ReadOnly", "Hidden"]) + obj.setPropertyStatus("Direction", ["ReadOnly", "Output"]) + def circularHoleExecute(self, obj, holes): """circularHoleExecute(obj, holes) ... generate helix commands for each hole in holes""" Path.Log.track() + obj.Direction = _caclulatePathDirection(obj.CutMode, obj.StartSide) + self.commandlist.append(Path.Command("(helix cut operation)")) self.commandlist.append(Path.Command("G0", {"Z": obj.ClearanceHeight.Value}))