Bug#1040426: FreeCAD Path: Generate dangeruos first G-code move while G49 is active

2023-07-11 Thread Petter Reinholdtsen
[Petter Reinholdtsen]
> This patch is also available as
> https://salsa.debian.org/science-team/freecad/-/merge_requests/23 >.
>
> Kurt and Sebastian, please have a look and object if you do not want
> this in the Debian package before 0.21.  It is already sent upstream
> with the hope of having it into the next release of FreeCAD.

As no-one seem to care, and no-one objected, I plan to merge the above
mentioned merge request today and upload a new version with it included
to unstable.

-- 
Happy hacking
Petter Reinholdtsen



Bug#1040426: FreeCAD Path: Generate dangeruos first G-code move while G49 is active

2023-07-08 Thread Petter Reinholdtsen


The change was just accepted upstream.
-- 
Happy hacking
Petter Reinholdtsen



Bug#1040426: FreeCAD Path: Generate dangeruos first G-code move while G49 is active

2023-07-08 Thread Petter Reinholdtsen


This patch is also available as
https://salsa.debian.org/science-team/freecad/-/merge_requests/23 >.

Kurt and Sebastian, please have a look and object if you do not want
this in the Debian package before 0.21.  It is already sent upstream
with the hope of having it into the next release of FreeCAD.

-- 
Happy hacking
Petter Reinholdtsen



Bug#1040426: FreeCAD Path: Generate dangeruos first G-code move while G49 is active

2023-07-05 Thread Petter Reinholdtsen


Package: freecad
Version: 0.20.2+dfsg1-4
Severity: important
Forwarded: https://github.com/FreeCAD/FreeCAD/issues/9866
Tags: patch

I created a FreeCAD model and generated some Path Jobs to mill it out,
and was very surprised when the generated G-code ended up digging the
first cutter straight into the work table. This is using the linuxcnc
post processor, but is unrelated to the postprocessor used.

The model is available from
https://codeberg.org/pere/thinkpad-x230-dc-socket-bracket >. I
used the G-code in the subdirectory linuxcnc-jobs in commit
ff248e59b7c44bda3e6320e6499ee9c240156dae for this failure.

I tracked this down to the first move in the generated in the G-code
file, which is a "G0 Z", which is no longer safe when the
15 cm tool height compensation has been wiped out with the G49 in the
preamble.

This is the generated G-code:

(Exported by FreeCAD)
(Post Processor: linuxcnc_post)
(Output Time:2023-07-03 14:24:06.190303)
(begin preamble)
G17 G54 G40 G49 G80 G90
G21
(begin operation: G54)
(machine units: mm/min)
G54 
G0 Z19.000 ; < This one is dangerous while G49 is active
(finish operation: G54)
(begin operation: 6mm-endmill001)
(machine units: mm/min)
(6mm-endmill001) 
M5
M6 T3 
G43 H3 
M3 S2500 
(finish operation: 6mm-endmill001)
(begin operation: MillFace)
(machine units: mm/min)
(MillFace) 
G0 Z19.000 

I see from https://forum.freecad.org/viewtopic.php?t=37839 > the
issue has been known since 2019, and was even mentioned as an issue when
the introduction of G43 was discussed in
https://forum.freecad.org/viewtopic.php?t=27180 >.  The dangerous
code is only generated when splitting the G-code into separate files per tool.

It is unclear to me why the G49 is used in the preamble to begin with.
Is it not better to put it just before the G43, and avoid any G0 move
before G43 has been used to load the correct tool height compensation?

I had used the automatic tool height measurement in the machine to set
the correct tool height compensation for the tool holder and bit in
question, and was very surprised to see that it dived straight down as
the first move. Was not able to stop it quickly enough to avoid damage.

I believe the following patch will fix the issue.  It is already sent
upstream.  I recommend considering it for the Debian package too.

commit d8f2f87130b28a769e91fe20ffad885feecf381d
Author: Petter Reinholdtsen 
Date:   Mon Jul 3 16:42:58 2023 +0200

Avoid dagerous move without tool height compensation after setting first 
fixture

The issue only happen when splitting jobs on tools (orderby == Tool), and 
when
USE_TLO was active and the preamble include G49.  The first move is then 
done
before tool height is set, and can cause damage if the existing tool height 
is set
to more than the gap between the spindle and the table or work piece, when 
the machine
take a sudden dive straight down.

Removed move between G49 and first G43, to ensure all moves are done after 
G43
correctly set tool height compensation.

Rewrote code to introduce new method fixtureSetup() to ensure all orderby 
alternatives
behave the same way.

Fixes #9866.

diff --git a/src/Mod/Path/Path/Post/Command.py 
b/src/Mod/Path/Path/Post/Command.py
index 9393e7282f..d1f0635cf5 100644
--- a/src/Mod/Path/Path/Post/Command.py
+++ b/src/Mod/Path/Path/Post/Command.py
@@ -252,6 +252,33 @@ def resolveFileName(job, subpartname, sequencenumber):
 return fullPath
 
 
+def fixtureSetup(order, fixture, job):
+"""Convert a Fixure setting to _TempObject instance with a G0 move to a
+safe height every time the fixture coordinate system change.  Skip
+the move for first fixture, to avoid moving before tool and tool
+height compensation is enabled.
+
+"""
+
+fobj = _TempObject()
+c1 = Path.Command(fixture)
+fobj.Path = Path.Path([c1])
+# Avoid any tool move after G49 in preamble and before tool change
+# and G43 in case tool height compensation is in use, to avoid
+# dangerous move without tool compesation.
+if order != 0:
+c2 = Path.Command(
+"G0 Z"
++ str(
+job.Stock.Shape.BoundBox.ZMax
++ job.SetupSheet.ClearanceHeightOffset.Value
+)
+)
+fobj.Path.addCommands(c2)
+fobj.InList.append(job)
+return fobj
+
+
 def buildPostList(job):
 """Takes the job and determines the specific objects and order to
 postprocess  Returns a list of objects which can be passed to
@@ -269,20 +296,7 @@ def buildPostList(job):
 currTool = None
 for index, f in enumerate(wcslist):
 # create an object to serve as the fixture path
-fobj = _TempObject()
-c1 = Path.Command(f)
-fobj.Path = Path.Path([c1])
-if index != 0:
-c2 = Path.Command(
-"G0 Z"
-+ str(
-