[bugfix-1.1.x] No longer possible to specify area in which probe works since #10019

[gloomyandy]

Prior to #10019 it was possible to specify an area of the bed that was to be used for probing (by specifying MIN_PROBE_X etc.). This was very useful for probes which are not accurate when used near to the edge of the bed. Following #10019 it seems that it is no longer possible to do this as the probable area is now considered to be everything that is reachable and within the bed area.

[ManuelMcLure]

This is a huge problem for those of us with inductive probes, since they do not work well close to the edge of the bed. I was going to try bugfix-1.1.x this weekend but this is a show-stopper.

[thinkyhead]

…it was possible to specify an area of the bed…

Probing should still only occur within the region defined by MIN_PROBE_X, etc. The change in #10019 simply gives callers more accurate information concerning the region of the bed that is reachable vs. unreachable so they can catch themselves doing bad things. It does not (or should not) change the behavior of the probing procedure itself, unless the caller is mis-using the function.

It would help to have additional information such as we request in the issue template. Describe the behavior you are seeing versus the behavior you expect, etc. Be sure to use the latest code for testing.

[gloomyandy]

I don't think this is the case when using UBL G29 P1 to automatically probe the mesh. This code makes use of of find_closest_mesh_point_of_type and find_furthest_invalid_mesh_point to find the next mesh point to probe. Both of these seem to rely only on position_is_reachable_by_probe to determine if the point should be probed. They do not do any additional tests on MIN_PROBE_X etc.

So here are the details...

Steps to Reproduce

My configuration uses UBL and has an 8x8 mesh size. I use an inductive probe and have defined MIN_PROBE_X etc. to provide a 10mm margin around the edge of the bed as the probe does not provide reliable readings in this area.

Run G28 followed by G29 P1 Expected behavior: [What you expect to happen] The bed should be probed automatically but the mesh points along the edge of the bed (all four sides) should not be probed as they either are outside the previously defined probable area (front and left side) or are not reachable by the probe (or both!) (rear and right side). So in this case only a 6x6 set of points should be probed (this was the case prior to change #10019)

Actual behavior: [What actually happens] The automatic probe happens but points along the front and left edges are probed and as expected return invalid values, the probe has to move much closer to the bed as it only "sees" part of the bed (in some cases this could result in the nozzle hitting the bed). So we end up with a 7x7 set of points being probed (the rear and right edges are not probed as they are not reachable).

Additional Information

Configuration file: https://github.com/gloomyandy/Marlin/blob/912f08a1c128495458d39914a799b41a785efa03/Marlin/Configuration.h

Version is bugfix-1.1.x as of a few minutes ago

[thinkyhead]

It makes no sense for find_closest_mesh_point_of_type and find_furthest_invalid_mesh_point to look beyond the mesh bounds for a candidate point. I will check them out and if needed devise a patch.

[thinkyhead]

The configuration you linked to has its MESH_INSET set to 0 and no override of MESH_MIN_X etc. (also I see no changes to your Conditionals_post.h, which is ok). Are you sure that's the configuration you're using? Because that would not get you a 10mm margin.

My reading of find_closest_mesh_point_of_type shows it to correctly look for mesh points only within the defined MESH_MIN/MESH_MAX boundaries. About to look at find_furthest_invalid_mesh_point…

…

…and, again, it only looks for actual mesh points within the defined bounds.

Double-check your MESH_INSET to make sure it's set to 10 and if you are, in fact, using a different configuration than the one you linked to, please post that.

[ManuelMcLure]

MESH_INSET not only limits the probable area, it also limits the size of the mesh and thus the area of the bed that can have UBL correction. Before 10019, we could specify the mesh to be the full size of the bed (MESH_INSET 0) yet use MIN_PROBE_X, etc. to limit the probing to the area where the probe works correctly. Then we could use G29 P1 to probe the probable area without danger of nozzle crashes due to probing too near the edge of the bed, and G29 P3/G29 P4 to fill in and tune the points where the probe doesn't reach. That is no longer possible since 10019. So we're left with either buying a different probe, losing print area, or working without ABL.

[thinkyhead]

That is no longer possible since 10019. So we're left with either buying a different probe, losing print area, or working without UBL.

Well, I don't want to force anyone to have to buy new probes!

However, PR 10019 makes a much-needed correction to the function which generally determines whether the probe can reach a given point within the bed — at all. That function needs to be agnostic about the type of probe or whether the probe gets confused by edges — and it fixes an issue for people who don't have your particular probe.

The fact that your probe was being kept from the edges was a lucky fluke. I can see why inductive probe users like the old bug. But it was a bug! –not a feature– and so now we need to provide a proper solution for your situation.

Given your settings, your probe should be allowed to move near the edge of the bed. There's nothing in Marlin that takes special accounting of inductive probes, and if you set no MESH_INSET then there's no explicit reason for Marlin to constrain it.

Here's what hasn't changed: If the MESH_INSET is set to 0 your mesh bounds still encompass the whole bed.

But here's what needs to change: As suggested by @Roxy-3D elsewhere, we need a new setting to tell Marlin not to probe within X millimeters of the edges (for inductive probes), and that new setting will need to be added to the position_is_reachable_by_probe function.

[thinkyhead]

I've added the PROBE_INSET option so you can tell Marlin not to probe close to the edge of the bed. All probing functions will respect this inset.

[gloomyandy]

Why do you need PROBE_INSET when MIN_PROBE_X etc already exists? Surely that is exactly what those values are for? We just need the probing code to respect the limits set by MIN_PROBE_X, MIN_PROBE_Y etc. I suppose PROBE_INSET might be used as a convenient way to set the values of MIN_PROBE_X etc. to give a margin but it is easy to set the values to do this. See line 811 and onwards in my Configuration.h file for details of how I set this.

As mentioned above the mesh limits should not need to be the same as the limits on the probable area. I do not want to use MESH_INSET as I want the mesh to cover the entire bed. I just want a way to tell Marlin which parts of the bed the probe works in and as far as I can see that is what MIN_PROBE_X etc. is intended for.

Is there some confusion here between mesh and probe limits, from my reading of the code the two should be separate. I would hope that all probing operations would respect the settings of MIN_PROBE_X etc. The setting of limits on the mesh size should not (and as far as I can tell does not) make any difference to the values set in MIN_PROBE_X etc. (if it does then perhaps that is a bug). Could you provide a little background on the bugs that 10019 is fixing? Looking at the code it seems to me that if you do not make explicit changes to them then MIN_PROBE_X etc. end up defining an area that is the same as that specified by the new version of position_is_reachable_by_probe. If it is intended that MIN_PROBE_X etc. only applies to levelling then perhaps the macros should be renamed? I do agree that the name of the function position_is_reachable_by_probe was incorrect with the old definition, perhaps there should be a function is_point_probable that respects both the reachable points and the limits imposed by MIN_PROBE_X etc. But I must admit I'm struggling to see when you would want to be able to move the probe to locations in which it will not work (e.g. outside of the area defined by MIN_PROBE_X etc.).

[thinkyhead]

MESH_INSET … If it is intended that MIN_PROBE_X etc. only applies to levelling

Correct. These settings only apply to leveling. They determine the bounds of the leveling grid. They are not pertinent to probes generally. At this time we're unlikely to change the name.

[gloomyandy]

These settings only applies to leveling. Those determine the bounds of the leveling grid. They are not pertinent to probes generally. At this time we're unlikely to change the name.

Hmm well I guess that explains some of the confusion. But doesn't MESH_MIN_X etc. already define the levelling grid limts? Also the comment in Conditionals_post.h:

 /**
   * Bed Probing rectangular bounds
   * These can be further constrained in code for Delta and SCARA
   */
 #if ENABLED(DELTA)
    // Probing points may be verified at compile time within the radius
    // using static_assert(HYPOT2(X2-X1,Y2-Y1)<=sq(DELTA_PRINTABLE_RADIUS),"bad probe point!")
    // so that may be added to SanityCheck.h in the future.
    #define _MIN_PROBE_X (X_CENTER - DELTA_PRINTABLE_RADIUS)
    #define _MIN_PROBE_Y (Y_CENTER - DELTA_PRINTABLE_RADIUS)

Implies that PROBE_MIN_X etc. is a general limit on the probable area and not levelling specific. Should the above defines be in some sort of conditional so they are only used when levelling is enabled?

However if the above is the case. Then rather than add PROBE_INSET would it not just make more sense to modify the levelling code to simply respect PROBE_MIN_X etc?

[thinkyhead]

Ah, right. I forgot the (MIN|MAX)_PROBE_[XY] options also exist distinct from the MESH boundaries. The change I made in 10019 acts as a complete programmatic replacement for (MIN|MAX)_PROBE_[XY], which simply pre-calculate the same limits, taking the hard-set probe XY offsets into account. Anyway, those are unaffected by MESH_INSET.

The reason I likely forgot is because I've been working on a project where probe and tool offsets are variable, and there are no hard-set limits based on hard-set probe offsets in that situation.

[gloomyandy]

No problem. Are you going to leave things as they are? If so I'll test the new PROBE_INSET code. If not let me know when things are as you want them to be and I'll give that code a try.

For what it is worth I do think that MIN_PROBE_X etc. is a more general solution (though slightly more difficult to use) than PROBE_INSET. Though as you say the (old) implementation would not work very well if the probe/tool offsets are not fixed.

Thanks again for taking the time to look into this!

[thinkyhead]

Actually, I've made a change. The existing setting MIN_PROBE_EDGE works the way I intended the added PROBE_INSET setting to function, so I've just moved MIN_PROBE_EDGE up and made it a general probe option.

With additional tweaks (restoring some of the old code) MIN_PROBE_X can be used to constrain it further, if needed.

[gloomyandy]

Let me know when the final version is available (in bugfix-1.1.x) and I'll be sure to test it.

[thinkyhead]

You can give this branch a test anytime now: https://github.com/thinkyhead/Marlin/tree/bf1_inductive_probe_inset

[gloomyandy]

OK just run a few quick tests. I removed my custom values of MIN_PROBE_X etc. from Configuration.h and made sure there was a MIN_PROBE_EDGE set to 10 in my case (which I think is currently the default). I then ran a test and things worked but not exactly as expected. I ended up with an 8x8 fully probed grid, the MIN_PROBE_EDGE was respected so it did not attempt to probe near the edge. However it had reduced my mesh side by 10mm on each side. I guess this is as it should be and is inline with replacing MESH_INSET by MIN_PROBE_EDGE.

To get things to work how I wanted them (mesh covering entire bed, probe not used within 10mm of the edge), I had to add extra defines for MESH_MIN_X etc. With these additional settings I can get exactly the same behaviour as I had before and as far as I can tell with the testing so far everything seems to work fine. With these settings there is an unprobed area of the mesh all around the edge of the bed that I have to fill in by other means. This is what I expected/wanted.

I'm not sure if merging MESH_INSET and MIN_PROBE_EDGE is a good idea or not. With the current setting of MIN_PROBE_EDGE (value 10, which works well for inductive probes), then some/many users may end up with a different sized mesh to what they had before (one that is smaller than the bed by a 10mm inset). I'm not sure if this is good or bad. On the one hand the entire mesh can be probed, but the new mesh does not cover all of the bed. It also seems slightly wrong that users will need to use an "advanced setting" to get things back to how they were before. I'm not saying what there is now is bad, just different.

Let me know if I can help with anything more.

[ManuelMcLure]

Although I personally can live with explicitly overriding MESH_[MIN|MAX]_[X|Y], I think from a standpoint of breaking as few existing configurations as possible it makes more sense to keep MESH_INSET and MIN_PROBE_EDGE separate. I would suggest that the default mesh size should be the bed size minus MESH_INSET (adjusted for nozzle reachability) and the probing limits should be bed size minus MIN_PROBE_EDGE (adjusted for probe reachability). And G29 should only probe points which are within both the mesh and the probing limits.

[thinkyhead]

I'm not sure if merging MESH_INSET and MIN_PROBE_EDGE is a good idea or not.

…from a standpoint of breaking as few existing configurations as possible…

I agree with these points. The MESH_INSET can still be useful. It exists as an easy way to specify MESH_MIN_X,MESH_MAX_X,MESH_MIN_Y,MESH_MAX_Y with only a single setting. Like MIN_PROBE_EDGE it also refers to the bed area — (0,0) to (X_BED_SIZE,Y_BED_SIZE) for Cartesians. With that in mind, I don't mind restoring it.

To sum up, the settings that would then be available:

• MIN_PROBE_EDGE prevents the probe from going near the physical edge of the bed for all probe operations (G29, G30, M48, etc.). Needed for probes that fail when too close to edges.
• MIN_PROBE_X,MAX_PROBE_X,MIN_PROBE_Y,MAX_PROBE_Y determine the hard boundaries where probing is allowed for all probe operations. If unspecified then they default to the bed size inset by MIN_PROBE_EDGE.
• MESH_INSET is an easy way to specify the boundaries of the bed leveling mesh (or points). Maybe MESH_INSET isn't the best name because it should constrain 3-point and linear grid too. So perhaps it should be renamed toLEVELING_INSET or something…
• MESH_MIN_X,MESH_MAX_X,MESH_MIN_Y,MESH_MAX_Y determine where the mesh will be (most likely within the bed!) for all bed leveling. Anything outside this area has to be extrapolated by leveling compensation algorithms. If unspecified they default to the bed size inset by MESH_INSET.

So, after I mod the PRs they'll mainly just promote MIN_PROBE_EDGE so it doesn't only apply to G29. They'll be a bit simpler.

[ManuelMcLure]

I heartily endorse this solution. It seems like this will cover pretty much any odd configuration requirement while at the same time being easy to configure in the majority of cases.

The only thing that might be an issue is the default value for MIN_PROBE_EDGE. On the one hand, 10 will probably work for all probes and is thus the "safe" option, although it might cause some confusion when users upgrade from earlier versions and see that the probe is no longer going to MESH_INSET (assuming MESH_INSET is less than 10). 0 will keep the current behavior (better for existing users) but might cause new users who don't understand the vagaries of inductive probes to end up probing too near the edge of the bed. All in all, I would lean towards 0 as the default to avoid unexpected changes in behavior, but part of that might be 25 years working for a database company where "thou shalt not break existing configurations with new releases" was the first commandment.

[thinkyhead]

I thought it was a good idea to leave the value unchanged (thou shalt not change the value of options that are enabled by default). I guess we'll find out how many users actually use G30.

[gloomyandy]

That sounds pretty much ideal to me. I've been running with the test firmware today and it has been working really well for me. Let me know when you update things again and I'll be sure to try it again.

Thanks for taking the time to look into this and for the fast responses!

[thinkyhead]

Everything is merged now, without too much disruption of the config options. The conditionals are a bit smarter, and the code is smarter about where the probe can be used. I'm happy to take care of all the loose ends and generally clean things up. With any luck I should have 1.1.9 out the door within a couple of days.

[github-actions[bot]]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.