Skip to content
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

CAM: Fixed F parameter handling for only rotary axes, with tests #18685

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

LarryWoestman
Copy link
Contributor

This fixes #18491.

Added tests and code to fix the F parameter handling when only rotary axes are used with F feed rates.

That required adding the entire "parameters" argument to all of the parameter handling routines because the F parameter handling routine needed to see what other parameters were used in the command.

@DerAndere1
Copy link

DerAndere1 commented Dec 25, 2024

The Last Line in following G Code should also be handled as a pure rotation, i.e. distance in degrees and feedrate in degrees per time unit. I am not sure whether the CAM module would ever produce such g-code and whether this case is already covered (i.e. linear axis letters appear in the g-code line but only Rotary axes actually move)

G90 (absolute positioning)
G1 X0 Y0 Z0 A0 B0 C0 F200
G1 X0 Y0 Z0 A90 B0 C0 F200 (Rotation)

@maxwxyz
Copy link
Collaborator

maxwxyz commented Dec 25, 2024

should this be backported to 1.0.1?

@LarryWoestman
Copy link
Contributor Author

This should be able to be backported without conflicts.

@maxwxyz maxwxyz added the Needs backport Needs backport to 1.0.1 label Dec 25, 2024
@LarryWoestman
Copy link
Contributor Author

At the moment I doubt if FreeCAD generates much, if any, rotary motion g-code in any of the "officially supported" (as compared to macros or add-on workbenches) code. However, I want the refactored* postprocessors to be able to handle future code generation possibilities where possible since we already have some level of work going on with lathe support and 4th axis support being experimented with.

@LarryWoestman
Copy link
Contributor Author

LarryWoestman commented Dec 25, 2024

I would consider "G1 X0 Y0 Z0 A90 B0 C0 F200" to be non-optimal "internal g-code data structures" at best. It should be "G1 A90 F200" instead, which would work with the existing logic. Why specify parameter data structures that don't have motion?
I would consider generating such data structures in FreeCAD to be poor design, but I suppose it could happen. Should such code in FreeCAD be considered "buggy" or acceptable (if it exists)?
I just realized that what we probably mean by "X0 Y0 Z0..." in the line is that the linear axes don't move from wherever they currently are rather than that they move to 0. This would be harder to recognize in the logic, but I think it would still be possible.

@maxwxyz maxwxyz requested a review from sliptonic January 4, 2025 08:36
@DerAndere1
Copy link

DerAndere1 commented Jan 4, 2025

Well, in addition to my example above, the Last G1 command in each of the following G-Code snippets should also be handled as pure rotational moves. Note how the Interpretation of the axis words depends on the active motion mode (G90, absolute positioning vs G91, relative positioning).

G91 (incremental positioning mode)
G1 X0 Y0 Z0 A90 B90 C90 F200

And

G90 (absolute positioning mode)
G1 X1 Y1 Z1 A90 B90 C90 F200
G1 X1 Y1 Z1 A0 B0 C0

I would even say that in certain situations it's good practice to explicitely state all(!) axis positions in abolute positioning mode, even if some axes would not move (e.g. before/after tool changes, G49, G43 etc.)

@LarryWoestman
Copy link
Contributor Author

You appear to be discussing what gcode is generated by a postprocessor where I am asking questions about what "internal data structures" are generated by the rest of FreeCAD. In other words, what is the expected "input" to the postprocessor more than what "output" the postprocessor should generate.
Writing code to handle cases that shouldn't exist is usually not a valuable use of time. Writing code to handle cases that will exist is necessary.

@LarryWoestman
Copy link
Contributor Author

LarryWoestman commented Jan 5, 2025

The more I think about this, the more ways I can think of for the rest of the FreeCAD code to unintentionally generate "internal data structures" that just happen to have the XYZUVW axes with the same coordinates as the previous "internal data structure".
I need to change the code to cope with this possibility even if the rest of the FreeCAD code doesn't do this at the moment. It could easily change to do this in the future without realizing the effects on this code.

I have changed this PR to "draft" mode while I work on it some more.

@LarryWoestman LarryWoestman marked this pull request as draft January 5, 2025 19:01
@sliptonic
Copy link
Member

I would consider "G1 X0 Y0 Z0 A90 B0 C0 F200" to be non-optimal "internal g-code data structures" at best. It should be "G1 A90 F200" instead, which would work with the existing logic. Why specify parameter data structures that don't have motion? I would consider generating such data structures in FreeCAD to be poor design, but I suppose it could happen.

Zen of Python rule #2: Explicit is better than implicit.

It's not optimal gcode but it is more explicit. Given a command like "G1 A90 F200", it isn't possible to infer the current value of X. You either have to work your way backwards through the previous commands or you need to create a state tracking machine to keep track of things as you go along.

It's trivial to make the final gcode more optimal and efficient. Harder to go the other way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Mod: CAM Related to the CAM/Path Workbench Needs backport Needs backport to 1.0.1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CAM: The refactored* postprocessors don't handle the feed rate correctly when rotary axes are involved
4 participants