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 G2/G3 rounding error #15507

Merged
merged 2 commits into from
Oct 11, 2019
Merged

Fix G2/G3 rounding error #15507

merged 2 commits into from
Oct 11, 2019

Conversation

edwilliams16
Copy link
Contributor

@edwilliams16 edwilliams16 commented Oct 10, 2019

Description

This fixes the problems from rounding error discussed in #14745
and an error introduced in the refactoring 56595a4

Related Issues

fixes #14745

Fix error introduced in #15204
Fix problem when rounding of arc radius by slicer made arc geometrically impossible.
@edwilliams16 edwilliams16 changed the base branch from 1.1.x to bugfix-2.0.x October 10, 2019 18:13
@thinkyhead
Copy link
Member

Good fix, thanks!

@thinkyhead thinkyhead merged commit d8aeeb8 into MarlinFirmware:bugfix-2.0.x Oct 11, 2019
@thinkyhead thinkyhead changed the title G2 g3 fix Fix G2/G3 rounding error Oct 11, 2019
@Evg33
Copy link
Contributor

Evg33 commented Oct 11, 2019

DUE_USB FAILED

Marlin\src\gcode\motion\G2_G3.cpp: In static member function 'static void GcodeSuite::G2_G3(bool)':
Marlin\src\gcode\motion\G2_G3.cpp:293:46: error: expected ',' or ';' before '/' token
           const xy_pos_t s = { -d2.y, d2.x } / len;      // Unit vector along perpendicular bisector
                                              ^
*** [.pio\build\DUE_USB\src\src\gcode\motion\G2_G3.cpp.o] Error 1

robbycandra added a commit to robbycandra/Marlin that referenced this pull request Oct 11, 2019
Copy link
Contributor Author

@edwilliams16 edwilliams16 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{ -d2.y, d2.x } / len didn't compile for me either
{-d2.y, d2.x} * RECIPROCAL(len) looked slower (1 divide, 2 multiplies instead of two divides)

so I used {-d2.y/len, d2.x/len}
The alternative of overloading / for the xy_pos_t type was more than I wanted to tackle for this instance, as it would likely result in the same compiled code.

@edwilliams16 edwilliams16 deleted the G2_G3_fix branch October 11, 2019 04:34
@thinkyhead
Copy link
Member

Casting was needed. xy_pos_t({ -d2.y, d2.x }) / len. But we just moved the divide to the next line.

rolkun pushed a commit to rolkun/Marlin that referenced this pull request Oct 25, 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.

None yet

3 participants