G38.2+ - #491
Conversation
|
@tmpvar : Thanks for the explanation. I just wanted some clarification why we needed to add more probing tools. The code looks good and small, so there shouldn't be any issue with eating up too much flash storage. There might be a couple of things I may add or change after integrating and reviewing the code later though, like a separate message (or no message for G38.3/.5). A few things:
I'm not totally against adding these. I generally don't want to make things more confusing for new users getting started with CNCing. These probing g-codes are very similar between all of them and are not immediately clear what the differences are and their use cases. |
There was a problem hiding this comment.
Shouldn't the sys changes be done down around line 985, after the rest of the error checking on the line is complete?
There was a problem hiding this comment.
The correct state will be setup prior to running probes in the current location, but I defer to @chamnit w.r.t. where the state change lives in the file
|
I am in no way the authority on how this should work! The only experience I have is with my handful of hobbyist grade homebuilt cncs.
I agree, and that is fine. My main concern is that tool lengths vary which affects the speed at which the probing cycle can occur. If we can trigger the switch while traveling at a reasonable rate, then we have a basis for performing the
That is a good question. My naive response is that the behavior should be the same
From my experiences, even after the pull-back the switch is still depressed and the 1/8" bar that's holding it in place is still deflected. The amount of deflection is more at higher probe rates. I my case (and I'd venture to guess others) if you back off to exactly the switch trigger point, the switch should still be triggered (even if the switch has the exact same on and off location). I do see your point though, and am curious why the bar is still deflected after the pull-back stage.
Yeah, this is what I'm banking on. Use |
|
@tmpvar : Ok. Now I see what you're getting to. You want to use G38.2/.3 as a seek and G38.4/.5 as a locate cycle. Makes sense. I think though, I'll need to remove the pull-back motion for this to work correctly. Perhaps we can do something along the lines of a compile-time option for the rest of the probing cycles, so that we can keep things simple for new users. Would that be an acceptable solution? @ashelly : Yep. I think I'll re-write portions of this code to fix little errors like these. I don't think it'll be right to ask @tmpvar to fix the nuances of the Grbl code. |
|
@chamnit a compile time option is fine by me. Also, if you are referring to the pull-back motion on I'm also willing to fix up this patch-set so that it adheres to the grbl style, which so far only seems like the issue @ashelly mentioned. Are there any other nits? |
|
@tmpvar : Hard to say exactly without studying the code, but two things pop out. One, I'd try to find a way to remove the sys.probe_away variable and move it to the probe cycle function. There's got to be a way to do it without adding a permanent variable. Second, I'd probably re-instate the probe_get_state() function. I've been trying to keep any pin specific calls as function calls that can be easily changed or updated for portability reasons. |
|
@tmpvar : I wasn't sure if you were still working on this pull request. Should I wait? |
|
@chamnit yeah, I'm taking another stab at it after thinking on it for a bit. I have removed the To fix that, I think adding a |
There was a problem hiding this comment.
since probe_get_state is called from the ISR, we have to put the mode somewhere so we can pass it along. This seems like as good of a place as any, since we're only using 1 bit of 8.
|
Just realized that this still needs the "no error" mode installed on G38.3 and G38.5 |
|
@tmpvar : Yeah. I started working on it last weekend, before I remembered to ask you first. So we weren't doing the same task. (I then got sidetracked by Windows updating hell trying to restore an old PC laptop.) I don't think the "no error" will be a problem. Everything up to to the probe call should be the same. The exit handling will only need to avoid setting the alarm and update the stored probe position to the failed location. There also needs to be an additional boolean parameter, according to linuxcnc, that tracks if a probe is successful or not. And, this would likely need to be printed with |
|
My vote: Adding it to PRB response makes it available in the correct context. |
|
@gerritv : Agreed. I didn't like the prospect of adding the boolean probe parameter either to the |
|
@chamnit, that's exactly what I did for my automation interface. I either get "[PRB: ]" when it's good, or "[PRB:Not Found]" on bad. It works well for my use case. |
|
According to what I have read, an unsuccessful probe returns 0 for the status and location of the point (I suppose the location at point of failure). G38.3 and G38.5 only add an Error indication as well, otherwise they perform the same function as G38.2 and G38.4 respectively. (Not that I have used any of the probe commands, yet. Due for addition to my GUI soon though) Since a fail might also signal error, then we should send an error: sub code. to allow a more versatile and robust error handler. |
|
Current behavior for G38.3/G38.5 moves back to the origin when the switch is not triggered. this should be fixed in the next commit by hopping over the pull-off section |
and while "no errors" is enabled (G38.3, G38.5)
this allows the PRB report to be valid when in "no errors" mode and the probe fails
|
Functional, minus the flag on the PRB report. I think adding a bool to track success is a good idea, that way the format is generally the same which makes for less surprise when writing a parser. I do think that the status should be returned immediately, but incase you cleared your terminal or something you should also be able to see it in the |
|
@gerritv : I debated on using the With G38.2 it was easy, all I needed to do was to enable the alarm and all execution stops per g-code definition. Clean and easy. G38.3/.5 kind of messes this up a bit, because it's a little different. If a probe fails, sending an If a G38.3/.5 probe command is valid (responds `ok') and the cycle fails, the GUI should receive separate feedback and keep streaming g-code. I suppose a problem would be if there are several probe commands sent and the GUI would then need to determine which one failed..... Dang. I hate it when there isn't a clean obvious solution. |
|
If you always send the probe fail/success status as part of the [PRB: response then 1 part is solved. For G38.2 and G38.4, you could additionally set Alarm? .3 and .5 should not send an error at all, just the Probe Status bit set to 0. The protocol issue with the send/response is a general one, adding message numbers to the Gcode packet and to the ok/error (Ack, NAK) would allow keeping it all in line. But that requires a rather different implementation of Grbl and all the GUI's. Maybe when the Arduino Zero is released into the wild we can think of adding that additional 'real' protocol. (I used to design and implement protocols in the sync comms days, SDLC, X.25, Uniscope and some proprietary ones, the problem we are having is a common one) |
|
@gerritv you need to unlock / home to get out of the alarm mode |
|
I know I need to do that, which is why I think it is a fault in implementation. It is an Error, not a catastrophic failure. It makes G38.2 useless or at least very risky to use: if it fails. I lose all my positions, feed rates etc just because the probe fails? I'm not totally certain this is what @chamnit intended. When the probe fails, the machine still knows it is at the same X and Y location, it even knows where the controlled point is on the Z axis. No need to force a Homing cycle etc.? |
|
@gerritv : The intent was to place a failed G38.2 probe was to put into an alarm state. This is grbl's error mechanism to stop everything. The gcode standard doesn't explicitly layout the exact details of what should happen, but it does say that program execution must stop, which is in effect an alarm. From my exposure to professional CNCs, an alarm is used for just baout everything from limits to simple g-code errors. This forces users to ensure that their g-code programs are good. However, machine position shouldn't be lost. The probe should have come to a stop before issuing the alarm. In these cases, like soft limits, machine position is retained. But you do bring up a good point, if you have G92 offsets and other g-code modes set, you do lose these upon a G38.2 probe fail. It does make this less useful, but can be useful in other scenarios. This is also another reason why @tmpvar is pushing installing G38.3/.5 as well, since these don't force program execution to stop. We just need to figure out the error mechanism for these two g-codes that doesn't stop the program. |
|
I understand the need to stop, having crashed a few times as well as broken cutters :-) IMO the really good news with implementing .3 and .5 is that no error or Alarm is required, just a status bit set to 0 instead of 1. For .2 and .4 the Alarm can then stay. This allows a probing program to continue even if it probed in thin air and is exactly what I would need it to do if I were mapping a model car body for replication. |
|
@chamnit re: the modes being lost, if you have the preset loaded in Gbl it will apply those after the Unlock? And a Gcode program should have its required settings at the top anyway. So that doesn't worry me either now. Thank you for clarifying the intent. |
|
@gerritv : Hmm. I just re-read the NIST and linuxcnc definitions. I must have hallucinated the program execution stop, because I don't see it. It just says "error is signaled", whatever that means. I suppose I interpreted it as a program stop. Has anyone use Mach3 or linuxcnc's probing feature? What's it behavoir? If we need to change the "error is signaled behavior, we may need to think about a new feedback mechanism to indicate these types of messages. Not sure how though at this point. |
|
I'll read through the Mach3 stuff later tonight. For now I wouldn't lose sleep over the interpretation, the flow should stop on some events and maybe this is one. |
|
http://linuxcnc.org/docs/html/gcode/gcode.html#sec:G38-probe Ah. I'm not crazy. It does say halt program execution in the most updated linuxcnc website. So, alarm stays. Conversely, we don't have to report anything for 38.3/.5. Just the position and record the fail parameter. |
|
Ahhh, I should stop reading their old wiki, very confusing. |
|
@tmpvar : I think so. We should include the probe_status boolean for completeness and add it to the probe message. It should look like I'm open to other suggestions, but this seems like a good compromise. |
|
@chamnit , a very parseable status string. |
|
@chamnit sounds good to me, where is a good place to store the status long term (for |
|
@tmpvar : I would just add it to the report print probe position function in report.c. It's called by both the probing cycle and |
|
@chamnit right, but that would require the addition of another property on |
|
You're right, but I don't see a way around it for now. I think there will On Mon, Sep 22, 2014 at 9:07 PM, Elijah Insua notifications@github.com
|
|
@chamnit ready for review when you get a minute, I'm sure there are further nits which I'm happy to fix! :) |
|
@tmpvar : Awesome! Looks great. I'll merge it later today! |
|
@tmpvar : I reviewed the code in depth over the weekend. There are some issues that need to be resolved. The g-code modal report $G is broken with probing (and was missing G38.2 anyhow) with this update. I'll need to change around some of the g-code parser things. I also will be altering some of the probe state handling. There doesn't seem to be a way to get around the fact that you need system variables to indicate state and probing mode. Anyhow, thanks for doing the leg work on this. It's a lot easier to see what's needed and some potential bugs and pitfalls when there is a set of working code there to read. |
|
@tmpvar : Ok. Had some time to make the changes and pushed them. I did a once over of the code and a little bit of testing on it. If you have some time, please review the code and try to break it. Thanks! |
|
@chamnit will do, been busy trying to kill some sporadic EMI in my setup which is causing homing to go a bit crazy. I've been meaning to build a rig to test out this code anyhow - will likely have time tomorrow to do so |
This is mostly a copy of #490's description which I've included at the bottom
It seems like @chamnit is having some reservations about landing this, so I guess it's worth explaining my use case.
I would really like to have a switch on my machine somewhere near
0,0,-zthat I can use in combination withG43(G43 Tool Length Offset) to automatically setup tools (depth for now) after manual tool changes.My main concern is with
G38.2and that once the switch is pressed it continues moving into the switch during theEXEC_FEED_HOLD/deceleration phase and eventually reports it's resting position. This makes sense, you certainly don't want to blow away the state of the machine doing a hard stop and losing steps. This is probably not as big a concern for machines with fast acceleration settings.However, to be robust against various machine configurations, backing off of the switch seems like a reasonable next operation to ensure the tool is exactly at the height of the button. This is roughly analogous to how homing is performed currently (i.e. back off, re-engage, etc.)
Given access to either the probe pin state or
G38.{4,5}a cycle much like the homing cycle could be created in user land tools (i.e. python/node/java) to help eliminate any variation between tool changes. I've chosen the standard way here as to avoid adding more reporting noise to the output, and with the hope that it is useful for people using other types of probes as wellcopied from #490
This is accomplished by adding a sys.probe_away flag that is used to flip the direction of the switch depending on which direction we are probing in.
truth table
This assumes that the switch is engaged when "probing away" begins, which appears to be sound given the wording in linuxcnc's description
grbl with this patch builds at 27758 bytes vs master at 27654 bytes