Fix #367 - Status report with wrong stat

[MitchBradley]

Fix for #367 : The addition of cm_cycle_start() to the beginning of _execute_gcode_block() in commit ae0145c (which was described as "commentary and naming changes") broke state management. With that addition, nearly every G and M code, except for G0, G1, G2, G3, causes a transition to state 5 (RUN) with no corresponding transition back afterwards. In fact, even G0 .. G3 are broken in some cases - if one of those Gcodes is rejected due to a bad or missing argument (e.g. no axis words), state transitions to RUN and sticks.

There is some rationale for that change from the discussion on the issue:

if the machine was not in a cycle and you send any gcode, it will automatically start one. Since we don’t wait for the user to push a “cycle start” button we automatically start a cycle when we see any gcode

I'm unconvinced by that argument, because, in the context of a g2core-based system where the UI is managed by a different program, "cycle start" results in one of two actions:

• The UI begins sending GCode, e.g. from a file, to g2core
• If the system is in feedhold state, feedhold is canceled and program execution resumes

So "cycle start" from the user standpoint is handled by the UI or "gcode sender", not by g2core. There is no need to switch to RUN state in order to simply execute non-motion GCode. g2core does the right thing without the errant cm_cycle_start()

The removal of this line completely fixed my system which is based on CNCjs and g2core. My system worked well using version 100.x and, prior to this fix, failed horribly with 101.

[mhlong10]

@MitchBradley - Why has this not been merged yet? I'm running into this exact same problem and filed issue #409 which I have subsequently closed (for whatever reason I was unable to find #367 when I originally searched).

I am going to pull this fix and see what happens (it is still unclear to me why cyc_cycle_start() was added in the first place so I'm a bit concerned removing it).

[MitchBradley]

My machine has been working well with the change.

[justinclift]

@mhlong10 How'd you go with this?

I'm inclined to merge this change, if both yourself and @MitchBradley have been having good success with it. It's super simple to revert if it's not good after all. 😁

[mhlong10]

I have done many carves with this fix and it has been working great. I’ve used Vcarve Pro (with arcs) and Easel to generate gcode. I have only had a single issue on my last carve which I’m not sure is related to this fix. I will mention it here and let others make a call on what they think. My call on this is that this fix is good for merging.

I was doing a very detailed carve using the array tool path on VCarve. The first two copies carved without issue but part way thru third the machine just stopped. No alarm, no message, and I was unable to send any commands to g2core. I had to reset the board to get it back. Since the first two copies worked fine the gcode itself is probably not the issue. Also, the queue was being kept full so I doubt it is the fix in this PR. The other possibility is that it is the gcode sender or a comms issue, both of which I haven’t had any issue with.

I am going to try and reproduce next week and debug a bit further.

[justinclift]

Thanks @mhlong10, that's useful info. 😄

Hopefully that hang is sorted out. Doesn't sound like it'd be related to the PRs merged today either. 😖

[MitchBradley]

I wonder if that hang could be related to the recently-fixed race condition #421

[justinclift]

Wondered that too, but it doesn't seem to sound like it. 😄

[karoria]

My machine is also working well with this change.

[justinclift]

Cool. Merged it. 😄

[mhlong10]

I know this is merged but just following up on my earlier comment. Good news. I reran the same failing gcode file several times without fail. So whatever I ran into was a transient issue.

[justinclift]

Awesome! Thanks @mhlong10. 😄