intelligent-agent / redeem

Firmware for Replicape
http://wiki.thing-printer.com/index.php?title=Redeem
GNU General Public License v3.0
36 stars 44 forks source link

G30 probes are affected by the bed leveling matrix #94

Open ThatWileyGuy opened 6 years ago

ThatWileyGuy commented 6 years ago

Is there any reason why we should be unconditionally ignoring the bed leveling matrix during a G30 probe?

eliasbakken commented 6 years ago

Yes, during probing, we do not want the bed level matrix to interfere. I think even during testing of the employed matrix, we only want it active during X, Y movement.

Wackerbarth commented 6 years ago

I disagree totally. I think that we should ALWAYS take "bed leveling", or any other height adjustment that we might make into account AT ALL TIMES. That way probing gives the adjusted height of the actual object. In the case of an "empty" perfect bed, we would get the error, "0.00", anywhere we probe.

Although I recognize that, in a strict sense, the corrections are not simply matrix transformations, they can generally be viewed as if they were. In that view, even without resetting the matrix, you can take a set of samples and, from them, derive an additional compensation transformation which gets concatenated onto the existing collection. And we don't actually accumulate an ever-growing chain of transforms. Using the rules of matrix algebra, we can reorder and merge the corrections into an equivalent canonical form.

And, if you don't want the correction, reset it. "I" is always a valid correction transformation.

eliasbakken commented 6 years ago

You are welcome to prove me wrong, but if we are talking about the same problem, my experience has been that during probing, you would not want to introduce errors from previous, possibly incorrect/invalid measurements. @ThatWileyGuy could you link to the line that has started this discussion? I assume I have been the one implementing this, I did the first working bed probing stuff.

Wackerbarth commented 6 years ago

@eliasbakken -- My argument is that G30 is the primitive probe command. As such, I feel that it MUST operate "in the current context". How else can I determine the amount of residual error in my calibration. I do not argue that, for the purpose of computing a new set of "bed tilt" parameters, as an expedient to simplify the transformations I reference above, you would want to set the current context to have no tilt. However, that belongs in the G29 (or whatever) routine that is utilizing the probe values.

ThatWileyGuy commented 6 years ago

I can definitely see both sides of this and I don't have strong feelings either way (yet).

The problem I encountered is that probe moves on the current PRU firmware are defined as complete when all axes in the move are blocked. For a pure-Z probe, that means the probe stops as soon as an endstop blocks the Z axis as you'd expect. For a Z probe with some steps on the X and Y axes due to the bed leveling matrix, that means blocking the Z axis won't stop the entire movement, so it'll generally report back as if the entire probe distance was covered and the movement never stopped. This behavior is, from a user's perspective, quite broken and unintuitive. It's particularly bad because the X/Y steps are so slow that you don't really see any movement - it looks like you started a G30, it hit the bed, the printer froze for a few seconds, then it moved too far upwards from the bed and returned a clearly incorrect probe result.

troy-jacobson commented 6 years ago

I was thinking of a generalized probing routine (not specifically z only probes.) Though not relevant to 3D printing, but hole center-finding is a common probe activity for CNC machines. In this case, the probe moves in X/Y until it touches the side of a cylinder. Then it finds a couple of other points on the side to calculate the center point. What that means for configuration is that treating the probe input strictly as an endstop may not be appropriate. Instead, it might be better to make a probe input a specific behavior. Then instead of using preset endstop_stops configurations, all motion will stop when the probe is triggered. The point at which the probe is triggered will be saved for future reference. This could implemented either by a special flag in the PRU code, or by deciding on "endstop_stops" values based on the axes involved in the probe move. The first idea sounds better to me, with the idea that there could be separate behavior in the PRU for triggering the probe vs hitting an endstop. I think the cancelable flag has some part in the behavior, and this way the probe move can be cancelable, but hitting an actual endstop would be an error/alert.

Wackerbarth commented 6 years ago

My position is that probing as Troy describes SHOULD be done in workpiece coordinates. Perhaps our current probing algorithm is the problem. Why can't the probe stop when ANY axis triggers? I also worry about any action which runs into a limit and keeps moving after that. It looks like a "mechanical disaster" in the making.

goeland86 commented 5 years ago

I believe the probing routine as it currently stands in the code has proven itself reliable, and if we include PR #169 in the code (which my tests reveal helps), then this discussion may no longer be needed.

Feel free to re-open it at a later date if problems with probing arise.

Wackerbarth commented 5 years ago

@goeland86 - The question is whether, or not, you can run any probing AFTER you set a non-identify leveling matrix. When I last tried this, on a cartesian system, probing did not work because there was motion in the X-Y plane as well as the Probing motion in Z.

goeland86 commented 5 years ago

@Wackerbarth you're the only person who has reported any issues with the probing on any printer geometry in the last 2 years that I'm aware of. And given that until I closed the ticket the discussion seems to have halted over 6 months ago, I don't perceive this as a critical issue at this juncture.

Wackerbarth commented 5 years ago

Well, that is YOUR opinion. Nothing has happened to fix this this issue because it isn't enough of a "show stopper" to keep you from getting SOMETHING to a release state. But, the fact that other issues have held up that release, and thereby stopped other progress, doesn't invalidate the issue.

eliasbakken commented 5 years ago

I remember spending a lot of time on this back in the days, probably a couple of years ago. The logic should be so that all the matrix should be bypassed during probing. But it can be nice to have the option of testing the probed bed after the matrix is in effect, in essence using the matrix to run the probing a second time. If the data is uploaded to "review" for showing it online, it can be nice to first run the probe (I use G29), then running it a second time with G29 S (IIRC) which "uses" the matrix and should then yield a flat bed. If it is not fairly flat, then there is either something wrong with the probing or the testing. @Wackerbarth https://github.com/Wackerbarth Could you try a probe + a test and show the data?

Kind regards, -Elias

Den søn. 27. jan. 2019 kl. 14:23 skrev Richard Wackerbarth < notifications@github.com>:

Well, that is YOUR opinion. Nothing has happened to fix this this issue because it isn't enough of a "show stopper" to keep you from getting SOMETHING to a release state. But, the fact that other issues have held up that release, and thereby stopped other progress, doesn't invalidate the issue.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/intelligent-agent/redeem/issues/94#issuecomment-457917479, or mute the thread https://github.com/notifications/unsubscribe-auth/ADlQ2QFBQcDq_Whwt9LgNfDhQ3elNPSWks5vHahhgaJpZM4TbM5X .

Wackerbarth commented 5 years ago

I'll do that when I get back. But the problem is that the probe doesn't stop when the Z axis hits the bed. Instead, it keeps running for the remaining X-Y distances, digging into the bed and giving absolutely wrong readings.

There are two approaches. I prefer the one which includes the previous correction, shows the residual error, and computes the new basis on an incremental basis. That strategy is a bit more flexible. And to get the "zero" the correction matrix (set leveling to an identity matrix), you can always include that operation as an additional step, or as a flag, in the calibration process.

zaped212 commented 5 years ago

@eliasbakken Just to make sure I am following the reasoning for not including the bed level matrix revolves around calculating the new bed level matrix if G29 is configured to update the bed level matrix. As I presume the current logic only takes the probe measurements / points to calculate the new bed level matrix. So theoretically if you had an unlevel bed, ran G29 with an identity matrix, obtained a non identity matrix, and then re-ran G29 WITH the current bed level matrix then you should get an identity matrix out since the previous matrix "leveled" the bed.

@Wackerbarth I read through the issue chain and it seems like the issue you are running into is due to the bed compensation matrix being taken into affect ( for your tests ) when performing the actauly probe move. Just from thinking about it, the compensation matrix ( if used ), I would think, would only be used to position the bed in the proper X, Y, Z position before the actually probe took affect. Then the probe move ( in this case ) would be a pure Z move. Ultimately the bed level matrix really should not affect the Z movements as it only adjusts height along the X,Y plane.

I think if you adjusted your test to enable the compensation matrix for just the X,Y move, and disabled it for the Z probe move then you would not have the problem you are seeing.

"IF" we are going to add support for probing with the bed compensation matrix we would need to modify atleast the following commands ( from my understanding ), though an audit would be needed. G30 ( add additional param to enable the bed level matrix, would leave default as having the matrix disabled ( identity matrix ) ) G29 ( similar, to G30, and if the bed level matrix is being used then we shouldnt allow any calculations of new bed matrixs ) G33 ( from my understanding should never take the compensation matrix into account? )

While I can see benifit from having this functionality

Wackerbarth commented 5 years ago

No, "bed leveling" (perhaps "bed tilt compensation") does a 3D rotation of the workspace to cause the workpiece to be built perpendicular to the tilted bed. The probing that you describe would be a skewed measurement. Admittedly, we rarely have so much tilt as to cause the cosine of that tilt to be significantly different from 1 at the scale of the probing throw distance.

And I think that your proposal with respect to adding a parameter to change the behavior just complicates things. I advocate that new compensation matrix be calculated by adjusting the matrix that currently is in use. That way, the user can, at any time, just run G29 and have the resulting matrix that best corrects for the tilt.

I also suggest that the G29 command also correct the z-height since it has the appropriate information to do so. The current scheme is very difficult to use when the z-height is being measured at any point other than (0,0). Unless you define the center of your bed as the origin (uncommon for cartesian systems), you are not likely to be doing your nozzle height probe at that origin.

zaped212 commented 5 years ago

Ahh @Wackerbarth didnt realize that the matrix had evolved to handle workspace rotation.

Wackerbarth just had a thought. What do you have your probe endstop configuration set to? For my delta I have ( from G32 ) M574 Z2 x_ccw,y_ccw,z_ccw ; Enable Z probe

I would expect since you mention cartesian, that yours is something like: M574 Z2 z_ccw ; Enable Z probe where only the z axis is affected.

I wonder if you need to configure your Z2 to prevent "all movement" when triggered instead of just the z axis ( as would be expected for Cartesian ). Also wonder if we would need to expand that to have "direction of travel" masks as we might not know what the compensation matrix is causing us to do.

My thought on the direction of travel mask ( z_probe_dir ?? ) is that we could use it to mask off the direction that we were actively traveling ( on that axis ) when the endstop was hit, to allow us to reverse out of it and

Wackerbarth commented 5 years ago

Yes, my probe stop is z_neg. But, this really breaks down if someone has a CORE-XZ machine (or, alternately, if you look at "homing" as a form of probing. You end up with an axis for which you have no way to know how to "finish". I argue that the appropriate probing action is for the probe to always stop ALL motion once the condition is met. For a delta, you effectively have that because motion in the Z direction involves motion of all actuators (and in the same normalized direction). But, for other configurations that is not true.

zaped212 commented 5 years ago

Yeah I was coming to the same thought. I am actually wondering if we should actually specify what endstop gets triggered durring a probe, and instead of specifying what directions it should mask ( pos, neg, cw, ccw ), the probe tigger should stop all current motion reguardless of direction and mask off that direction automatically until released.

It did seem a little odd that I needed to specify the direction of rotation to mask off for my probe routine as that information should be known by the printer when its performing the probe. aka: I moved these axis these directions and the probe triggered.

Wackerbarth commented 5 years ago

As I see it, probing is not an action that gains by significant look-ahead planning. I would utilize the planner to position the probe with an empty queue and then issue a probing movement which would also end with an empty queue (providing a measure of the portion of that motion that occurred before the trigger and suppressing ALL of the remainder of the motion). Then the portion of the program that initiated the probe motion could adjust values in the printer state accordingly before issuing the next set of instructions.

Wackerbarth commented 5 years ago

@eliasbakken - Per your request: key=6EQAQH2928ICQR9RRT0U These are run using the G29 code in the Anet-A8 config with the initial M561 removed.

The bed has a significant tilt (for the test) The command sequence was M561; G29; G29. As you can see, the first run yields a reasonable correction matrix. However, the subsequent set of probes are meaningless.