luc-github / Repetier-Firmware-4-Davinci

Repetier-Firmware-0.92 based for DaVinci printer (Beta - so far so good)
GNU General Public License v3.0
194 stars 98 forks source link

Safety: Modify filament detection code to home before issuing change command #84

Closed jnadke closed 9 years ago

jnadke commented 9 years ago

Currently the optical sensor triggers a filament load which displays a message asking to proceed before homing the axes.

When the printing stops, the print head stays heated above the last known position, parked above the printing object. This creates melted or even burn marks and could be a very unsafe condition where a fire could start.

Suggestion: In ui.cpp add: if (!Printer::isXHomed())executeAction(UI_ACTION_HOME_X,true); if (!Printer::isYHomed())executeAction(UI_ACTION_HOME_Y,true);

just before line 4353: pushMenu(&ui_menu_load_unload,true);

for the UI_ACTION_NO_FILAMENT command.

jnadke commented 9 years ago

At least I think this is the correct implementation, I'll test it out at home tomorrow and issue a patch if nobody else has tackled it before then.

luc-github commented 9 years ago

Hi what version of FW do you use ? now change filament moving is on top of drip box, and when printer stop/pause it go to dripbox also

luc-github commented 9 years ago

you are printing from SD or from host ?

jnadke commented 9 years ago

I was using the build from April 30th. From the changelog I don't see anything as being different.

I'm printing from host (OctoPrint).

jnadke commented 9 years ago

I've updated and see the proper code.

Sorry for the issue. I've been tracking the commits and didn't see the issue fixed, but my local branch must've been out of sync.

I do see the proper code:

    if(sd.sdmode )sd.pausePrint(true);
    else //if printing from Host, request a pause
    {
        Com::printFLN(PSTR("RequestPause: No Filament!"));
        Commands::waitUntilEndOfAllMoves();
    }

I'll close this.

luc-github commented 9 years ago

Well that was why I asked you how you print , I do not always test printing with host and looking at the code I have a doubt about behaviour when printing from host and parking position

jnadke commented 9 years ago

I'll test out with updated code and if it still doesn't work we'll see how to get a proper park into place.

It looks like my git was having issues and wasn't pulling all the code properly, I rebased and have the correct ui.cpp code quoted above for the NO_FILAMENT case.

jnadke commented 9 years ago

I'm re-opening the issue, it still does not home with the latest code.

Which makes sense, waitUntilEndOfAllMoves only does what it implies.

luc-github commented 9 years ago

Ok will work on it today, thank for pointing this out

jnadke commented 9 years ago

I'm working on a good fix for it as well.

waitUntilEndOfAllMoves probably isn't the best command to use, since we want to preserve the commands the host has sent. It's probably best to replicate the code the sdcard does, but we'll have to save the printer state since the printer may have stopped mid-command (will it?).

https://github.com/luc-github/Repetier-Firmware-0.92/blob/master/src/ArduinoDUE/Repetier/ui.cpp#L4293

Also, there's a slight issue with the ensuing load in that the home command: https://github.com/luc-github/Repetier-Firmware-0.92/blob/master/src/ArduinoDUE/Repetier/ui.cpp#L4379

In my system both X and Y are homed at the same time. In the rare case that you're at Z=0, from certain positions on the bed the extruder could hit the bed retention clips. But for the most part it's reasonable to expect someone to know they have enough filament for the first few layers.

luc-github commented 9 years ago

for host you have no others way than wait pause is set by host when he received the pause request = no more command is send =waitUntilEndOfAllMoves,( currently there is no immediat pause in repetier) then park the head That said this is for repetier, I have no idea if the host pause command is working on S3D or Octoprint, when printed from host, I assumed host handle the memory position as it the one who knows when it pause.

about : if (!Printer::isXHomed())executeAction(UI_ACTION_HOME_X,true); this should never happen during printing or only if motor are disabled because printer is idle for a long time.

so currently parking is not the difficult part just add 2 lines after : waitUntilEndOfAllMoves the tricky part is : does RequestPause: No Filament! work with all hosts ? because if not it will continue printing but without filament

luc-github commented 9 years ago

here some issue about pause FYR: https://github.com/luc-github/Repetier-Firmware-0.92/issues/26 https://github.com/luc-github/Repetier-Firmware-0.92/issues/25

jnadke commented 9 years ago

The issue is the printer serial port has a buffer as well, so the host will send the commands as long as there's room.

When using waitUntilEndOfAllMoves the printer will finish out the serial buffer without any filament, and then when the print resumes the print could have moved an entire Z layer without filament (this is an issue I had).

Both Repetier-Host and OctoPrint seem to honor the Pause request, but there's still the issue of the depleted buffer.

You're probably more knowlegable than me right now on how Repetier works, I've finally been digging through the code this afternoon.

Ideally, the printer would stop immediately and let the serial buffer be full. The host end should honor the fact that the buffer is full (as long as ping-pong / ack request is enabled).

luc-github commented 9 years ago

well, serial buffer is one of the buffer, also have gcode buffer and line buffer. I had discussion with repetier developer and it is not in his plan to fix this issue of not immediate pause, if you look at his implementation of out of filament he just block usb connection but let buffer to get empty. that why I created issue in tracker, I just have no time for this right now with other projects, and actually seems affect few people as I do not see any complain on repetier github, which is weird for me. If you work on it , you are more than welcome - thanks for your great help

luc-github commented 9 years ago

so adding park position like this:

  if(sd.sdmode )sd.pausePrint(true);
  else //if printing from Host, request a pause
    {
    Com::printFLN(PSTR("RequestPause: No Filament!"));
    Commands::waitUntilEndOfAllMoves();
    Printer::moveToReal(IGNORE_COORDINATE,IGNORE_COORDINATE,Printer::currentPositionSteps[Z_AXIS]+10,IGNORE_COORDINATE,Printer::homingFeedrate[Z_AXIS]);
    Printer::moveToReal(Printer::xMin,Printer::yMin,IGNORE_COORDINATE,IGNORE_COORDINATE,Printer::homingFeedrate[0]);
}

should park noozle for host pause even not solving immediat pause, this should work with any host,
Another solution is to use the script "Run on pause" in repetier but I am not sure every host have this kind of setting

What do you think ?

luc-github commented 9 years ago

I just review the code and wondering why I did not implemented the position for host and now I remember why!!!! when using host you have additionnal macro at least in repetier when doing pause so I did not do anything to avoid conflict with host commands.

from another point of view I did not found such macro in S3D, so just for safety I will implement the park position to avoid nozzle to ruin model waiting for filament change as you suggest

luc-github commented 9 years ago

implemented https://github.com/luc-github/Repetier-Firmware-0.92/commit/c8194ad482673d00cd061a4566f910f4db9b9552