Refactor post command

[sliptonic]

Significant change to how post-processing is done.

This does not change any existing post-processors but changes how they are called.
Sets the stage for refactoring the individual postprocessors to have less duplicated code.

[sliptonic]

Fixes #6117

[LarryWoestman]

(copying my comments about this branch from Discord for future reference)

I have managed to download and build this branch. I will add comments about things I noticed as I run into them.

It passes TestCAMApp (as expected).

Some tests in Tests.TestPathPost fail with "NameError: name 'job' is not defined", so that code will need some work. As far as I know, I am the only person who uses that code (to compare old and new output of some of the postprocessors). Not a surprise or an issue except for me.

At the moment I am using a build on Windows 11 for this testing.

I ran through creating a 50 mm cube with 10 mm stock on top, then created a CAM Job to face the stock off of the top. I used the CAM_Sanity check a few times, then postprocessed the job using the linuxcnc postprocessor. I didn't run into any surprises.

The UI seemed familiar to someone who hasn't used it in a while 🙂

I was trying to add a Profile operation, but didn't get anything in the geometry to work on by default. I tried clicking on the Stock and Model lines in the tree, but ran into an error:

15:50:45  Base.ERROR: Please select features of a solid
15:51:15  Illegal storage access...
15:51:15  pyException: Traceback (most recent call last):
  File "<string>", line 1, in <module>
<class 'OSError'>: Illegal storage access! Please save your work under a new file name and restart the application!
15:51:15  Illegal storage access! Please save your work under a new file name and restart the application! 

The python console showed:

Gui.runCommand('CAM_Profile',0)
Gui.Selection.clearSelection()
Gui.ActiveDocument.setEdit(App.getDocument('Unnamed').getObject('Clone'), 0)
Gui.Selection.addSelection('Unnamed','Stock')

I can't figure out how to select the whole body at the moment.
After a restart of FreeCAD, I tried again to add a Profile to the job, but have no Base Geometry in the Profile and can't seem to find a way to select anything from the tree area.
I tried selecting the Stock from the tree before starting the Profile operation with the same "Base.ERROR Please select features of a sold" error. Either there is a bug here or more likely I don't understand how to use FreeCAD any more 🙂
This is the test file that I created in case it is helpful.
new_block.FCStd.txt

[sliptonic]

Nothing in this PR would affect the setup of the profile operation. I tested it with your model and everything worked as normal.
I can just add a profile with nothing selected and it will work on the whole base object. I can also click a face of the model, then click it again to select the entire shape. Either works.

That TestPathPost test is a monster. It's really complex. So it's fine for debugging and local testing but definitely shouldn't be enabled in the core.

Once we get this part of the refactoring done, let's touch bases about moving forward with the individual posts. I can help with unit tests and structure.

[LarryWoestman]

I was able to add a profile with nothing selected, although I did get a "Please select features from a single solid" message in the Report view. The profile postprocessed without any obvious issues.

Is there anything else that I could test that would target the changes more precisely?

I agree that TestPathPost does a lot, and is complex. It also requires a lot of data files that we decided were not appropriate to be checked in to the FreeCAD source tree. However, it also provides valuable information for my current efforts to refactor some of the postprocessors while producing equivalent output. Is there somewhere else that testing-only code should go?

I have been working on tests for the A, B, and C parameters. I have a draft PR (#13702) that is "stuck" because some of the recent changes break the tests in ways that I don't understand. I could use your help figuring out what PathUtils.getPathWithPlacement is doing to the A, B, and C parameters (and why). Once those tests work properly for the linuxcnc_post, I plan to add them to the tests for the "refactored*" posts.

I would also like to change the "refactored*" posts as necessary to use the new interfaces in this PR going forward.

[sliptonic]

Is there anything else that I could test that would target the changes more precisely?

Does output splitting work consistently? Does name generation from template strings work consistently?
Does order-by work for fixtures, tools, operations.
Does postprocessing pick up the post args if they are in user preferences/job/both/neither?

I doubt that getPathWithPlacement even considers rotational axes. I haven't looked at that code for a long time and I didn't write it. I assumed it was just pure translation in XYZ.

Let's find a time to chat via zoom/jitsi/slack and talk about post refactoring. If we shoot for saturday maybe we can get a bunch of folks to join and have a broader CAM roadmap discussion.

[LarryWoestman]

It appears from my test results that getPathWithPlacement at least changes the A, B, and C values. It looks like it modifies them to end up in certain quadrants (0 to 180 degrees, for example) but why the results are as they are escapes me. I tried following the calls in the routine and got completely lost deep in the C++ vector code :-)

I would be happy to chat and talk about refactoring. Saturday would work for me if it isn't too early.

I will try to do more testing tomorrow along the lines that you described.

[LarryWoestman]

When I create a job, then postprocess it with no output file specified, I get "/test_order_by.nc" as my default file name. Note the leading "/", which is not valid (at least on Windows 11).

When I am trying to set up a facing operation, the first dialog box is titled "Choose a Pat... ?". It probably needs to say "Choose a Path... ?".

I'm getting stuck trying to create a job (or jobs) that have three bodies, one at G54, one at G55, and one at G56, and then postprocess a face operation for all three bodies in one file. If I put all three bodies in one job, then I can't figure out how to put them in different coordinate systems. If I put each body in its own job, then I can't figure out how to postprocess all three jobs in one file. There doesn't appear to be a way to select more than one job for postprocessing as far as I can tell.

I have had multiple instances where errors occur related to deleting something that has already been deleted, but I can't figure out how I caused the problem and I can't get the problem(s) to repeat.

[sliptonic]

When I create a job, then postprocess it with no output file specified, I get "/test_order_by.nc" as my default file name. Note the leading "/", which is not valid (at least on Windows 11).

Could you try replacing this line:
https://github.com/Ondsel-Development/FreeCAD/blob/RefactorPostCommand/src/Mod/CAM/Path/Post/Utils.py#L130

with
file_path = os.path.normpath(file_path.replace(key, value))

[LarryWoestman]

I made the change but I still get the same output file string.

[sliptonic]

I've made a small change to how it handles the case where no filename/path is defined in either the job or the user preferences. In that case, it will default to the current working directory. Can you retry it with the latest and also check if this test passes?

./bin/FreeCADCmd -t TestCAMApp.TestFileNameGenerator.test000

[LarryWoestman]

I will be happy to do that, but I won't have time until Friday evening or Saturday afternoon (PDT).

[LarryWoestman]

TestFileNameGenerator.test000 passed, but test020, test046, and test050 of TestCAMApp fail:

======================================================================
FAIL: test020 (Tests.TestPathPost.TestFileNameGenerator)

Traceback (most recent call last):
File "C:\src\RefactorPostCommand\build\Mod\CAM\Tests\TestPathPost.py", line 203, in test020
self.assertEqual(filename, f"{self.testfilename}.nc")
AssertionError: 'C:\src\RefactorPostCommand\build\bin\test_filenaming.nc' != 'test_filenaming.nc'

• C:\src\RefactorPostCommand\build\bin\test_filenaming.nc

• test_filenaming.nc

======================================================================
FAIL: test046 (Tests.TestPathPost.TestFileNameGenerator)
Testing the sequence number substitution

Traceback (most recent call last):
File "C:\src\RefactorPostCommand\build\Mod\CAM\Tests\TestPathPost.py", line 255, in test046
self.assertEqual(filename, os.path.normpath(expected_filename))
AssertionError: 'C:\src\RefactorPostCommand\build\bin\0-test_filenaming.nc' != '0-test_filenaming.nc'

• C:\src\RefactorPostCommand\build\bin\0-test_filenaming.nc

• 0-test_filenaming.nc

======================================================================
FAIL: test050 (Tests.TestPathPost.TestFileNameGenerator)

Traceback (most recent call last):
File "C:\src\RefactorPostCommand\build\Mod\CAM\Tests\TestPathPost.py", line 269, in test050
self.assertEqual(filename, os.path.normpath("0-test_filenaming.nc"))
AssertionError: 'C:\src\RefactorPostCommand\build\bin\0-test_filenaming.nc' != '0-test_filenaming.nc'

• C:\src\RefactorPostCommand\build\bin\0-test_filenaming.nc

• 0-test_filenaming.nc

When I tried to recreate the issue, it worked this time. No leading "/".

[sliptonic]

Doh! Fixed

[LarryWoestman]

The latest push passes the TestCAMApp tests on my Windows 11 build (for what it is worth).