Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Printing from SD can't be stopped by menu #4042

Closed
QoooQ opened this issue Jun 14, 2016 · 33 comments
Closed

Printing from SD can't be stopped by menu #4042

QoooQ opened this issue Jun 14, 2016 · 33 comments
Labels
Bug: Potential ? Needs: More Data We need more data in order to proceed
Milestone
1.1.0

Comments

@QoooQ
Copy link

QoooQ commented Jun 14, 2016 •
edited
Loading

hi guy i use RCBugFix firmware
when i printing........3min later.........tune print stop
the X-axis is go and back cycle ( go ahead Retreat cycle)
what is problem ???

and i change firmware 1.1.0 RC6 is ok!!!
@thinkyhead
Copy link
Member

You will have to do more diagnostics to help us figure out what the problem is.

@thinkyhead thinkyhead added Bug: Potential ? Needs: More Data We need more data in order to proceed labels Jun 15, 2016
@QoooQ
Copy link
Author

QoooQ commented Jun 16, 2016 •
edited by jbrazio
Loading

I update mp4 you can watch it know what problem?! Thanks

@jbrazio
Copy link
Contributor

jbrazio commented Jun 16, 2016

Are you printing from SD or from a host ?

@QoooQ
Copy link
Author

QoooQ commented Jun 16, 2016 •
edited
Loading

SD card!!!
board: mks gen 1.4 lcd 2004

@jbrazio jbrazio changed the title RCBugFix-bug?! Printing from SD can't be stopped by menu Jun 16, 2016
@Grogyan
Copy link
Contributor

Grogyan commented Jun 16, 2016

From the video, it appears that the buffer is somehow executing the last few moves, in an endless loop.

@maukcc
Copy link
Contributor

maukcc commented Jun 16, 2016 •
edited
Loading

This is the same behaviour I talked about when we were doing the keypad mashes thingy.
Don`t do homing as it will do that in a loop forever :)

@jbrazio
Copy link
Contributor

jbrazio commented Jun 16, 2016 •
edited
Loading

@maukcc even with Scott's patch ?

@maukcc
Copy link
Contributor

maukcc commented Jun 16, 2016

I am using RC6 now, and have not checked RCbugfix for a bit. I will test when I have the time.

@MrExo3D
Copy link

MrExo3D commented Jun 16, 2016 •
edited
Loading

Ive run into the exact same issue.. shaking after a stop is issued. im using RCBugfix

@thinkyhead
Copy link
Member

I am using RC6 now, and have not checked RCbugfix for a bit.

As such an active contributor you should definitely be using RCBugFix as much as possible. We especially need feedback on our patches to those issues that you've been helping us discover.

@thinkyhead
Copy link
Member

thinkyhead commented Jun 17, 2016 •
edited
Loading

Ok, so it sounds like the buffer – either the SD buffer or the regular command queue, is going into a state out of sync with itself. I will look at the older "Stop SD Print" code and compare to the current code. My guess is that either the ring buffer is getting corrupted, or the SD buffer is being submitted continually.

We did patch the Stop routine to try and purge commands left in the buffer. Probably the code to purge is errant.

@fbarcenas
Copy link

happen the same...

@jbrazio
Copy link
Contributor

jbrazio commented Jun 23, 2016

I can confirm with latest RCBugfix I have the same loop behavior and have to power cycle.

@thinkyhead
Copy link
Member

thinkyhead commented Jun 24, 2016 •
edited
Loading

@jbrazio I've looked at the code and still have no clue as to what is looping. All I know is, before I attempted to clear the command buffer, the machine would execute any commands that were still left in the buffer, but only once. So far I can't see where the buffer-handling logic is screwy, but obviously something is amiss!

Perhaps you can try putting some beeps and moves alternating in a GCode file, then SD print it, and use "Stop print" to see what happens. If it keeps beeping, then we know the command buffer is in an endless loop. If it keeps moving, but not beeping, then we know it's a planner or stepper buffer problem.

G1 Z10 F8000
G1 X0 Y0
M300 S698 P300
G1 X90
M300 S523 P50
G1 X180
M300 S494 P50
G1 X90
M300 S523 P100
G1 Y90
M300 S554 P300
G1 Y180
M300 S523 P300
G1 Y90
M300 S523 P100
G1 X0 Y0

@fbarcenas
Copy link

The beeper of my board is broken or happen something rare..., i can't test this.

Checking the problem a lot times i see than always when i stop do a diagonal in X/Y, never the extruder move.

Is like the buffer not is empty???¿?

@thinkyhead
Copy link
Member

The beeper of my board is broken

It has never worked, or only with the current code?

when i stop do a diagonal in X/Y,

When you click "Stop print" the printer continues to move diagonally until you turn it off?

Is like the buffer not is empty

We have determined that much, now we're trying to figure out what the cause is. So far no one has added logging code to the command loop to see what's going on. But perhaps one of us will try that soon.

@fbarcenas
Copy link

1º #4127
2º yes

@ImmersedN3d
Copy link

ImmersedN3d commented Jul 3, 2016 •
edited
Loading

I can also confirm that the issue exists on the current RC Bugfix build as of 7/1/16. Stopping a print from SD results in a "Loop" of a short segment of gcode.

My beeper works. ( I like the added beep features ) I'll test the code when my current print finishes.

@thinkyhead
Copy link
Member

thinkyhead commented Jul 3, 2016 •
edited
Loading

I'll be looking at this issue in the next day or so, as soon as I can get over to the workshop to mess with my printer. I'm sure it will turn out to be something very simple and foolish.

@thinkyhead
Copy link
Member

Here's something that I believe will fix the issue in the meantime, while we try to figure out the mistake in clear_command_queue.

 static void lcd_sdcard_stop() {
   card.stopSDPrint();
-  clear_command_queue();
+  // clear_command_queue();
   stepper.quick_stop();
   print_job_timer.stop();
   thermalManager.autotempShutdown();
   cancel_heatup = true;
   lcd_setstatus(MSG_PRINT_ABORTED, true);
   #if DISABLED(DELTA) && DISABLED(SCARA)
     set_current_position_from_planner();
   #endif // !DELTA && !SCARA
 }

@maukcc
Copy link
Contributor

maukcc commented Jul 5, 2016 •
edited by thinkyhead
Loading

What I also found in the lcd_sdcard_stop sequence is that it does the lcd_sdcard_stop. And after that it does 3 (not 2, not 4) commands from the gcode file that it has in buffer from where the stop was initiated.

M190 S60            ;set bed temp and wait
M109 T0 S195        ;set nozzle 1 temp and wait
M117 Cleaning .....
G1 X1 F12000        ;go to wipe point
G92 E0              ;set E
G1 Z1.5 F1200

So if I initiate a stop after the M190, it will do the stop sequence and cut of current M190 (bed heating), and then after the stop sequence, it will do the M109, the M117 and the G1 X1 F12000 (3 commands).

And then it is properly stopped.
This is why we always think the cancel_heatup is not working. It actually is, but it gets re-initiated after the stop process.

@AnHardt
Copy link
Contributor

AnHardt commented Jul 5, 2016 •
edited
Loading

Please try:

static char* current_command, *current_command_args;
- static int cmd_queue_index_r = 0;
- static int cmd_queue_index_w = 0;
- static int commands_in_queue = 0;
+ static volatile int cmd_queue_index_r = 0;
+ static volatile int cmd_queue_index_w = 0;
+ static volatile int commands_in_queue = 0;
static char command_queue[BUFSIZE][MAX_CMD_SIZE];

In case of recursion this may make the difference.

@maukcc
Copy link
Contributor

maukcc commented Jul 5, 2016 •
edited
Loading

@AnHardt
Sorry that gave the same results. 3 commands in "buffer"
I am using RC6 to try this, if that matters. (not bugfix)

@thinkyhead
Copy link
Member

thinkyhead commented Jul 5, 2016 •
edited
Loading

@AnHardt I don't see anyplace in the code where volatile would make a difference. There's no place where cmd_queue_index_r and cmd_queue_index_w are accessed more than once (where a change would be expected). And the compiler should be smart enough not to simply re-use the most recent fetched value of commands_in_queue in cases like these:

  if (commands_in_queue < BUFSIZE) get_available_commands();
  . . .
  if (commands_in_queue) {
    . . .
    commands_in_queue--;
  }
if (commands_in_queue == 0) stop_buffering = false;
. . .
while (commands_in_queue < BUFSIZE && MYSERIAL.available() > 0) {
  . . .
  _enqueuecommand(serial_line_buffer, true);
  . . .
}

@thinkyhead
Copy link
Member

thinkyhead commented Jul 5, 2016 •
edited
Loading

And after that it does 3 … commands from the gcode file that it has in buffer
…the same results. 3 commands in "buffer"

@maukcc Yes, that's the old behavior, and what I had hoped eliminate with the clear_command_queue() function. But we find that instead of actually clearing the command queue, it leaves the queue in a state where it continually loops instead.

Logging lines in just the right places should make it obvious what's going on. Unfortunately I am not near my printer today, so I cannot test this right now.

@thinkyhead
Copy link
Member

thinkyhead commented Jul 5, 2016 •
edited
Loading

Ok, so I posted a branch with debugging added at https://github.com/thinkyhead/Marlin/tree/rc_debug_stop_print. When you use "Stop print" it sets a flag to start displaying debug information before calling clear_command_queue(). If you have a host connected, then you will see the current values of the command-queue variables, used in fetching the next command….

if (canceled_sd_print) {
  DEBUG_VAR(card.sdprinting);
  DEBUG_VAR(commands_in_queue);
  DEBUG_VAR(cmd_queue_index_r);
  DEBUG_VAR(cmd_queue_index_w);
}

@AnHardt
Copy link
Contributor

AnHardt commented Jul 5, 2016 •
edited
Loading

Ok. Got it.
I can reproduce the 'loop' now. Using a 'host' alters the behaviour by sending M105,M27. A simple terminal helps to see what is going on.

SDTEST.G

M190 S60 ;set bed temp and wait
G28
G1 X0 Y0 Z5
G1 X100 Y0
G1 X100 Y100
G1 X0 Y100
G1 X0 Y0

Log

echo:Now fresh file: SDTEST.G
File opened: 1642 Size: 101
File selected
ok P15 B3 R128 "M23 SDTEST.G" 
ok P15 B3 R128 "M24" 
 T:28.8 /0.0 B:56.0 /60.0 B@:0 @:0 W:?
 T:28.9 /0.0 B:56.0 /60.0 B@:127 @:0 W:?
 T:28.7 /0.0 B:55.9 /60.0 B@:127 @:0 W:?
 T:28.7 /0.0 B:55.8 /60.0 B@:127 @:0 W:?
 T:28.7 /0.0 B:55.8 /60.0 B@:127 @:0 W:?
 T:28.8 /0.0 B:56.0 /60.0 B@:127 @:0 W:?
 T:28.7 /0.0 B:56.0 /60.0 B@:127 @:0 W:?
 T:28.7 /0.0 B:56.0 /60.0 B@:127 @:0 W:?
 T:28.7 /0.0 B:56.2 /60.0 B@:127 @:0 W:?
 T:28.8 /0.0 B:56.3 /60.0 B@:127 @:0 W:?
 T:28.8 /0.0 B:56.5 /60.0 B@:127 @:0 W:?
card.sdprinting0
commands_in_queue-1
cmd_queue_index_r0
cmd_queue_index_w3
echo:enqueueing "M84 X Y Z E"
X:100.00 Y:50.00 Z:15.60 E:0.00 Count X: 9412 Y:4706 Z:62400
card.sdprinting0
commands_in_queue-1
cmd_queue_index_r1
cmd_queue_index_w0
card.sdprinting0
commands_in_queue-2
cmd_queue_index_r2
cmd_queue_index_w0
card.sdprinting0
commands_in_queue-3
cmd_queue_index_r3
cmd_queue_index_w0
echo:busy: processing
echo:busy: processing
echo:busy: processing
echo:busy: processing
card.sdprinting0
commands_in_queue-4
cmd_queue_index_r0
cmd_queue_index_w0
echo:busy: processing
echo:busy: processing
echo:busy: processing
echo:busy: processing
echo:busy: processing
echo:busy: processing
echo:busy: processing
echo:busy: processing
X:100.00 Y:50.00 Z:15.60 E:0.00 Count X: 9412 Y:4706 Z:62400
card.sdprinting0
commands_in_queue-5
cmd_queue_index_r1
cmd_queue_index_w0
card.sdprinting0
commands_in_queue-6
cmd_queue_index_r2
cmd_queue_index_w0
card.sdprinting0
commands_in_queue-7
cmd_queue_index_r3
cmd_queue_index_w0
echo:busy: processing
echo:busy: processing
echo:busy: processing
echo:busy: processing
card.sdprinting0
commands_in_queue-8
cmd_queue_index_r0
cmd_queue_index_w0
echo:busy: processing
echo:busy: processing
echo:busy: processing
echo:busy: processing
echo:busy: processing
echo:busy: processing
echo:busy: processing
echo:busy: processing
X:100.00 Y:50.00 Z:15.60 E:0.00 Count X: 9412 Y:4706 Z:62400
card.sdprinting0
commands_in_queue-9

        }
        process_next_command();
      }

    #else

      process_next_command();

    #endif // SDSUPPORT

    commands_in_queue--;
    cmd_queue_index_r = (cmd_queue_index_r + 1) % BUFSIZE;
  }

We are in process_next_command(); executing M190 and there in ideling. In the inner idle we detect the stop and - execute it - set commands_in_queue to 0, leave M190, leave process_next_command(); reduce process_next_command(); by one to -1! Our test if we have a new command in the queue is if (commands_in_queue). Happy looping.

@thinkyhead
Copy link
Member

thinkyhead commented Jul 5, 2016 •
edited
Loading

That's what I figured, but I couldn't trace where inside of loop() it was setting commands_in_queue to zero. Anyway, the fix is simple!

if (commands_in_queue) --commands_in_queue;

@AnHardt
Copy link
Contributor

AnHardt commented Jul 5, 2016 •
edited
Loading 

    if (commands_in_queue) {
      commands_in_queue--;
      cmd_queue_index_r = (cmd_queue_index_r + 1) % BUFSIZE;
    }

!

@thinkyhead thinkyhead mentioned this issue Jul 5, 2016
Merged
@thinkyhead
Copy link
Member

Ok, I think we got it! #4214 is merged. Printing should now stop with no more commands processed!

@maukcc
Copy link
Contributor

maukcc commented Jul 6, 2016

Where does this code go? And what is the change? Then I can give it a test.

@Blue-Marlin
Copy link
Contributor

Blue-Marlin commented Jul 6, 2016 •
edited
Loading

@maukcc
The code is already in RCBugFix.

@jbrazio jbrazio modified the milestone: 1.1.0 Jul 16, 2016
@github-actions
Copy link

github-actions bot commented Apr 6, 2022

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked and limited conversation to collaborators Apr 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Bug: Potential ? Needs: More Data We need more data in order to proceed
Projects
None yet
Milestone
1.1.0
Development

No branches or pull requests
10 participants
@AnHardt @jbrazio @thinkyhead @maukcc @Grogyan @ImmersedN3d @MrExo3D @fbarcenas @Blue-Marlin @QoooQ