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

Fix typo in G34 logic #15938

Merged
merged 1 commit into from
Nov 20, 2019
Merged

Fix typo in G34 logic #15938

merged 1 commit into from
Nov 20, 2019

Conversation

Evgeny-SPB
Copy link
Contributor

Description

After commit 832cb7e logic in Z-Probing is broken.

In this Function we use downward / upward stepper sequence for probing

// iteration odd/even --> downward / upward stepper sequence
const uint8_t iprobe = (iteration & 1) ? G34_PROBE_COUNT - 1 - i : i;

In commit 832cb7e there was a name change of a few variables, and a typo occurred.
BEFORE:

const float z_probed_height = probe_at_point(z_auto_align_pos[zstepper], raise_after, 0, true);
z_measured[zstepper] = z_probed_height + Z_CLEARANCE_BETWEEN_PROBES;

AFTER:

const float z_probed_height = probe_at_point(z_auto_align_pos[i], raise_after, 0, true);
z_measured[iprobe] = z_probed_height + Z_CLEARANCE_BETWEEN_PROBES;

So we measure height in position "[i]" and put this measurement in "[iprobe]" position.

On first iteration (iteration = 0), everything is working as expected, because i = iprobe, on the next iteration i != iprobe, so G34 is not working and gives odd results.

Benefits

G34 works as expected

Related Issues

No

@sjasonsmith
Copy link
Contributor

This seems to be a minor error that was introduced during merging. The change looks right to me.

When I tested post-merge I must not have noticed it probed the points in the same order on all iterations.

I may have only re-tested post-merge with known stepper positions, so even if iteration #2 corrected in the wrong direction, my second iteration correction is sub-millimeter so I wouldn't have really noticed.

I also don't have this triple-Z printer at home, so I've only used it a handful of times since this change went in.

@thinkyhead thinkyhead merged commit 7116a86 into MarlinFirmware:bugfix-2.0.x Nov 20, 2019
philippniethammer pushed a commit to philippniethammer/Marlin that referenced this pull request Dec 21, 2019
christran206 pushed a commit to christran206/Marlin2.0-SKR-Mini-E3-1.2 that referenced this pull request Dec 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants