gcode: fix ROTATION_ENABLE 2nd-axis offset double-applied on rotated G54 moves#984
Merged
terjeio merged 1 commit intoJul 18, 2026
Conversation
…G54 XY moves In the both-rotation-axes branch, bit_false(r_around.mask, bit(idx)) used the stale outer loop variable idx instead of idx_0. When idx != idx_0, idx_1 (the second rotation-plane axis) came out wrong, so in the final offset loop that axis was not recognized as a rotation axis and its WCS offset was applied a second time. Result: a rotated G54 move with both X and Y present targeted (work + 2x offset) on the second axis -> false soft-limit (Alarm:2). Reproduced on hardware; only a nonzero WCS rotation + a full-plane G54 move triggers it. Fix: clear bit(idx_0). Upstream grblHAL bug - PR-worthy. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
terjeio
reviewed
Jul 18, 2026
| } else { | ||
| idx_0 = ffs(r_around.mask) - 1; | ||
| bit_false(r_around.mask, bit(idx)); | ||
| bit_false(r_around.mask, bit(idx_0)); // clear the FIRST rotation axis to find the second (was bit(idx) - a stale loop var, which mis-set idx_1 -> the second axis's WCS offset got applied twice -> false soft-limit on rotated G54 XY moves) |
Contributor
There was a problem hiding this comment.
I'll strip this comment when committing to my upstream repo - it is not needed here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug
In the both-rotation-axes branch of
gc_execute_block()(theROTATION_ENABLEG53/rotation handling added around the same time as #977), the second rotation-plane
axis index is derived like this:
bit_falseclears using the outer loop'sidxinstead of the just-computedidx_0.When
idx != idx_0,idx_1(the second rotation-plane axis) comes out wrong, so inthe final offset-application loop that axis is no longer recognized as a rotation
axis and its WCS (G54) offset gets applied a second time. Net effect: a rotated G54
move that has both axes of the active plane in the same block targets
work_offset + 2xon the second axis instead of the correct position, which reliablyproduces a false soft-limit trip (
ALARM:2) once that lands outside travel.Reproduced on hardware (Teensy 4.1 / grblHAL_Teensy4): only triggers with a nonzero
WCS rotation (
G54with$rotation set) plus a full-plane move (both X and Y wordspresent, e.g.
G54 G0 X10 Y10).Fix
Clear
bit(idx_0)instead of the stalebit(idx)— one line.Testing
Verified on real hardware: rotated G54 XY moves land at the correct target and no
longer false-trip the soft limit.