MaslowCNC / Firmware

Maslow Firmware
GNU General Public License v3.0
263 stars 133 forks source link

Serial buffer overflow does not presently halt processing #535

Closed blurfl closed 5 years ago

blurfl commented 5 years ago

Testing against WebControl for serial buffer operation, I've noticed that if a buffer overflow occurs, the firmware only sets the sys.stop flag, which causes the main loop to shut off spindle power, run kinetics.init() and clear the flag. The effect is that WC keeps sending gcode, though important lines have been missed due to the overflow. In some cases, the effect of the missing lines sends the cutter off in an unexpected direction.

I'm thinking that the firmware should respond to the buffer overflow by cancelling the cut (as does the 'Stop' button) and posting some status message ("Line overflow"), feedback message or maybe an alert ("ALARM: the serial buffer has lost data.").

Is there a reason to keep on cutting instead of bailing out?

davidelang commented 5 years ago

I don't think so. If you know you have had an overflow, stopping with an alarm is the safest thing to do. you don't know what you missed, so you don't know what it's safe to do.

you can't even try to retract the Z axis, because you don't really know what's safe (if you could figure out a way to move it to the height it was at the start of the program or just after the last tool change, that would help prevent the bit from rubbing after you stop)

David Lang

BarbourSmith commented 5 years ago

I agree, I can't think of any reason to keep sending lines. I believe that the firmware already even reports a buffer overflow so by responding to that we could stop the cut there.

blurfl commented 5 years ago

grbl sends a line starting with 'error:', GC::serialPortThread specifically ignores lines starting 'error:', but that seems like a good place to handle the situation in GC. The firmware would identify the overflow, clear its buffer, send the 'error:...' message and enter the 'pause' state. GC would identify the 'error:...' line, clear its buffer, and wait for the pause to be ended.

BarbourSmith commented 5 years ago

Yeah, I like that plan. I believe we ignore the messages starting with "error" but display them in a pop-up? Or do we ignore them completely and do nothing?

blurfl commented 5 years ago

Presently the 'error:' messages are treated like the 'ok' line. GC could display the 'error:' in the same way as 'message:' but halt sending as well.

BarbourSmith commented 5 years ago

I think we should check what comes after the 'error' part before halting because I believe that the GRBL standard sends error messages somewhat often...but I am not absolutely sure

blurfl commented 5 years ago

Now that WC correctly handles buffering, I wonder whether this change is still needed. GC, and now WC, do a pretty good job of avoiding the serial buffer overflow.

I have PRs for firmware to send the error and clear its buffer and for GC to respond by executing the 'Stop' command.

In order to test the PRs, one needs to change the GC::serialPortThread::bufferSize from 126 to a larger value (200 for instance) and set GC::MaslowSettings::bufferOn to 'true' (blue 'ON' showing) to get GC to flood the firmware buffer. Without these changes, the buffer overflow just doesn't arise. GC and WC are pretty good at their job!😁👍

Should I close this issue as unnecessary?

BarbourSmith commented 5 years ago

Very nice! Yes, I like that plan. I thing that this is safe to close as long as it is no longer an active problem.

davidelang commented 5 years ago

I think that overly long lines can cause it as well, I think having handling of the buffer overflow in a fail-safe condition is a good idea.

David Lang

blurfl commented 5 years ago

overly long lines The present setup handles lines up to 126 characters (grbl is limited to 80, I think...) and in testing I couldn't come up with a valid gcode line approaching that length without comments. Examples welcome...

davidelang commented 5 years ago

On Mon, 21 Oct 2019, Scott Smith wrote:

overly long lines The present setup handles lines up to 126 characters (grbl is limited to 80, I think...) and in testing I couldn't come up with a valid gcode line approaching that length without comments. Examples welcome...

I am thinking in terms of future proofing things, so that we can accept input from things like universal gcode sender.

I don't trust the senders to strip out all the comments :-)

David Lang