search

Klipper3d / klipper

Klipper is a 3d-printer firmware
GNU General Public License v3.0
8.98k stars 5.17k forks source link

[Probe] Decouple Homing from probing Session. #6616

Open viesturz opened 2 weeks ago

viesturz commented 2 weeks ago

The probe refactor is very helpful in making things more modular. However the Homing and Probing are stil tied together. This is a one-liner to make them independant. Allowing to have multiple probes and assigning homing to one.

Related: https://github.com/viesturz/klipper-toolchanger/commit/83873f82a9a64de35cafe698965efa2ef51f7adc

KevinOConnor commented 1 week ago

Thanks. Some feedback:

  1. Any change to the main Klipper repo would need to come along with some user facing functionality. We don't make changes to the main Klipper repo to facilitate out-of-repo modifications of the code.
  2. I originally had the HomingViaProbeHelper as separate from ProbeSessionHelper, but I kept running into subtle dependencies. Ultimately I concluded that the "multi-probing" code is tightly coupled to the "descend until trigger" code. So, it's not clear to me that this change will be helpful.

-Kevin

viesturz commented 1 week ago

Can you expand on the subtle dependencies? As it stands there is no overlap in the code. Genuinely trying to understand what is going on.

KevinOConnor commented 1 week ago

Some core issues I ran into:

  1. It doesn't really make sense to "average multiple samples" unless one is using "descend until trigger". Probes that can do "scan probing" don't need nor want to "average multiple samples". So, from a high-level perspective it's not clear that it makes sense to separate those classes.
  2. Confusingly, there are currently three different interfaces to the low-level probe hardware: a) start_probe_session(), run_probe(), pull_probed_results(), end_probe_session(); b) multi_probe_start(), probe_prepare(), homing_move_begin(), homing_move_end(), probe_finalize(), multi_probe_end(); and c) multi_probe_start(), probing_move() (which calls probe_prepare(), homing_move_begin(), homing_move_end(), probe_finalize() ), multi_probe_end(). If HomingViaProbeHelper is not instantiated then mode "c" breaks as probe_prepare() and probe_finalize() are not invoked. As before, the low-level probe interface is overly confusing - but it looked to me that fixing it will need a rework of the homing interface - which will take some time to accomplish.

-Kevin