Skip to content

'G29 H' to set a centered square probe area (linear, bilinear)#12242

Merged
thinkyhead merged 2 commits intoMarlinFirmware:bugfix-2.0.xfrom
MasterPIC:Customized
Oct 28, 2018
Merged

'G29 H' to set a centered square probe area (linear, bilinear)#12242
thinkyhead merged 2 commits intoMarlinFirmware:bugfix-2.0.xfrom
MasterPIC:Customized

Conversation

@MasterPIC
Copy link
Contributor

G29 - Added G parameter to quickly set dimension of square centered probing area.
It affects only linear and bilinear automatic bed leveling.
G29_SQUARE_GRID_SIZE is to be enabled to activate the feature.

…rea.

G29_SQUARE_GRID_SIZE is to be defined to enable the feature.
@Roxy-3D
Copy link
Member

Roxy-3D commented Oct 27, 2018

You are going to find great resistance to using G, M, and N as parameters.

You need to use another letter that isn't a GCode command.

@MasterPIC
Copy link
Contributor Author

I already made a PR with a different and still wrong letter in the past:
#7923

Now I should re-edit all config files just for a wrong comment. :(

@Roxy-3D
Copy link
Member

Roxy-3D commented Oct 27, 2018

Yes. S and P are widely used across many GCode (and MCode) commands.

@MasterPIC
Copy link
Contributor Author

I see S is already used as argument.
It seems only H, I, K and U are still free. What do you suggest?

@Roxy-3D
Copy link
Member

Roxy-3D commented Oct 27, 2018

I personally don't have a preference. This may not be possible in this case, but I try to name the option something that the parameter letter makes sense. (I know you tried to do this with the 'G'). But if we can't come up with a name or title for the option that aligns with H, I, K or U... I guess we just pick one at random.

front_probe_bed_position = (MAX_PROBE_Y + MIN_PROBE_Y-dimens)/2;
back_probe_bed_position = front_probe_bed_position + dimens;
}
else
Copy link
Member

Choose a reason for hiding this comment

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

Tabs are verboten. Please use two spaces for indentation.


// Enable G29 G parameter to set dimension of square centered probing area
//#define G29_SQUARE_GRID_SIZE

Copy link
Member

@thinkyhead thinkyhead Oct 27, 2018

Choose a reason for hiding this comment

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

Please strip all trailing whitespace from these new lines, or better yet, just drop this extra configuration option altogether and make it a standard part of UBL.

Copy link
Member

Choose a reason for hiding this comment

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

@thinkyhead I believe this only affects:

#if ENABLED(AUTO_BED_LEVELING_LINEAR) || ENABLED(AUTO_BED_LEVELING_BILINEAR)

Copy link
Member

Choose a reason for hiding this comment

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

Ah, quite so!


if (parser.seen('G')){
int dimens = (int)parser.linearval('G');
left_probe_bed_position = (MAX_PROBE_X + MIN_PROBE_X-dimens)/2;
Copy link
Member

Choose a reason for hiding this comment

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

Please include a single space before and after all mathematical operators.

@thinkyhead thinkyhead changed the title G29 - G parameter to set dimension of square centered probing area 'G29 H' to set a centered square probe area (linear, bilinear) Oct 28, 2018
@thinkyhead
Copy link
Member

thinkyhead commented Oct 28, 2018

  • Went with 'H' because it's shaped like a square with a line in the center. 😝
  • Using value_linear_units instead of linearval('H') because parser.seen('H') primes it.
  • Applied constraints so the square won't be out of bounds.
  • Made the parameter always-on because it's inexpensive.

@thinkyhead thinkyhead merged commit 6fbc873 into MarlinFirmware:bugfix-2.0.x Oct 28, 2018
@Roxy-3D
Copy link
Member

Roxy-3D commented Oct 28, 2018

Thank You @thinkyhead

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

Comments