Fix autoleveller interpolation bug and make it use correct variable numbers #491
Conversation
This file contains 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
eyal0
reviewed
Dec 7, 2020
| @@ -414,9 +410,7 @@ X-4.81601 Y-2.92779 Z[#3+-0.04000] | |||
| #2=[#516+[#517-#516]*0.35816] | |||
| #3=[#1+[#2-#1]*0.99789] | |||
| X-4.81377 Y-2.93518 Z[#3+-0.04000] | |||
| #1=[#516+[#517-#516]*0.33331] | |||
| #2=[#519+[#520-#519]*0.33331] | |||
There was a problem hiding this comment.
Great! I see here that we used to access a variable that is larger than #517, but in the new code, there is no usage of variables greater than #517. Excellent!
|
I ran these commands on your PR: The code is unchanged but the integration tests are fixed. There is a new bug in CI, I think that the MacOS environment broke. I'll fix that and then we'll be able to submit this with working CI. Thank you for your efforts. Very well done! |
If the autoleveller wants to calculate the height of a point that is directly between two measured points, it will interpolate between just those two points, instead of using all four points in the adjacent square. This fixes a bug where, if the point being calculated was at the far left or top of the design, the resulting G code would ask for points off the edge of the design that hadn't been measured.
The autoleveller's constructor generates a collection of unique variable numbers that it can use when calculating its interpolated heights, however these where only used in one place (when moving the tool vertically down at the start of a line) and variables 1, 2 and 3 were used in the rest of the code. This caused the output G code to reference the wrong variable when calculating the milling depth at the start of a line. The autoleveller now uses its generated variables throughout its code.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This should fix issue #490 by making the making the interpolation algorithm only uses two points if it's directly between them.
Also, the autoleveller's constructor generates a collection of unique variables, but these were only being used in one place, with variables
#1-#3being used everywhere else. This change makes it so that the generated variables are used throughout the code.