terjeio / ioSender

A GCode Sender for Grbl and grblHAL written in C# (Windows only).
BSD 3-Clause "New" or "Revised" License
207 stars 65 forks source link

Jogging keyboard handler appears to send incorrect stop command to grblHAL #320

Closed dancollins closed 10 months ago

dancollins commented 11 months ago

I'll start this off with my thought of "this must be working for someone else, and I'm missing something". If this really is a bug, then that would explain the behaviour.

Expectation

Pressing an arrow key on my keyboard (aka cursor keys left/right/up/down) should start moving an axis until that key is released.

Reality

The machine continues jogging (with JOG in the status bar) - and I'm too scared to find out if it will stop. I hit the RESET button, and then re-home.

Hardware/software

Analysis

  1. Key press in ioSender is received here: https://github.com/terjeio/ioSender/blob/master/CNC%20Core/CNC%20Core/KeypressHandler.cs#L172
  2. ioSender will send GrblConstants.CMD_JOG_CANCEL when the button is released as part of JogCancel() here: https://github.com/terjeio/ioSender/blob/master/CNC%20Core/CNC%20Core/KeypressHandler.cs#L450
  3. grblHAL will receive that character, and process it in protocol_main_loop() here: https://github.com/grblHAL/core/blob/master/protocol.c#L211
  4. grblHAL checks for the ASCII_CAN symbol specifically for ending jog mode
  5. GrblConstants.CMD_JOG_CANCEL is defined as 0x85, while ASCII_CAN is defined as 0x18
  6. This mismatch means grblHAL's jogging is never stopped, and explains the observed behavior

Suggestion

Patch ioSender to send GrblConstants.CMD_RESET when jogging is canceled.

terjeio commented 10 months ago

I retested the LPC1769 driver when this issue was raised and I am pretty sure I posted a comment... Anyway, I'll try again and sorry for the late reply.

Jog cancel works with ioSender when I test again now, here is how the controller handles it:

When 0x85 is processed by the single character command parser cancel_read_buffer() is called in the stream handler. This flushes the read (input) buffer and inserts a 0x18 (ASCII_CAN and also grbl reset - CMD_RESET, see below) character that is subsequently read by the main loop. This flushes the line input buffer (by setting the character counter to 0) and initiates motion cancel if a jog is ongoing. The reason for flushing the buffers is to get rid of any pending job commands that may linger in the buffers.

Note that any 0x18 - CMD_RESET receveid by the stream handler will be flagged for stripping (drop) by the single character command parser and thus never enter the read (input) buffer. This is the reason I decided to use 0x18 (as ASCII_CAN ) to signal to the man loop that the line buffer should be flushed and any jog active jog motion to be terminated.

Patch ioSender to send GrblConstants.CMD_RESET when jogging is canceled.

CMD_RESET cannot be used to cancel a jog as this will result in losing track of the current position if it happens during a jog move. This since there is no feedback from the motors that continue to move a little due to inertia after the step pulses are abruply stopped by the reset.

dancollins commented 10 months ago

Thanks for the response. I'm glad I asked rather than mindlessly patching - I hadn't noticed the way the CMD_RESET was handled differently,

I was learning how to use my touch plate yesterday, and invariably needed to jog once again. For whatever reason, it was jogging once per key stroke (press and then release). Holding didn't jog further, and the amount jogged was what was set in the jog panel. Have I just forgotten how to jog continuously since I last tried, or is this as simple as me setting the jog distance to 100 mm?

Apologies for this dragging out - I'd really like continuous jogging where I hold a button and release when it's roughly where I want it.

terjeio commented 10 months ago

See Setup and configuration.

dancollins commented 10 months ago

I'm going to mark this as resolved, based on your testing that it works. I hadn't ever set the jog mode argument so I suspect I had just set the step size to 100 mm and perhaps hit the key more than once.

Thanks for the detailed analysis and pointing me where I should have looked first!