machinekit / QtQuickVcp

A Virtual Control Panel for Machinekit written in Qt/C++/QML
Other
128 stars 74 forks source link

M00/M01 Resume Behavior #162

Closed dkhughes closed 7 years ago

dkhughes commented 7 years ago

When a program encounters an M00 or enabled M01, the GUI controls do not properly reflect the state of the control to allow the operator to continue a program. The Pause/Resume action is enabled, but the user must first Pause then Resume even though machinekit only requires the resume command to be sent to continue running the program.

Here is a sample program to duplicate the behavior:

G17 G20 G90 G40 G54

#<count>=0.0
o100 do
(Simulate a M00 condition)
(like a manual tool change op.)
G01 X5.0 Y5.0 F150.
M00 (Wait on operator)
G01 X0. Y0.0
#<count> = [#<count> + 1.];
o100 while[#<count> LT 10.]

G40 G1 X0.0 F100.

M02

What's expected: The M00 should enable either the resume button or enable the RunProgramAction again.

What happens with the RunProgramAction?: The running flag is still active since the program is technically running. This check https://github.com/qtquickvcp/QtQuickVcp/blob/master/src/applicationcontrols/RunProgramAction.qml#L51 keeps the run button disabled.

What happens with the PauseResumeAction?: The action continues to offer Pause as the only option since the GUI is unaware that the M00 caused the control to pause already. This check https://github.com/qtquickvcp/QtQuickVcp/blob/master/src/applicationcontrols/PauseResumeProgramAction.qml#L40 makes activating the PauseResumeAction twice required before the control continues execution.

The proposed fix: I made the PauseResumeAction also dependent on the status.task.taskPaused field. This allowed for correctly representing the pause state whether the user initiated it or the ngc program did.

If this sounds acceptable I can submit a PR.

machinekoder commented 7 years ago

Yes, the fix sounds good. Please go ahead and create a PR.

machinekoder commented 7 years ago

@dkhughes Any progress on this?

dkhughes commented 7 years ago

@machinekoder Sorry for the delay. I've been a little side tracked. Okay, so checking this out again, the checked property on the button would need to be forced. I think it would be better to tie the display state to the status.task.taskPaused value, and then make logical changes based on that instead of the local button checked state.

See the proposed changes here: https://github.com/dkhughes/QtQuickVcp/commit/8093416707f4a65173667f1950cf8ca45c02740d

Edit fixed bad link

machinekoder commented 7 years ago

Good point. We may also bind the checked state to _paused: Either via:

checked: _paused
or 
Binding { target: root; property: "checked"; value: root._paused }
machinekoder commented 7 years ago

Feel free to create a PR. Makes things easier to discuss.