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

Further cleanup of UBL #6552

Merged
merged 9 commits into from
May 3, 2017

Conversation

thinkyhead
Copy link
Member

@thinkyhead thinkyhead commented May 2, 2017

Rebase #6501 and apply more cleanup…

  • Fix a bug in smart_fill_mesh
  • Reduce code size of smart_fill_mesh
  • Use 'PSTR' for ::debug methods' title argument
  • Fix formatting, apply coding standards

Concise diff

@Bob-the-Kuhn
Copy link
Contributor

In UBL_G29.cpp, please replaces all the X_MIN_POS ... travel limits with the UBL_MESH_MIN_X ... versions.

@thinkyhead
Copy link
Member Author

thinkyhead commented May 2, 2017

How about in G26?

@thinkyhead thinkyhead force-pushed the rc_more_ubl_cleanup branch 2 times, most recently from 6fa7f38 to 3494c9a Compare May 2, 2017 23:54
safe_delay(20);
for (uint8_t i = 0; i < GRID_MAX_POINTS_X; i++) {
for (uint8_t j = 0; j < GRID_MAX_POINTS_Y; j++) {
float x_tmp = pgm_read_float(ubl.mesh_index_to_xpos[i]),
Copy link
Contributor

Choose a reason for hiding this comment

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

Need an ampersand here, I believe.

float x_tmp = pgm_read_float(ubl.mesh_index_to_xpos[i]),
should be
float x_tmp = pgm_read_float(&ubl.mesh_index_to_xpos[i]),

Same with the next line.

Copy link
Member Author

Choose a reason for hiding this comment

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

!!!

Copy link
Member

Choose a reason for hiding this comment

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

Agreed!

@thinkyhead thinkyhead force-pushed the rc_more_ubl_cleanup branch 15 times, most recently from ccc4676 to 6843d0e Compare May 3, 2017 03:18
@thinkyhead thinkyhead force-pushed the rc_more_ubl_cleanup branch from 6843d0e to 7ba7474 Compare May 3, 2017 03:22
@thinkyhead thinkyhead merged commit 6283913 into MarlinFirmware:RCBugFix May 3, 2017
@thinkyhead thinkyhead deleted the rc_more_ubl_cleanup branch May 3, 2017 03:46
@Roxy-3D
Copy link
Member

Roxy-3D commented May 3, 2017

How about in G26?

Yes... for now... that is probably the right thing to do. But I suspect G26 has value to the Bi-Linear and original Mesh bed leveling. In which case, using the UBL constants is not the 'right' thing to do...

@thinkyhead
Copy link
Member Author

If it's "right" for UBL then it's ok. It should be self-consistent.

@Roxy-3D
Copy link
Member

Roxy-3D commented May 3, 2017

Wait, is this correct?
The first three are squared, but the three others are not!

Good catch! This is wrong.... These are xy, xz and yz terms in the polynomial. They are not x^2, y^2 or z^2 terms. The code will NOT work like this.

-  lsf->x2bar = lsf->x2bar / N - lsf->xbar * lsf->xbar;
 -  lsf->y2bar = lsf->y2bar / N - lsf->ybar * lsf->ybar;
 -  lsf->z2bar = lsf->z2bar / N - lsf->zbar * lsf->zbar;
 -  lsf->xybar = lsf->xybar / N - lsf->xbar * lsf->ybar;
 -  lsf->yzbar = lsf->yzbar / N - lsf->ybar * lsf->zbar;
 -  lsf->xzbar = lsf->xzbar / N - lsf->xbar * lsf->zbar;
 +  lsf->x2bar = lsf->x2bar / N - sq(lsf->xbar);
 +  lsf->y2bar = lsf->y2bar / N - sq(lsf->ybar);
 +  lsf->z2bar = lsf->z2bar / N - sq(lsf->zbar);
 +  lsf->xybar = lsf->xybar / N - sq(lsf->xbar);
 +  lsf->yzbar = lsf->yzbar / N - sq(lsf->ybar);
 +  lsf->xzbar = lsf->xzbar / N - sq(lsf->xbar);

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.

5 participants