MarlinFirmware / Marlin

Marlin is an optimized firmware for RepRap 3D printers based on the Arduino platform. Many commercial 3D printers come with Marlin installed. Check with your vendor if you need source code for your specific machine.
https://marlinfw.org
GNU General Public License v3.0
16.11k stars 19.2k forks source link

feature definition #4658

Closed ruggb closed 8 years ago

ruggb commented 8 years ago

The explanation in this section leaves something to be desired.

//this prevents dangerous Extruder moves, i.e. if the temperature is under the limit
//can be software-disabled for whatever purposes by
#define PREVENT_DANGEROUS_EXTRUDE
//if PREVENT_DANGEROUS_EXTRUDE is on, you can still disable (uncomment) very long bits of extrusion separately.
#define PREVENT_LENGTHY_EXTRUDE

Is there something missing on this line; `//can be software-disabled for whatever purposes by WHAT?

HOW do I do this?; //if PREVENT_DANGEROUS_EXTRUDE is on, you can still disable (uncomment) very long bits of extrusion separately. I don't see any commented lines to uncomment

Roxy-3D commented 8 years ago

There are two different extruder checks. One is done for temperature. Another is done for any extrude that is too long.

define PREVENT_LENGTHY_EXTRUDE

ruggb commented 8 years ago

I got that but this line appears to be missing something `//can be software-disabled for whatever purposes by -----------what? and this line tells me to do something but //if PREVENT_DANGEROUS_EXTRUDE is on, you can still disable (uncomment) very long bits of extrusion separately. I don't see any commented lines to uncomment

ruggb commented 8 years ago

Thanks for your answer----- nothing impolite - just stating facts -- I capped the question WHAT and HOW as I didn't want it to be missed.

However, you didn't answer my questions re by -- what and How to uncomment something I don't see.

On 8/19/2016 10:37 PM, Roxy-3D wrote:

A more polite tone is going to get you an answer faster.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/MarlinFirmware/Marlin/issues/4658#issuecomment-241173965, or mute the thread https://github.com/notifications/unsubscribe-auth/ALz0nlmRpgjnWeHJcC65JjMZ88-2eUBtks5qhmhhgaJpZM4Jo-QC.

Roxy-3D commented 8 years ago

If PREVENT_DANGEROUS_EXTRUDE is enabled, you can still disable it or adjust it with a M302. You can set the temperature it considers dangerous with an S parameter.

#if ENABLED(PREVENT_DANGEROUS_EXTRUDE)
  /**
   * M302: Allow cold extrudes, or set the minimum extrude S<temperature>.
   */
  inline void gcode_M302() {
    thermalManager.extrude_min_temp = code_seen('S') ? code_value_temp_abs() : 0;
  }
#endif // PREVENT_DANGEROUS_EXTRUDE

The actual logic to stop the extrude is here in Marlin_main.cpp

#if ENABLED(PREVENT_DANGEROUS_EXTRUDE)
  inline void prevent_dangerous_extrude(float& curr_e, float& dest_e) {
    if (DEBUGGING(DRYRUN)) return;
    float de = dest_e - curr_e;
    if (de) {
      if (thermalManager.tooColdToExtrude(active_extruder)) {
        curr_e = dest_e; // Behave as if the move really took place, but ignore E part
        SERIAL_ECHO_START;
        SERIAL_ECHOLNPGM(MSG_ERR_COLD_EXTRUDE_STOP);
      }
      #if ENABLED(PREVENT_LENGTHY_EXTRUDE)
        if (labs(de) > EXTRUDE_MAXLENGTH) {
          curr_e = dest_e; // Behave as if the move really took place, but ignore E part
          SERIAL_ECHO_START;
          SERIAL_ECHOLNPGM(MSG_ERR_LONG_EXTRUDE_STOP);
        }
      #endif
    }
  }
#endif // PREVENT_DANGEROUS_EXTRUDE

The code in Planner.cpp says in order to prevent a very long extrude, you also need to have PREVENT_DANGEROUS_EXTRUDE enabled:

  #if ENABLED(PREVENT_DANGEROUS_EXTRUDE)
    if (de) {
      if (thermalManager.tooColdToExtrude(extruder)) {
        position[E_AXIS] = target[E_AXIS]; // Behave as if the move really took place, but ignore E part
        de = 0; // no difference
        SERIAL_ECHO_START;
        SERIAL_ECHOLNPGM(MSG_ERR_COLD_EXTRUDE_STOP);
      }
      #if ENABLED(PREVENT_LENGTHY_EXTRUDE)
        if (labs(de) > axis_steps_per_mm[E_AXIS] * (EXTRUDE_MAXLENGTH)) {
          position[E_AXIS] = target[E_AXIS]; // Behave as if the move really took place, but ignore E part
          de = 0; // no difference
          SERIAL_ECHO_START;
          SERIAL_ECHOLNPGM(MSG_ERR_LONG_EXTRUDE_STOP);
        }
      #endif
    }
  #endif

The code in Temperature.cpp controls the default temperature that is considered too low to extrude:

#if ENABLED(PREVENT_DANGEROUS_EXTRUDE)
  float Temperature::extrude_min_temp = EXTRUDE_MINTEMP;
#endif

That is about the extent of the logic for these two controls.

ruggb commented 8 years ago

@Roxy-3D I love your answer. It is obvious there are pieces of this function all over the place. I can understand the difficulty in maintaining it. Wackerbarth has his work cut out to develop a modular form to facilitate maintenance. Now, given that the "instruction manual" for this firmware is entwined with the code in these config files, what can be added/changed in this line to make this comment clear? As it is, it tells nothing that I can understand. // (WHAT) can be software-disabled for whatever purposes by (HOW)

And how can this line be modified to tell me what line I am to uncomment? - though it would seem like I would want to comment something out to disable it. //if PREVENT_DANGEROUS_EXTRUDE is on, you can still disable (uncomment) very long bits of extrusion separately.

I'm not sure I would have called it "DANGEROUS" - COLD or DAMAGING seems more appropriate. "DANGEROUS" implies something like a meltdown.

FOLLOW ON ??? - does this have anything to do with a "dry run" function (M111)?

Roxy-3D commented 8 years ago

I always thought 'Dangerous' was a bit too dramatic. I don't know that this analysis is true. But sometimes I screw up and say something that needs to be corrected. If it is on a Thermal behavior topic, it will be somebody that speaks German that corrects me. Some times the Bi-(or Tri-)-Lingual people writing these comments miss subtle sub-meanings in the text they put in place. (And remember, they are trying to shrink down tons of thoughts to put into just a few words to make a good comment) I'm not even Bi-Lingual. I can notice a small issue with a comment where it is sort of implying something that is not true. But I could not write a similar comment in German.

This is Open Source. They won't complain if you clean up the English in the comments! Go for it!

ruggb commented 8 years ago

Well the function is called "DANGEROUS" so changing the comments will just confuse things unless the function is renamed. But hey they changed "EXTRUDER" to "HOTEND" so maybe there is hope. BUT my question remains unanswered and I have no clue what the answer is! And I wouldn't know how to change it if I did know.

Roxy-3D commented 8 years ago

Functions get renamed all the time. First there was 3-Point bed leveling. Then there was Accurate Bed Leveling. That stayed that way for a while, but it turned into Grid Leveling. Accurate implied the 3-Point was not accurate. And that was not true.

Let's wait a few days and see what others have to say. It would be interesting to know more about the history of this code, and to hear suggestions for changes. This code is evolving. It is now a C++ 'object'.

thinkyhead commented 8 years ago

this line appears to be missing something

It probably got truncated in error by some commit long ago. We can check the history of the file to see what it used to be.

Generally, I agree that we need to continue cleaning up and improving comments on options and on the code. I try to make such changes as I find them.

Wackerbarth has his work cut out…

Who? We haven't heard that name around here in a long, long time.

thinkyhead commented 8 years ago

I'll just change it to something along these lines…

// This option prevents extrusion if the temperature is below EXTRUDE_MINTEMP.
// It also enables the M302 command to set the minimum extrusion temperature
// or to allow moving the extruder regardless of the hotend temperature.
// *** IT IS HIGHLY RECOMMENDED TO LEAVE THIS OPTION ENABLED! ***
#define PREVENT_COLD_EXTRUSION
#define EXTRUDE_MINTEMP 170

// This option prevents a single extrusion longer than EXTRUDE_MAXLENGTH.
#define PREVENT_LENGTHY_EXTRUDE
#define EXTRUDE_MAXLENGTH (X_MAX_LENGTH+Y_MAX_LENGTH)
Roxy-3D commented 8 years ago

is it too late to make it:

#define EXTRUDE_MAXLENGTH (sqrt(X_MAX_LENGTH*X_MAX_LENGTH)+(Y_MAX_LENGTH*Y_MAX_LENGTH)))
thinkyhead commented 8 years ago

is it too late to make it:

The length here is actually very arbitrary. After all, we're talking about the amount of filament being fed into the extruder, not the length coming out. So adding the X and Y axis length is just a crude way of making some number proportional to the size of the XY plane. Really EXTRUDE_MAXLENGTH should just be some reasonably large fixed value like 500.

ruggb commented 8 years ago

Thank you! The follow on question was relative to DRY RUN. I don't know if this function is related, but a dry run implementation would be nice if it is simple.

thinkyhead commented 8 years ago

@ruggb To enable dry run use M111 S8.

ruggb commented 8 years ago

thx - I wasn't aware that it was a feature in Marlin yet.

thinkyhead commented 8 years ago

Yes, we have a lot of documentation to write.

tablatronix commented 4 years ago

I know this is old issue, but it is unclear if M302 relates to onlyPREVENT_COLD_EXTRUSION Does this only disable cold extrude or also lengthly extrude?

I was understanding this to toggle PREVENT_DANGEROUS_EXTRUDE which precludes both of these settings in the code.

github-actions[bot] commented 4 years ago

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[bot] commented 3 years ago

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[bot] commented 3 years ago

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[bot] commented 2 years ago

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.