MarlinFirmware / Marlin

Marlin is an optimized firmware for RepRap 3D printers based on the Arduino platform. Many commercial 3D printers come with Marlin installed. Check with your vendor if you need source code for your specific machine.
https://marlinfw.org
GNU General Public License v3.0
16.3k stars 19.24k forks source link

Is probe repeatability test valid with PROBE_MANUAL ? #7202

Closed fiveangle closed 7 years ago

fiveangle commented 7 years ago

If so, it's failing:

Marlin_main.cpp: In function 'void gcode_M48()':
Marlin_main.cpp:6822: error: 'probe_pt' was not declared in this scope
     const float t = probe_pt(X_probe_location, Y_probe_location, stow_probe_after_each, verbose_level);
                                                                                                      ^
Marlin_main.cpp:6944: error: expected primary-expression before ')' token
     if (STOW_PROBE()) return;
                     ^
Using library U8glib at version 1.19.1 in folder: /Users/speedster/Documents/Arduino/libraries/U8glib 
Using library Wire at version 1.0 in folder: /Applications/Arduino 1.8.2-teensy.app/Contents/Java/hardware/teensy/avr/libraries/Wire 
'probe_pt' was not declared in this scope

I would think it isn't valid, and the repeatability probe test within SanityCheck.h needs to be moved into the PROBE_DEFINED section of the conditional block and augmented so it will thrown an error if only PROBE_MANUAL is defined.

Does someone know which is correct ?

thinkyhead commented 7 years ago

A simple test for the combination of repeatability test plus probe_manually would work. Or, maybe HAS_REAL_PROBE and CAN_PROBE should replace HAS_BED_PROBE — each in the appropriate instance. But also, yes, it should still throw a sanity error for the combination of repeatability and manual probing.

thinkyhead commented 7 years ago

Oh wait, I see HAS_BED_PROBE already excludes PROBE_MANUALLY

thinkyhead commented 7 years ago

7212 should prevent this.

Roxy-3D commented 7 years ago

Oh wait, I see HAS_BED_PROBE already excludes PROBE_MANUALLY…

This feels like the wrong direction. If a machine is setup to do manual probes... It does have a way to measure points. It isn't automatic. It is a real nuisance. But it does have a bed probe.

And here is why I'm making the point... I think going forward we need to embrace what ever probes are available. We can do automatic probing with 'real' probes. But we can still do manual probing in locations that the real probe can not reach.

Maybe a name change of HAS_BED_PROBE to HAS_MACHINE_CONTROLLED_PROBE or something similar makes sense? Going forward, we probably want to be able to use what ever probes are available.

fiveangle commented 7 years ago

@Roxy-3D - I see your logic here, and normally agree, but there's also the concept of letter of the law, and spirit of the law. Is there any universe where a user would sit in front of a probe with a paper and beleive the values were different between each move of the nozzle ? It just doesn't make sense in this case.

If a users mechanicals are failing and they want to see if the values change between movements of the nozzle, they will see it immediatley during the measurement, so running the test as though it were autonomous, and comparing the results afterward, doesn't add any value here.

Preventing this feature with PROBE_MANUAL benefits more to guide unsavvy users to better understand the feature and what it is intended for if they enable with PROBE_MANUAL and observe the sanity check warning. For the 1-in-1000 savvy users who may have some minor mechanical issue on their bot and they wish to run the process for OCD accademic reasons to see if their own tactile skills are up to snuff is really outside the scope, don't you think ?

[I'm now imagining the new latest game sweeping the nation... PROBE_MANUAL REPEATABILITY_TEST championships - How accurate is your business card ? :]

github-actions[bot] commented 3 years ago

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.