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

Add feature to avoid bed clips while probing #25256

Merged
merged 12 commits into from
Feb 15, 2023

Conversation

apulver
Copy link
Contributor

@apulver apulver commented Jan 20, 2023

Description

Allows user to define four square areas in Configuration.h that probe.h's "can_reach( ... )" function will reject.

Requirements

None

Benefits

Using mesh_inset or probing_margin to avoid probing clips either shrinks the mesh or needlessly excludes many points that can be probed. This will avoid just the areas of the clips and the areas in which the nozzle is above the clip.

Configurations

Relevant #defines have been added to Configuration.h

Related Issues

None?

@EvilGremlin
Copy link
Contributor

Does this 1cm of extrapolated unprobed area make any difference at all? Does it make visible quantifiable difference in print quality? I'm sure it doesn't, unless your bed literally looks like this
image
In which cale probing area is not a concern at all

@apulver
Copy link
Contributor Author

apulver commented Jan 20, 2023

Does this 1cm of extrapolated unprobed area make any difference at all? Does it make visible quantifiable difference in print quality?

This feature does not extrapolate unprobed areas. Rather, it avoids probing just those areas in which clips would interfere with the probe or nozzle, so that the user does not have to use mesh_inset or probing_margin. Depending on the probe offset, using mesh_inset for this purpose may reduce the size of the mesh significantly. If probing_margin were used, then the user would have to manually probe many extra points that the probe could have reached just fine without any interference from the clips.

@apulver
Copy link
Contributor Author

apulver commented Jan 20, 2023

Incidentally, if G29Jx is just computing a linear transform to apply to the current mesh, then it would be better to have its own separate "margin", set in farther inward than the margin (if any) used for building the mesh.

Also, I wrote the feature with UBL in mind to help the user maximize the mesh area and number of points probed. It does not rearrange the grid in bilinear leveling or anything like that (which is perhaps what EvilGremlin thought.) It's my understanding that with UBL, there is no extrapolation to outside the mesh area and that it's important that mesh_inset be as low as possible.

@EvilGremlin
Copy link
Contributor

EvilGremlin commented Jan 20, 2023

...probing_margin were used, then the user would have to manually probe many extra points

Nope, i mean both bigger mesh_insert or autofill with extrapolated values are perfectly fine when we're talking about ~1cm.

I think feature is ok in general and might find some use, just not for bed clips specifically. It drastically complicate geometry setup for no real benefit over simple mesh_inset.

@apulver
Copy link
Contributor Author

apulver commented Jan 20, 2023

Nope, i mean both bigger mesh_insert or autofill with extrapolated values are perfectly fine when we're talking about ~1cm.

Don't forget you have to combine the clip dimensions with the x/y probe offsets when you're looking to set a margin or inset. You could lose several centimeters to make both the nozzle and probe clear the clips. It matters when you're trying to print a large item.

I think feature is ok in general and might find some use, just not for bed clips specifically. It drastically complicate geometry setup for no real benefit over simple mesh_inset.

What exactly does it complicate? It's only a few comparison operations to avoid four areas adjacent to the bed perimeter with the probe and nozzle. It's probably optimized out of the compiled firmware entirely when it's disabled in Configuration.h. Many inexpensive printers come with a build surface that attaches with clips to a not-so-flat bed. I think it's very well-justified.

@EvilGremlin
Copy link
Contributor

What? No, not several, just clip size. Probe offset vs travel limits is main loss in auto probing. In fact, in most cases it's preferable to set bigger mesh_inset (20-30mm) so all points will be probed automatically.

What exactly does it complicate

Support overhead. Soooo many people can't figure out geometry setup even as it is... Ultimately it's up to Scott, but even accepting PR i wouldn't include all this in configs.

@apulver
Copy link
Contributor Author

apulver commented Jan 20, 2023

What? No, not several, just clip size. Probe offset vs travel limits is main loss in auto probing. In fact, in most cases it's preferable to set bigger mesh_inset (20-30mm) so all points will be probed automatically.

Suppose your clip extends 10mm into the bed. If the nozzle is 15mm closer to that side of the bed than the probe, then the closest point with mesh_inset would be 25mm. The opposite side still has to be inset 10mm for its clips. This is not preferable because, if I understand, UBL does not extrapolate the mesh outside its perimeter. So on a typical printer you might have 35x235mm of bed area that UBL does not apply any z correction for. This is a problem when you want to print a big item.

What exactly does it complicate

Support overhead. Soooo many people can't figure out geometry setup even as it is... Ultimately it's up to Scott, but even accepting PR i wouldn't include all this in configs.

What do you mean by "geometry setup"? This feature would in many cases obviate the need to adjust MESH_INSET and PROBING_MARGIN in the first place, which are relatively opaque. If the user does not want it, they can leave it disabled, just like other geometric features such as skew compensation.

… here are only used to compute a linear transform via least squares (which is then applied to the mesh), a small inset is fine. Otherwise, the G29J will abort if it tries to probe a point that is_reachable rejects e.g. when AVOID_BED_CLIPS is enabled. Also, parenthesize the arithmetic expressions in Configuration.h so that order of operations does not get messed up by macro expansion.
@EvilGremlin
Copy link
Contributor

Oh well, we have very different views on leveling setup logic, that's fine.

@ellensp
Copy link
Contributor

ellensp commented Jan 21, 2023

One issue I see is most systems that use clips, the position is not fixed, the clip will move position every time you clip and unclip.
Sometimes deliberately, if your trying to print a large part and the clip is in the way, you move the clip..

@thisiskeithb
Copy link
Member

One issue I see is most systems that use clips, the position is not fixed, the clip will move position every time you clip and unclip.

Yep. PROBING_MARGIN_FRONT/BACK already exists and negates the need for these changes in most cases since that is more universal / simplifies configuration setup.

@apulver
Copy link
Contributor Author

apulver commented Jan 21, 2023

One issue I see is most systems that use clips, the position is not fixed, the clip will move position every time you clip and unclip.

Yep. PROBING_MARGIN_FRONT/BACK already exists and negates the need for these changes in most cases since that is more universal / simplifies configuration setup.

Personally I always took care to position the clips the same way every time when I was using bilinear leveling in order to avoid collisions and bad probe values. It's not possible to do this with a finer grid as is typical for UBL. Suit yourselves though.

@thinkyhead thinkyhead force-pushed the bugfix-2.1.x branch 6 times, most recently from f7016f1 to 589d611 Compare February 9, 2023 04:51
@thinkyhead
Copy link
Member

thinkyhead commented Feb 15, 2023

I'm not entirely disinclined to add this since it only affects UBL, which is already quite a meticulous beast and tends to be used by meticulous humans. But it does add a lot of new defines. So, I will at least reduce the number of defines and make them into 4-element arrays, one per obstacle.

One difficulty I see with this feature, as implemented, is that you might be probing a point that is near a clip, and even though the probe is taking care to avoid the clip, the nozzle might still run into it.

thinkyhead added a commit to MarlinFirmware/Configurations that referenced this pull request Feb 15, 2023
@thinkyhead thinkyhead force-pushed the bugfix-2.1.x branch 4 times, most recently from 3edee7f to 6bc5a49 Compare February 15, 2023 05:21
thinkyhead added a commit to MarlinFirmware/Configurations that referenced this pull request Feb 15, 2023
@thinkyhead thinkyhead merged commit fc20533 into MarlinFirmware:bugfix-2.1.x Feb 15, 2023
thinkyhead pushed a commit to thinkyhead/Marlin that referenced this pull request Feb 22, 2023
LMBernardo pushed a commit to LMBernardo/Marlin that referenced this pull request Mar 19, 2023
thinkyhead pushed a commit that referenced this pull request Apr 7, 2023
EvilGremlin pushed a commit to EvilGremlin/Marlin that referenced this pull request Apr 8, 2023
thinkyhead pushed a commit that referenced this pull request Apr 10, 2023
thinkyhead pushed a commit to thinkyhead/Marlin that referenced this pull request May 16, 2023
EvilGremlin pushed a commit to EvilGremlin/Marlin that referenced this pull request May 17, 2023
tspiva pushed a commit to tspiva/Marlin that referenced this pull request May 25, 2023
Andy-Big pushed a commit to Andy-Big/Marlin_FB_Reborn that referenced this pull request Jul 9, 2023
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