bdring / FluidNC

The next generation of motion control firmware
Other
1.62k stars 386 forks source link

Report Run while delaying during a job; prevents UI glitches #1245

Closed MitchBradley closed 4 months ago

MitchBradley commented 5 months ago

The code also sets the stage for a possible per-channel setting to control whether state names are Grbl-compatible, vs reporting the new states with new names. For example, instead of Delay state being reported as Run, with the proposed new setting it would say Delay. Similarly for ConfigAlarm and Critical. This would let UIs describe the situation more precisely.

breiler commented 5 months ago

If I understood the code correctly, when for instance issuing a G4 P{n} this change will make it return the status Delay? I think that this is a good idea to have a state signalling that the controller is currently busy doing something instead of Idle that to me signals that it is ready to accept commands.

MitchBradley commented 5 months ago
bdring commented 4 months ago

I am OK with your definition of the delay reporting.

With that said, I am not sure we need to report differently for streaming. I doubt senders care. Are there other considerations?

If I send G4 P5 at the console I would be happy to see run status until it finishes.

MitchBradley commented 4 months ago

I suppose we could try it and see if anyone has probles

MitchBradley commented 4 months ago

What should the state be after M0 ?

... answering my own question, I suppose that the answer is Hold.

bdring commented 4 months ago

Probably hold

LNC has this note about MDI mode. It probably does not apply to us because MDI is the same as streaming for us. I

https://linuxcnc.org/docs/stable/html/gcode/m-code.html#mcode:m0-m1

MitchBradley commented 4 months ago

It turns out that making it work during streaming is rather hard. When a cycle stop happens inside a job, you can assume that the desired next state is Delay (now named Dwell), because next state Idle only occurs after the job has been completely sent and job mode is no longer in effect.

When cycle stop happens during streaming, there are many different scenarios. They typically occur in conjunction with protocol_buffer_synchronize(), where the cycle stop triggers its return. The problem is that protocol_buffer_synchronize() does not know which state to enter when it is done, as that information is implied by the surrounding code that calls P_B_S. I am testing a fix that involves passing a next_state argument to P_B_S. It seems to be working, but there are a lot of cases that have to be tested - probing, limits, parameter setting, and quite a few other calls to P_B_S.

MitchBradley commented 4 months ago

Okay here is a real conundrum. Let us postulate that it is not desirable to go through transient Idle states when a synchronize occurs. Suppose that a sender is sending from a file that contains "G0 ...", "G57", "G0 ...". The ideal behavior would be that the first G0 caused Run state, the G57 synchronizes (causes motion to complete), then the second G0 executes while state remains Run. But that is really hard to do because FluidNC does not know that the second G0 will arrive, so the synchronize cannot know that it should stay in Run instead of going to Idle.

MitchBradley commented 4 months ago

I have tried a number of different approaches to making it work during sending, without success. Everything I have tried runs afoul of the "appropriate next state is unknowable after a delay when sending" problem. I am now trying a different tack - change the Grbl status message parser in WebUI and GrblParserC so that, if there is an "SD" field indicating that a file is running, replace "Idle" state with "Run" state. This may require some reporting changes during macros.

MitchBradley commented 4 months ago

The UI-centric fix works rather well so I am abandoning this approach. A PR to resolve a couple of problems with macro status reports will follow.