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

Introduce the reprobe and rewipe features #3553

Closed
wants to merge 11 commits into from

Conversation

scalez
Copy link
Contributor

@scalez scalez commented Apr 18, 2016

Uses the G26 command to continue allowing G-code to enter the buffer after the final attempt has failed. This G-code should be placed at the beginning of each start G-code profile.

Uses the G26 command to continue allowing G-code to enter the buffer after the final attempt has failed. This G-code should be placed at the beginning of each start G-code profile.
@scalez
Copy link
Contributor Author

scalez commented Apr 18, 2016

Here's what this feature accomplishes: https://goblinrefuge.com/mediagoblin/u/kuzmenko/m/reprobe-and-rewipe/
Here's what happens without this feature: https://goblinrefuge.com/mediagoblin/u/kuzmenko/m/original-g29-probing/

@scalez
Copy link
Contributor Author

scalez commented Apr 18, 2016

Originally, Z would move down until -(Z_MAX_LENGTH + 10) (in our case this is -171!!!) and when -(Z_MAX_LENGTH + 10) is reached it considers it a successful probe and moves onto the next point and so on.

With this feature, if MIN_PROBE_PT is reached then it is considered a failed probe and attempts to retry the full AUTO_BED_LEVELING_GRID routine up to NUM_ATTEMPTS times.

scalez added 2 commits April 18, 2016 10:28
…6() function

The actual execution of G26 is done inside get_serial_commands(), this commit is just for completeness.
@jbrazio
Copy link
Contributor

jbrazio commented Apr 18, 2016

If I understood if the probe fails to detect the closed circuit [by going over a z-min value] it will automatically go into a wipe mode ? This seems a really nice feature but IMO as it is this PR cannot be merged for two reasons:

  • We're trying to finish the RC cycle, most new functionality [unless with high community demand] is currently frozen
  • This functionality should be inside a new probe definition because the use case for it is very specific to Lulzbots and Co.

I would propose to port this PR into MarlinDev so we can plan its merge into the Development version of Marlin.

@scalez
Copy link
Contributor Author

scalez commented Apr 18, 2016

@jbrazio for your second point, this is not specific to our method of probing because if any other probing method (photoelectric, mechanical switch, ultrasonic, capacitive, inductive) reaches the lowest probing point assigned to zPosition, as changed on L1600, which is currently -(Z_MAX_LENGTH + 10), it considers that probe successful, regardless of the physical mechanism used to trigger the probing.

Also, the rewipe is an optional feature with a preprocessor def of its own in Configuration.h; you can choose to define it on L582. If REWIPE is not applicable to your setup then you can simply enable REPROBE by itself and your machine will reattempt probing without wiping.

The current probing method could easily damage or destroy a probe (if not also the bed) which fails to trigger, again regardless of which transducer method is used. Because of this fault, I would suggest merging it with RCBugFix, even if it is disabled by default (as is in this PR).


#ifdef REPROBE
#define Z_RETRY_PT 15 // Z height to move the hotend after failed probe
#define NUM_ATTEMPTS 3 // Number of attempts to make before quiting
Copy link
Member

Choose a reason for hiding this comment

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

"giving up" (as below) is better than "quitting" here.

@jbrazio
Copy link
Contributor

jbrazio commented Apr 19, 2016

@scalez how are you currently homing the Z axis, do you have a max endstop on that axis ?

@scalez
Copy link
Contributor Author

scalez commented Apr 19, 2016

@jbrazio on the Mini we home to max on Z, TAZ 6 is also using this feature and it homes to min.

@scalez
Copy link
Contributor Author

scalez commented Apr 19, 2016

@thinkyhead c6eef47 reformatted the code, let me know what you think.

@jbrazio
Copy link
Contributor

jbrazio commented Apr 19, 2016

TAZ 6 is also using this feature and it homes to min

Using the probe as it's Z_MIN endstop (or vice-versa) ?

@scalez
Copy link
Contributor Author

scalez commented Apr 19, 2016

@jbrazio TAZ 6 has a Z_MIN and Z_MAX switch as well as a Z_PROBE.

@jbrazio
Copy link
Contributor

jbrazio commented Apr 19, 2016

Why am I doing this questions.. if a robot does not have a dedicated home switch thus uses the probe for Z_MIN, how can you know -(Z_MAX_LENGTH + 10) ?

I like the feature "not to break the bed" if a probe goes bad, but if a probe goes bad.. homing will also go bad. Otherwise, if we have Z_MIN and a Z_PROBE then isn't this the same as activating #define min_software_endstops true ?

…the MIN_PROBE_PT comment in each of the example configuration headers
@scalez
Copy link
Contributor Author

scalez commented Apr 19, 2016

@jbrazio if someone designs their machine to use their probe for homing then the probe is used to find the origin. Therefore, when the probe is triggered that would be Z=0. Regardless of the method used for homing, when a machine is homed it has found its origin (0, 0, 0). -(Z_MAX_LENGTH + 10) is just a position relative to the origin.

If a machine is using its probe for both homing and probing, and probing failed, then homing would likely also fail, this is just an inherent drawback to using a probe for homing.

If you have both a Z_MIN and a Z_PROBE then min_software_endstops is not necessary. The dedicated min switch is used for finding Z=0, this is how we do it on our TAZ 6.

@jbrazio
Copy link
Contributor

jbrazio commented May 18, 2016

All I know is based on this vid: https://youtu.be/_RzFC5FplmY?t=4m24s

@thinkyhead
Copy link
Member

No problem! I'll download their Marlin source and see what they're doing.

@thinkyhead
Copy link
Member

thinkyhead commented May 18, 2016

The mini uses simple 4-point leveling, with the "probe" set to -1.43 Z distance from the nozzle. The nozzle+clips are apparently connected to one of the endstop switches (Z min perhaps, as it seems to Home to Z Max). Software endstops are disabled, and ENDSTOPS_ONLY_FOR_HOMING is disabled, presumably as a way to keep the endstops active for G29 with the nozzle.

@scalez
Copy link
Contributor Author

scalez commented May 19, 2016

@thinkyhead we will be using software endstops in our future Minis. Refer to the source code found here: https://github.com/alephobjects/Marlin/tree/Gladiola.

@scalez
Copy link
Contributor Author

scalez commented May 19, 2016

@thinkyhead Currently, the digital pin can be left as a pullup, but for electromagnetic radiation test purposes we leave the digital pin LOW when not in use. The digital pin is connected to the bed and GND to the nozzle. The same is done for the TAZ 6.

serial_count = 0;
return;
}
#endif
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jbrazio @thinkyhead please take a look at this snippet. In case the probing fails all attempts G26 is used to give control back to the host, all other commands are ignored so that the print doesn't process. This is managed in the parser and is intended to handle hosts that aren't aware of this feature.

We prefer to reserve G26 if possible.

Granted if the intent is to be able to do manual cleaning of the nozzle and continue then the host needs to prevent all G-Code from being sent other than G26, Marlin cannot hold each command it receives in its buffer until it sees a G26, not enough memory; and it shouldn't throw away all other commands if they're not G26 as done here. This only done to handle "unaware" hosts.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jbrazio @thinkyhead would it possible for us to reserve G26?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hum you may I've checked and G26 is not defined anywhere. But I assume when we need user intervention we would call stop() right ? To get out of stop() you may use M999 S1.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jbrazio currently we don't call stop(). The buffer can still get filled when IsStopped() == true. To prevent any host from sending commands into the buffer after probing has failed we throw away all commands other than G26. That's why we handle G26 in the parser. M999 S1 could be used for this purpose as well but since we allocated G26 before this PR and are afraid of having future conflicts with it.

Copy link
Member

@thinkyhead thinkyhead May 20, 2016

Choose a reason for hiding this comment

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

@scalez We've had some discussions recently about the handling of commands ahead of the normal command buffer. I don't know how conclusive they were. But understand that if a host fills up the buffer after issuing G29 (most likely while printing a GCode file) then the G26 command may not be seen until the G29 command exits and the machine resumes accepting commands.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@thinkyhead Currently we're flushing the command buffer when we enter the PROBE_FAIL_PANIC state. So even if the buffer is filled it is cleared and there is plenty of room for G26 to enter the buffer.

Copy link
Member

Choose a reason for hiding this comment

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

I see! So, I'm also wondering why we need a new command or a "special" kind of panic, since we have M999 (and recently added M999 S1 to preserve the rx_buffer). Also, if, as you say, the buffer is flushed then there should be no need to do special handling of G26 outside of the normal command buffer. (We want to minimize calls to strcmp in the gcode parser.)

card.closefile();
card.sdprinting = false;
#endif
clear_buffer();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@thinkyhead command buffer is flushed here.

@scalez
Copy link
Contributor Author

scalez commented May 27, 2016

@AnHardt @jbrazio @thinkyhead: After doing some testing it turns our M999 S1 will break backwards compatibility with our machines. For machines which do not recognize the S1 parameter (prior to #3789) using M999 S1 in our start G-Code causes the rx_buffer to be flushed and Cura LE immediately stops/prevents the print. Therefore, we're reconsidering G26 to maintain backwards compatibility.

Do you guys have any suggestions?

@jbrazio
Copy link
Contributor

jbrazio commented May 27, 2016

@scalez All the functionality will be implemented using M999 S1 and G26 is "an alias" to it ?

@scalez
Copy link
Contributor Author

scalez commented May 27, 2016

@jbrazio that would an excellent way to do it imo.

@jbrazio
Copy link
Contributor

jbrazio commented May 27, 2016

OK this means that we can then decide if we set a flag to control either G26 is active or not.

@scalez
Copy link
Contributor Author

scalez commented May 27, 2016

FWIW G26 is currently used for this purpose on the LulzBot TAZ 6.

Conflicts:
	Marlin/Configuration.h
	Marlin/Marlin_main.cpp
	Marlin/example_configurations/Felix/Configuration.h
	Marlin/example_configurations/Felix/DUAL/Configuration.h
	Marlin/example_configurations/Hephestos/Configuration.h
	Marlin/example_configurations/Hephestos_2/Configuration.h
	Marlin/example_configurations/K8200/Configuration.h
	Marlin/example_configurations/RepRapWorld/Megatronics/Configuration.h
	Marlin/example_configurations/RigidBot/Configuration.h
	Marlin/example_configurations/SCARA/Configuration.h
	Marlin/example_configurations/TAZ4/Configuration.h
	Marlin/example_configurations/WITBOX/Configuration.h
	Marlin/example_configurations/adafruit/ST7565/Configuration.h
	Marlin/example_configurations/delta/biv2.5/Configuration.h
	Marlin/example_configurations/delta/generic/Configuration.h
	Marlin/example_configurations/delta/kossel_mini/Configuration.h
	Marlin/example_configurations/delta/kossel_pro/Configuration.h
	Marlin/example_configurations/delta/kossel_xl/Configuration.h
	Marlin/example_configurations/makibox/Configuration.h
	Marlin/example_configurations/tvrrug/Round2/Configuration.h
@jbrazio
Copy link
Contributor

jbrazio commented Jul 14, 2016

@scalez now that #4054 has been merged into RCBugfix, when do you plan to update this PR and implement the REPROBE feature using the standard G12 for nozzle cleanup ?

@jbrazio jbrazio removed Needs: Testing Testing is needed for this change PR: New Feature Referred: MarlinDev labels Jul 15, 2016
@alephobjects-sync-bot alephobjects-sync-bot deleted the ReprobeRewipe branch July 15, 2016 15:53
@scalez
Copy link
Contributor Author

scalez commented Jul 18, 2016

@jbrazio I will be creating a new PR soon.

@scalez scalez closed this Jul 18, 2016
@scalez
Copy link
Contributor Author

scalez commented Jul 18, 2016

@jbrazio @thinkyhead @AnHardt one thing that the new PR will depend on is the ability to place a command at the beginning of the command_queue buffer. Since the command_queue buffer could be full this puts us back to a situation similar to #4226.

We may be able to make a new function similar _enqueuecommand which shifts all commands back by one and places G12 and G29 at the front of the queue or we could make G12 and G29 additional high-priority commands.

Here's what needs to occur when a probe fail event is triggered:

  1. Stop probing
  2. Add G12 to the front of the queue
  3. Add G29 to the second position of the queue (will likely be added in the order G29 then G12 )
  4. Let G12 and G29 be performed
  5. If G29 fails probing repeat this sequence NUM_ATTEMPTS times (go back to 1.)
  6. If all attempts fail then give up

@AnHardt's emergency_parser should only be used for parsing commands that are piped from rx_buffer -> command_queue so it is likely out of the question. Therefore we need to come up with a function that places G12 and G29 at the front of the queue even if it is full.

What do you guys think is the best way to solve this?

@Blue-Marlin
Copy link
Contributor

Call Nozzle::clean(pattern, strokes, objects); directly.
Split gcode__G29 in a parameter evaluating part and a executing one. Then call the executing one directly.
Avoid queuing ta all.

@jbrazio
Copy link
Contributor

jbrazio commented Jul 18, 2016

We may be able to make a new function similar _enqueuecommand which shifts all commands back by one and places G12 and G29 at the front of the queue or we could make G12 and G29 additional high-priority commands.

That would be one way to do it.. other one would be to call Nozzle::clean() directly..

-edit-
@Blue-Marlin beat me by seconds lol

@scalez
Copy link
Contributor Author

scalez commented Jul 18, 2016

@Blue-Marlin @jbrazio from the discussion earlier it seemed we wanted it to enter the queue, but if everyone is okay with calling the function directly then I'm cool with that.

@jbrazio
Copy link
Contributor

jbrazio commented Jul 18, 2016

That would be my fault, I spoke about "G12" but I have no issues with direct function calling.

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