From aeed49a80e4b409be8406f5c0472a12ebd54b8f9 Mon Sep 17 00:00:00 2001 From: DRracer Date: Wed, 5 Jun 2019 12:19:18 +0200 Subject: [PATCH 1/3] Fix filament runout on optical filament sensors Reworked the IR variant accordingly Some code-size optimization in related functions --- Firmware/Marlin_main.cpp | 33 +++++++++---------- Firmware/fsensor.cpp | 71 +++++++++++++++++++++++++++------------- 2 files changed, 65 insertions(+), 39 deletions(-) mode change 100644 => 100755 Firmware/Marlin_main.cpp mode change 100644 => 100755 Firmware/fsensor.cpp diff --git a/Firmware/Marlin_main.cpp b/Firmware/Marlin_main.cpp old mode 100644 new mode 100755 index b431d6bb..02a420be --- a/Firmware/Marlin_main.cpp +++ b/Firmware/Marlin_main.cpp @@ -3553,10 +3553,6 @@ void process_commands() enquecommand_P(PSTR("M24")); } #ifdef FILAMENT_SENSOR - else if (code_seen("fsensor_recover_IR")) //! PRUSA fsensor_recover_IR - { - fsensor_restore_print_and_continue_IR(); - } else if (code_seen("fsensor_recover")) //! PRUSA fsensor_recover { fsensor_restore_print_and_continue(); @@ -6591,7 +6587,7 @@ if((eSoundMode==e_SOUND_MODE_LOUD)||(eSoundMode==e_SOUND_MODE_ONCE)) float x_position = current_position[X_AXIS]; float y_position = current_position[Y_AXIS]; - float z_shift = 0; + float z_shift = 0; // is it necessary to be a float? float e_shift_init = 0; float e_shift_late = 0; bool automatic = false; @@ -6628,8 +6624,13 @@ if((eSoundMode==e_SOUND_MODE_LOUD)||(eSoundMode==e_SOUND_MODE_ONCE)) else { #ifdef FILAMENTCHANGE_ZADD - z_shift= FILAMENTCHANGE_ZADD ; - if(current_position[Z_AXIS] < 25) z_shift+= 25 ; + static_assert(Z_MAX_POS < (255 - FILAMENTCHANGE_ZADD), "Z-range too high, change the following code from uint8_t to uint16_t"); + // avoid floating point arithmetics when not necessary - results in shorter code + uint8_t ztmp = uint8_t( current_position[Z_AXIS] ); + if(ztmp < uint8_t(25)){ + z_shift = uint8_t(25) - ztmp; // make sure to be at least 25mm above the heat bed + } + z_shift += FILAMENTCHANGE_ZADD ; // always move above printout #endif } //Move XY to side @@ -9273,22 +9274,20 @@ void stop_and_save_print_to_ram(float z_move, float e_move) // First unretract (relative extrusion) if(!saved_extruder_relative_mode){ - strcpy_P(buf, PSTR("M83")); - enquecommand(buf, false); + enquecommand(PSTR("M83"), true); } //retract 45mm/s - strcpy_P(buf, PSTR("G1 E")); - dtostrf(e_move, 6, 3, buf + strlen(buf)); - strcat_P(buf, PSTR(" F")); - dtostrf(2700, 8, 3, buf + strlen(buf)); + // A single sprintf may not be faster, but is definitely 20B shorter + // than a sequence of commands building the string piece by piece + // A snprintf would have been a safer call, but since it is not used + // in the whole program, its implementation would bring more bytes to the total size + // The behavior of dtostrf 8,3 should be roughly the same as %-0.3 + sprintf_P(buf, PSTR("G1 E%-0.3f F2700"), e_move); enquecommand(buf, false); // Then lift Z axis - strcpy_P(buf, PSTR("G1 Z")); - dtostrf(saved_pos[Z_AXIS] + z_move, 8, 3, buf + strlen(buf)); - strcat_P(buf, PSTR(" F")); - dtostrf(homing_feedrate[Z_AXIS], 8, 3, buf + strlen(buf)); + sprintf_P(buf, PSTR("G1 Z%-0.3f F%-0.3f"), saved_pos[Z_AXIS] + z_move, homing_feedrate[Z_AXIS]); // At this point the command queue is empty. enquecommand(buf, false); // If this call is invoked from the main Arduino loop() function, let the caller know that the command diff --git a/Firmware/fsensor.cpp b/Firmware/fsensor.cpp old mode 100644 new mode 100755 index f9ca8909..fd7936ee --- a/Firmware/fsensor.cpp +++ b/Firmware/fsensor.cpp @@ -123,17 +123,12 @@ void fsensor_stop_and_save_print(void) stop_and_save_print_to_ram(0, 0); //XYZE - no change } -void fsensor_restore_print_and_continue_IR(void) -{ - fsensor_watch_runout = true; - fsensor_err_cnt = 0; - fsensor_m600_enqueued = false; -} - void fsensor_restore_print_and_continue(void) { printf_P(PSTR("fsensor_restore_print_and_continue\n")); - fsensor_restore_print_and_continue_IR(); + fsensor_watch_runout = true; + fsensor_err_cnt = 0; + fsensor_m600_enqueued = false; restore_print_from_ram_and_continue(0); //XYZ = orig, E - no change } @@ -527,6 +522,47 @@ void fsensor_st_block_chunk(block_t* bl, int cnt) } } +//! This ensures generating z-position at least 25mm above the heat bed. +//! Making this a template enables changing the computation data type easily at all spots where necessary. +//! @param current_z current z-position +//! @return z-position at least 25mm above the heat bed plus FILAMENTCHANGE_ZADD +template +inline T fsensor_clamp_z(float current_z){ + T z( current_z ); + if(z < T(25)){ // make sure the compiler understands, that the constant 25 is of correct type + // - necessary for uint8_t -> results in shorter code + z = T(25); // move to at least 25mm above heat bed + } + return z + T(FILAMENTCHANGE_ZADD); // always move above the printout by FILAMENTCHANGE_ZADD (default 2mm) +} + +//! Common code for enqueing M600 and supplemental codes into the command queue. +//! Used both for the IR sensor and the PAT9125 +void fsensor_enque_M600(){ + printf_P(PSTR("fsensor_update - M600\n")); + eeprom_update_byte((uint8_t*)EEPROM_FERROR_COUNT, eeprom_read_byte((uint8_t*)EEPROM_FERROR_COUNT) + 1); + eeprom_update_word((uint16_t*)EEPROM_FERROR_COUNT_TOT, eeprom_read_word((uint16_t*)EEPROM_FERROR_COUNT_TOT) + 1); + enquecommand_front_P(PSTR("PRUSA fsensor_recover")); + fsensor_m600_enqueued = true; + enquecommand_front_P((PSTR("M600"))); +#define xstr(a) str(a) +#define str(a) #a + static const char gcodeMove[] PROGMEM = + "G1 X" xstr(FILAMENTCHANGE_XPOS) + " Y" xstr(FILAMENTCHANGE_YPOS) + " Z%u"; +#undef str +#undef xstr + char buf[32]; + // integer arithmetics is far shorter, I don't need a precise float position here, just move a bit above + // 8bit arithmetics in fsensor_clamp_z is 10B shorter than 16bit (not talking about float ;) ) + // The compile-time static_assert here ensures, that the computation gets enough bits in case of Z-range too high, + // i.e. makes the user change the data type, which also results in larger code + static_assert(Z_MAX_POS < (255 - FILAMENTCHANGE_ZADD), "Z-range too high, change fsensor_clamp_z to "); + sprintf_P(buf, gcodeMove, fsensor_clamp_z(current_position[Z_AXIS]) ); + enquecommand_front(buf, false); +} + //! @brief filament sensor update (perform M600 on filament runout) //! //! Works only if filament sensor is enabled. @@ -535,7 +571,7 @@ void fsensor_st_block_chunk(block_t* bl, int cnt) void fsensor_update(void) { #ifdef PAT9125 - if (fsensor_enabled && fsensor_watch_runout && (fsensor_err_cnt > FSENSOR_ERR_MAX)) + if (fsensor_enabled && fsensor_watch_runout && (fsensor_err_cnt > FSENSOR_ERR_MAX) && ( ! fsensor_m600_enqueued) ) { bool autoload_enabled_tmp = fsensor_autoload_enabled; fsensor_autoload_enabled = false; @@ -575,11 +611,7 @@ void fsensor_update(void) } else { - printf_P(PSTR("fsensor_update - M600\n")); - eeprom_update_byte((uint8_t*)EEPROM_FERROR_COUNT, eeprom_read_byte((uint8_t*)EEPROM_FERROR_COUNT) + 1); - eeprom_update_word((uint16_t*)EEPROM_FERROR_COUNT_TOT, eeprom_read_word((uint16_t*)EEPROM_FERROR_COUNT_TOT) + 1); - enquecommand_front_P(PSTR("PRUSA fsensor_recover")); - enquecommand_front_P((PSTR("M600"))); + fsensor_enque_M600(); fsensor_watch_runout = false; } fsensor_autoload_enabled = autoload_enabled_tmp; @@ -587,14 +619,9 @@ void fsensor_update(void) } #else //PAT9125 if ((digitalRead(IR_SENSOR_PIN) == 1) && CHECK_FSENSOR && fsensor_enabled && ir_sensor_detected && ( ! fsensor_m600_enqueued) ) - { // just plan a simple M600 without any additional position save/restore, - // which caused weird heating issues standing directly over the print - printf_P(PSTR("fsensor_update - M600\n")); - eeprom_update_byte((uint8_t*)EEPROM_FERROR_COUNT, eeprom_read_byte((uint8_t*)EEPROM_FERROR_COUNT) + 1); - eeprom_update_word((uint16_t*)EEPROM_FERROR_COUNT_TOT, eeprom_read_word((uint16_t*)EEPROM_FERROR_COUNT_TOT) + 1); - enquecommand_front_P(PSTR("PRUSA fsensor_recover_IR")); - fsensor_m600_enqueued = true; - enquecommand_front_P((PSTR("M600"))); + { + fsensor_stop_and_save_print(); + fsensor_enque_M600(); } #endif //PAT9125 } From 9c3bb14baeb87eb4adac28d20aa050428726c51d Mon Sep 17 00:00:00 2001 From: Marek Bel Date: Wed, 5 Jun 2019 18:34:10 +0200 Subject: [PATCH 2/3] remove function forward decl: fsensor_restore_print_and_continue_IR revert LCD_BL_PIN change --- Firmware/fsensor.h | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) mode change 100644 => 100755 Firmware/fsensor.h diff --git a/Firmware/fsensor.h b/Firmware/fsensor.h old mode 100644 new mode 100755 index 421eee43..fa7ab585 --- a/Firmware/fsensor.h +++ b/Firmware/fsensor.h @@ -18,9 +18,7 @@ extern bool fsensor_oq_meassure_enabled; //! @name save restore printing //! @{ extern void fsensor_stop_and_save_print(void); -//! special handling for the IR sensor (no restore position and heating, since this is already correctly handled in the M600 itself) -extern void fsensor_restore_print_and_continue_IR(void); -//! legacy restore print - restore position and heatup to original temperature - for the MMU and the optical fsensor +//! restore print - restore position and heatup to original temperature extern void fsensor_restore_print_and_continue(void); //! @} From 19351df8a72c9344ae984e42e432689a69c287d3 Mon Sep 17 00:00:00 2001 From: Marek Bel Date: Wed, 5 Jun 2019 18:34:26 +0200 Subject: [PATCH 3/3] Extracted computation of z_shift for M600 into a separate function to improve readability. Surprisingly, also the code got shorter. --- Firmware/Marlin_main.cpp | 36 +++++++++++++++++++++++++++--------- 1 file changed, 27 insertions(+), 9 deletions(-) diff --git a/Firmware/Marlin_main.cpp b/Firmware/Marlin_main.cpp index 02a420be..c4683da2 100755 --- a/Firmware/Marlin_main.cpp +++ b/Firmware/Marlin_main.cpp @@ -3001,6 +3001,32 @@ void gcode_M114() SERIAL_PROTOCOLLN(""); } +//! extracted code to compute z_shift for M600 in case of filament change operation +//! requested from fsensors. +//! The function ensures, that the printhead lifts to at least 25mm above the heat bed +//! unlike the previous implementation, which was adding 25mm even when the head was +//! printing at e.g. 24mm height. +//! A safety margin of FILAMENTCHANGE_ZADD is added in all cases to avoid touching +//! the printout. +//! This function is templated to enable fast change of computation data type. +//! @return new z_shift value +template +static T gcode_M600_filament_change_z_shift() +{ +#ifdef FILAMENTCHANGE_ZADD + static_assert(Z_MAX_POS < (255 - FILAMENTCHANGE_ZADD), "Z-range too high, change the T type from uint8_t to uint16_t"); + // avoid floating point arithmetics when not necessary - results in shorter code + T ztmp = T( current_position[Z_AXIS] ); + T z_shift = 0; + if(ztmp < T(25)){ + z_shift = T(25) - ztmp; // make sure to be at least 25mm above the heat bed + } + return z_shift + T(FILAMENTCHANGE_ZADD); // always move above printout +#else + return T(0); +#endif +} + static void gcode_M600(bool automatic, float x_position, float y_position, float z_shift, float e_shift, float /*e_shift_late*/) { st_synchronize(); @@ -6623,15 +6649,7 @@ if((eSoundMode==e_SOUND_MODE_LOUD)||(eSoundMode==e_SOUND_MODE_ONCE)) } else { - #ifdef FILAMENTCHANGE_ZADD - static_assert(Z_MAX_POS < (255 - FILAMENTCHANGE_ZADD), "Z-range too high, change the following code from uint8_t to uint16_t"); - // avoid floating point arithmetics when not necessary - results in shorter code - uint8_t ztmp = uint8_t( current_position[Z_AXIS] ); - if(ztmp < uint8_t(25)){ - z_shift = uint8_t(25) - ztmp; // make sure to be at least 25mm above the heat bed - } - z_shift += FILAMENTCHANGE_ZADD ; // always move above printout - #endif + z_shift = gcode_M600_filament_change_z_shift(); } //Move XY to side if(code_seen('X'))