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

Get bed probe position from current mesh #23

Merged
merged 3 commits into from
Feb 20, 2022

Conversation

julianschill
Copy link
Contributor

Instead of reading the position of the relative reference index from the config at startup, it now reads the position from the last measured mesh on every call of CALIBRATE_Z. If it is set in the config it will take the configured values.

Copy link

@garethky garethky left a comment

Choose a reason for hiding this comment

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

Thanks for doing the PR. I verified that this works on my printer with my script for probing the print area. I had to add code to pick a RELATIVE_REFERENCE_INDEX. That's a bit of a problem for small numbers of probed points. e.g. in a 4x4 probe grid there is no center, points 5, 6, 9 and 10 as close as you can get to center.

I was hoping for Gcode parameters to set the bed probing point directly. I have left a comment with that code change.

if self.config.getfloat('probe_bed_x', None) is None \
or self.config.getfloat('probe_bed_y', None) is None:
try:
mesh = self.printer.lookup_object('bed_mesh', default=None)
Copy link

Choose a reason for hiding this comment

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

Here is the code for checking for parameters. I tested that this also works on my printer:

probe_bed_x = gcmd.get_float('PROBE_BED_X', default = None)
probe_bed_y = gcmd.get_float('PROBE_BED_Y', default = None)
if probe_bed_x is not None and probe_bed_y is not None:
    self.probe_bed_site = [probe_bed_x, probe_bed_y]
else:
    mesh = self.printer.lookup_object('bed_mesh', default=None)
    rri = mesh.bmc.relative_reference_index    
    self.probe_bed_site = mesh.bmc.points[rri]
    logging.debug("Z-CALIBRATION probe_bed_x=%.3f probe_bed_y=%.3f"
            % (self.probe_bed_site[0], self.probe_bed_site[1]))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe I can add this as an additional option you can use, when not using a mesh. People then can define the center of the print as probe point.

Copy link
Member

Choose a reason for hiding this comment

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

There is a different PR for this and I even really don't like this idea since you can chose a different position to the RRI.

@@ -108,11 +108,6 @@ def handle_connect(self):
mesh = self.printer.lookup_object('bed_mesh', default=None)
if mesh is None or mesh.bmc.relative_reference_index is None:
Copy link

Choose a reason for hiding this comment

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

If this can now be filled out at runtime I probably should not get an error here if I don't have relative_reference_index configured in my default bed mesh. I think we could delete all the way up to line 105.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wonder what happens then, if you just run BED_MESH_CALIBRATE without parameters and you don't have a RRI defined in the config? I think we should force the users to define a RRI when using z calibration.

Choose a reason for hiding this comment

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

Not necessarily. If we run a BED_MESH_CALIBRATE for the sub-area of the bed the RRI can be passed at that time, even if this value is not defined. You are forcing it to be defined in the config but the value isn't actually read until later. Do the checking when the value is required.

Copy link
Member

Choose a reason for hiding this comment

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

I would never do this! If you rely on the RRI, then it needs to be configured properly at startup! No matter if you change or set it afterwards...

@julianschill
Copy link
Contributor Author

The Relative Reference point is the point, where the bed is Zero. By probing this point we make sure the mesh is relative to the calibrated zero. I don't see any reason to probe somewhere else if you have a mesh. It doesn’t matter which point of the mesh it is, though. It can be a corner or the middle or whatever you like. As long as you probe the same point everything is aligned.

@garethky
Copy link

garethky commented Dec 5, 2021

What you have written will work. A user can set the RRI to 0 and that should always work. The only thing it impacts (AFAIK) is how the display shows up for bed mesh. If you don't take my suggestion I'm fine with it as-is. (but see my other comment about deleting the RRI check on startup)

Copy link
Member

@TitusLabs TitusLabs left a comment

Choose a reason for hiding this comment

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

Please resolve the conflicts and change that small thing and I will merge it.

@@ -108,11 +108,6 @@ def handle_connect(self):
mesh = self.printer.lookup_object('bed_mesh', default=None)
if mesh is None or mesh.bmc.relative_reference_index is None:
Copy link
Member

Choose a reason for hiding this comment

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

I would never do this! If you rely on the RRI, then it needs to be configured properly at startup! No matter if you change or set it afterwards...

z_calibration.py Show resolved Hide resolved
if self.config.getfloat('probe_bed_x', None) is None \
or self.config.getfloat('probe_bed_y', None) is None:
try:
mesh = self.printer.lookup_object('bed_mesh', default=None)
Copy link
Member

Choose a reason for hiding this comment

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

There is a different PR for this and I even really don't like this idea since you can chose a different position to the RRI.

@TitusLabs TitusLabs linked an issue Feb 20, 2022 that may be closed by this pull request
@TitusLabs TitusLabs merged commit d2a6ace into protoloft:master Feb 20, 2022
@bjoern79de
Copy link

I'm looking ahead to this change, because I'm using a dynamic bed_mesh in my start code for each print:
https://gist.github.com/ChipCE/95fdbd3c2f3a064397f9610f915f7d02

But I'm wondering why the coords of the reference point (reported by bed_mesh_calibrate) doesn't match the probe coords of calibrate_z:

20:42:09
bed_mesh: relative_reference_index 1 is (175.95, 106.94)
...
20:43:38
Z-CALIBRATION: ENDSTOP=0.000 NOZZLE=-0.095 SWITCH=5.492 PROBE=6.057 --> OFFSET=0.370000
20:43:38
probe at 171.950,87.187 is z=6.056250

@julianschill
Copy link
Contributor Author

@bjoern79de Is this with the changed version or with the old version? Can you post your config for the mesh and the probe, please?

@TitusLabs
Copy link
Member

The calibration tries to hit this point with the probe. So the configured offset from the probe's configuration section is added to this point to get there.

@bjoern79de
Copy link

bjoern79de commented Feb 21, 2022

The calibration tries to hit this point with the probe. So the configured offset from the probe's configuration section is added to this point to get there.

Ah thanks for the hint - then it's my fault. I was not aware of the fact, that the mesh calibration logs nozzle-coords.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FR] Be able to configure the plugin at runtime
4 participants