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 vector normalization in JD #17938

Merged
merged 4 commits into from
May 11, 2020

Conversation

XDA-Bam
Copy link
Contributor

@XDA-Bam XDA-Bam commented May 10, 2020

Description

As far as I can see in the code, block->millimeters and thereby inverse_millimeters does not include extruder steps (see this code). Using inverse_millimeters to normalize unit_vec will therefore lead to incorrect results for non-travel moves and cause junction_cos_theta to be incorrect as well. This PR fixes that by correctly normalizing unit_vec in case the extruder moves.

Note: I'm not too familiar with the Marlin codebase, so please have a good look at this to make sure I'm not missing something. If I'm wrong and block->millimeters does include extruder steps, please let me know and I'll close this.

Benefits

  • unit_vec will actually be a unit vector after normalization.
  • Computation of junction_cos_theta will be correct for non-travel moves.

Related Issues

Might affect #17920, because the incorrect normalization should result in a further reduced limit_sqr. The effect might be too small to notice, though.

XDA-Bam and others added 4 commits May 10, 2020 14:12
As far as I can see in the code, inverse_millimeters does not include extruder steps. Using inverse_millimeters to normalize unit_vec will therefore lead to incorrect results for non-travel moves and cause junction_cos_theta to be incorrect as well.
@thinkyhead thinkyhead merged commit 3381a4a into MarlinFirmware:bugfix-2.0.x May 11, 2020
vgadreau pushed a commit to vgadreau/Marlin that referenced this pull request May 29, 2020
jmp0x0000 pushed a commit to jmp0x0000/Marlin that referenced this pull request Aug 7, 2020
njibhu pushed a commit to njibhu/Marlin that referenced this pull request Aug 24, 2020
HairingX pushed a commit to HairingX/Marlin that referenced this pull request Jun 16, 2021
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.

2 participants