GcodeDriver.waitForConfirmation does not check for errorResponse

[Dimi]

Hi,

when we send a feed command and our feeder responds with an error message, we get a timeout exception instead.

After looking at the code I found that the waitForConfirmation method of GcodeDriver doesn't check for error messages.

To not overcomplicate a fix, could I suggest, that we add a call to bailOnError before throwing a timeout exception?

Thanks :)

[mark maker]

Do you mean an error message matching COMMAND_ERROR_REGEX ?

Then yes, this might be a bug. Will have a look when I next find the time.

_Mark

--
You received this message because you are subscribed to the Google Groups "OpenPnP" group.
To unsubscribe from this group and stop receiving emails from it, send an email to openpnp+u...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/openpnp/f6adf722-9023-4eda-a87a-48e005586c95n%40googlegroups.com.

[Dimi]

Yes, that's what I had in mind.

Thank you :)

[Dimitri Polonski]

Any updates? Anything I can do to help?

You received this message because you are subscribed to a topic in the Google Groups "OpenPnP" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/openpnp/b4kPdywJ05k/unsubscribe.
To unsubscribe from this group and all its topics, send an email to openpnp+u...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/openpnp/0b135100-63ca-4b0e-9939-edecd5bd74dbn%40googlegroups.com.

[mark maker]

Hi Dimi,

Thanks for reminding me, this seems to have slipped my mind.

I now looked at it, and yes we could in deed just add the bailOnError() to the code. But I'm not sure if this really solves your problem? Because it will still wait for the timeout. The difference is merely cosmetic (you see the error in the message box instead of the timeout).

But if you're looking for a way to signal "feeder empty" or something like this from the feeder, that's not going to make you happy. This functionality is currently not implemented in OpenPnP AutoFeeder and (mis)using COMMAND_ERROR_REGEX errors is not valid.

To explain why, I need to elaborate.

I guess there is a certain ambiguity in OpenPnP regarding what an error matching COMMAND_ERROR_REGEX actually means. For motion controllers is not merely a "negative return value", but typically a HALT condition of the machine, for instance when limit switches were hit, when stepper stalling was detected, an E-stop triggered, etc.

The only correct response after such an occurrence, would be to disable/re-enable the machine (the commands from that might contain the needed M999 to reset any HALT conditions).

But this is not formally enforced in OpenPnP, and Job feed&pick retry-loops will (falsely) try to recover from such conditions. So one could be tempted to (mis)use the error condition from a feeder controller as a "negative return value", so to speak. I suspect that is exactly what you actually try to do for your feeder. Right?

But I believe this is not a valid approach. Instead, I think one would need to do the following:

1. Add a new (optional) Feed Status Actuator to the ReferenceAutoFeeder, that is read after the feed actuation. If your feeder does not support asking for the feed status as a separate command, you could make it so that the Feed Actuator does nothing, and the feed is actually triggered by the Feed Status Actuator ACTUATOR_READ_COMMAND. If the response (ACTUATOR_READ_REGEX) shows a negative status (feed unsuccessful), it can throw an Exception in a controlled manner, that triggers the feeder retry loop.
   

And conversely to negate future ambiguity and misuse:

2. Make it so that COMMAND_ERROR_REGEX errors throws a Job-interrupting Exception, that is not caught in any Job Processor retry-loop.
   Note, I introduced such a concept recently (for a different reason):
   https://github.com/openpnp/openpnp/pull/1404
3. Likewise, make it so that any timeout throws an Job-interrupting Exception.
4. Add the bailOnError as you suggested, or (Bonus) break up the waitForConfirmation() timeout poll into a loop with small timeouts and keep calling bailOnError.
   

What do you think?

_Mark

To view this discussion on the web visit https://groups.google.com/d/msgid/openpnp/CALiO%3Daoa9pyY-WuvnFMxOS%3D4CQL2SyACN%3Dnb-b5q4c4o4s9jSw%40mail.gmail.com.