From e8ce5e140e98bb3de1b690bb6aed014a276acd41 Mon Sep 17 00:00:00 2001 From: Yuri D'Elia Date: Sat, 1 Jun 2019 22:06:55 +0200 Subject: [PATCH 01/13] FS: Detect runout earlier --- Firmware/fsensor.cpp | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/Firmware/fsensor.cpp b/Firmware/fsensor.cpp index fce8f6fed..f4cb647a9 100755 --- a/Firmware/fsensor.cpp +++ b/Firmware/fsensor.cpp @@ -19,7 +19,7 @@ //! @name Basic parameters //! @{ #define FSENSOR_CHUNK_LEN 0.64F //!< filament sensor chunk length 0.64mm -#define FSENSOR_ERR_MAX 17 //!< filament sensor maximum error count for runout detection +#define FSENSOR_ERR_MAX 9 //!< filament sensor maximum error count for runout detection //! @} //! @name Optical quality measurement parameters @@ -453,10 +453,7 @@ ISR(FSENSOR_INT_PIN_VECT) { if (pat9125_y < 0) { - if (fsensor_err_cnt) - fsensor_err_cnt += 2; - else - fsensor_err_cnt++; + fsensor_err_cnt++; } else if (pat9125_y > 0) { From 8fb8ff4bf43eb371217ba9b80b4ea64cf8f7cee0 Mon Sep 17 00:00:00 2001 From: Yuri D'Elia Date: Sun, 2 Jun 2019 18:00:31 +0200 Subject: [PATCH 02/13] Include fsensor recovery stats in the last print stats --- Firmware/Marlin_main.cpp | 1 + Firmware/fsensor.cpp | 5 +++++ Firmware/fsensor.h | 1 + Firmware/ultralcd.cpp | 13 ++++++++----- 4 files changed, 15 insertions(+), 5 deletions(-) diff --git a/Firmware/Marlin_main.cpp b/Firmware/Marlin_main.cpp index 43e3081fe..0a1e5439b 100755 --- a/Firmware/Marlin_main.cpp +++ b/Firmware/Marlin_main.cpp @@ -634,6 +634,7 @@ void crashdet_cancel() void failstats_reset_print() { + fsensor_softfail = 0; eeprom_update_byte((uint8_t *)EEPROM_CRASH_COUNT_X, 0); eeprom_update_byte((uint8_t *)EEPROM_CRASH_COUNT_Y, 0); eeprom_update_byte((uint8_t *)EEPROM_FERROR_COUNT, 0); diff --git a/Firmware/fsensor.cpp b/Firmware/fsensor.cpp index f4cb647a9..19c8b1f81 100755 --- a/Firmware/fsensor.cpp +++ b/Firmware/fsensor.cpp @@ -83,6 +83,8 @@ uint8_t fsensor_autoload_c; uint32_t fsensor_autoload_last_millis; // uint8_t fsensor_autoload_sum; +// +uint8_t fsensor_softfail = 0; //! @} @@ -598,7 +600,10 @@ void fsensor_update(void) fsensor_oq_meassure_enabled = oq_meassure_enabled_tmp; if (!err) + { printf_P(PSTR("fsensor_err_cnt = 0\n")); + ++fsensor_softfail; + } else fsensor_enque_M600(); } diff --git a/Firmware/fsensor.h b/Firmware/fsensor.h index 5ed7f7fc8..fba131db3 100755 --- a/Firmware/fsensor.h +++ b/Firmware/fsensor.h @@ -14,6 +14,7 @@ extern bool fsensor_enabled; extern bool fsensor_not_responding; //enable/disable quality meassurement extern bool fsensor_oq_meassure_enabled; +extern uint8_t fsensor_softfail; //! @name save restore printing diff --git a/Firmware/ultralcd.cpp b/Firmware/ultralcd.cpp index cefd9b8c5..0461ff803 100755 --- a/Firmware/ultralcd.cpp +++ b/Firmware/ultralcd.cpp @@ -1796,11 +1796,14 @@ static void lcd_menu_fails_stats_print() uint8_t crashX = eeprom_read_byte((uint8_t*)EEPROM_CRASH_COUNT_X); uint8_t crashY = eeprom_read_byte((uint8_t*)EEPROM_CRASH_COUNT_Y); lcd_home(); - lcd_printf_P(failStatsFmt, - _i("Last print failures"), ////c=20 r=1 - _i("Power failures"), power, ////c=14 r=1 - _i("Filam. runouts"), filam, ////c=14 r=1 - _i("Crash"), crashX, crashY); ////c=7 r=1 + lcd_printf_P(PSTR("%S\n" + " %S %-3d\n" + " %S H %-3d S %-3d\n" + " %S X %-3d Y %-3d"), + _i("Last print failures"), + _i("Power failures"), power, + _i("Runouts"), filam, fsensor_softfail, + _i("Crash"), crashX, crashY); menu_back_if_clicked_fb(); } From ce74b746f14cf8ea54be925b154838cf0cb0de47 Mon Sep 17 00:00:00 2001 From: Yuri D'Elia Date: Mon, 3 Jun 2019 17:31:19 +0200 Subject: [PATCH 03/13] FS: Trigger a runout with repeated soft-failures (clog?) --- Firmware/fsensor.cpp | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/Firmware/fsensor.cpp b/Firmware/fsensor.cpp index 19c8b1f81..7daea8dac 100755 --- a/Firmware/fsensor.cpp +++ b/Firmware/fsensor.cpp @@ -20,6 +20,9 @@ //! @{ #define FSENSOR_CHUNK_LEN 0.64F //!< filament sensor chunk length 0.64mm #define FSENSOR_ERR_MAX 9 //!< filament sensor maximum error count for runout detection + +#define FSENSOR_SOFTERR_CMAX 3 //!< number of contiguous soft failures before a triggering a runout +#define FSENSOR_SOFTERR_DELTA 30000 //!< maximum interval (ms) to consider soft failures contiguous //! @} //! @name Optical quality measurement parameters @@ -85,6 +88,8 @@ uint32_t fsensor_autoload_last_millis; uint8_t fsensor_autoload_sum; // uint8_t fsensor_softfail = 0; +uint8_t fsensor_softfail_ccnt = 0; +unsigned long fsensor_softfail_last = 0; //! @} @@ -596,16 +601,25 @@ void fsensor_update(void) err |= (fsensor_oq_yd_sum < (4 * FSENSOR_OQ_MIN_YD)); fsensor_restore_print_and_continue(); - fsensor_autoload_enabled = autoload_enabled_tmp; - fsensor_oq_meassure_enabled = oq_meassure_enabled_tmp; + fsensor_autoload_enabled = autoload_enabled_tmp; + fsensor_oq_meassure_enabled = oq_meassure_enabled_tmp; - if (!err) + unsigned long now = _millis(); + if (!err && (now - fsensor_softfail_last) > FSENSOR_SOFTERR_DELTA) + fsensor_softfail_ccnt = 0; + if (!err && fsensor_softfail_ccnt <= FSENSOR_SOFTERR_CMAX) { - printf_P(PSTR("fsensor_err_cnt = 0\n")); + printf_P(PSTR("fsensor_err_cnt = 0\n")); ++fsensor_softfail; - } - else - fsensor_enque_M600(); + ++fsensor_softfail_ccnt; + fsensor_softfail_last = now; + } + else + { + fsensor_softfail_ccnt = 0; + fsensor_softfail_last = 0; + fsensor_enque_M600(); + } } #else //PAT9125 if (CHECK_FSENSOR && ir_sensor_detected) From d47363d85a5a813e86af8458bacc0f4970904615 Mon Sep 17 00:00:00 2001 From: Yuri D'Elia Date: Wed, 5 Feb 2020 14:23:41 +0100 Subject: [PATCH 04/13] Update the filament axis resolution when E resolution is changed The filament sensor "chunk lenght" needs to be updated every time the E axis resolution is changed in order to trigger at the same distance. Introduce a new function fsensor_set_axis_steps_per_unit() and use it consistent during init, in M92 and M350. --- Firmware/Marlin_main.cpp | 6 ++++-- Firmware/fsensor.cpp | 7 ++++++- Firmware/fsensor.h | 3 +++ 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/Firmware/Marlin_main.cpp b/Firmware/Marlin_main.cpp index 0a1e5439b..047b7082b 100755 --- a/Firmware/Marlin_main.cpp +++ b/Firmware/Marlin_main.cpp @@ -6673,7 +6673,7 @@ Sigma_Exit: { if(code_seen(axis_codes[i])) { - if(i == 3) { // E + if(i == E_AXIS) { // E float value = code_value(); if(value < 20.0) { float factor = cs.axis_steps_per_unit[i] / value; // increase e constants if M92 E14 is given for netfab. @@ -6682,6 +6682,7 @@ Sigma_Exit: axis_steps_per_sqr_second[i] *= factor; } cs.axis_steps_per_unit[i] = value; + fsensor_set_axis_steps_per_unit(value); } else { cs.axis_steps_per_unit[i] = code_value(); @@ -8429,7 +8430,6 @@ Sigma_Exit: res_valid |= (i == E_AXIS) && ((res_new == 64) || (res_new == 128)); // resolutions valid for E only if (res_valid) { - st_synchronize(); uint16_t res = tmc2130_get_res(i); tmc2130_set_res(i, res_new); @@ -8446,6 +8446,8 @@ Sigma_Exit: cs.axis_steps_per_unit[i] /= fac; position[i] /= fac; } + if (i == E_AXIS) + fsensor_set_axis_steps_per_unit(cs.axis_steps_per_unit[i]); } } } diff --git a/Firmware/fsensor.cpp b/Firmware/fsensor.cpp index 7daea8dac..297460d61 100755 --- a/Firmware/fsensor.cpp +++ b/Firmware/fsensor.cpp @@ -149,6 +149,11 @@ void fsensor_checkpoint_print(void) restore_print_from_ram_and_continue(0); } +void fsensor_set_axis_steps_per_unit(float u) +{ + fsensor_chunk_len = (int16_t)(FSENSOR_CHUNK_LEN * u); +} + void fsensor_init(void) { #ifdef PAT9125 @@ -161,7 +166,7 @@ void fsensor_init(void) #ifdef PAT9125 uint8_t oq_meassure_enabled = eeprom_read_byte((uint8_t*)EEPROM_FSENS_OQ_MEASS_ENABLED); fsensor_oq_meassure_enabled = (oq_meassure_enabled == 1)?true:false; - fsensor_chunk_len = (int16_t)(FSENSOR_CHUNK_LEN * cs.axis_steps_per_unit[E_AXIS]); + fsensor_set_axis_steps_per_unit(cs.axis_steps_per_unit[E_AXIS]); if (!pat9125) { diff --git a/Firmware/fsensor.h b/Firmware/fsensor.h index fba131db3..0c791d412 100755 --- a/Firmware/fsensor.h +++ b/Firmware/fsensor.h @@ -29,6 +29,9 @@ extern void fsensor_checkpoint_print(void); //! initialize extern void fsensor_init(void); +//! update axis resolution +extern void fsensor_set_axis_steps_per_unit(float u); + //! @name enable/disable //! @{ extern bool fsensor_enable(bool bUpdateEEPROM=true); From 6fbd632c844c527bb0812e36bc03318097d54087 Mon Sep 17 00:00:00 2001 From: Yuri D'Elia Date: Wed, 5 Feb 2020 15:17:19 +0100 Subject: [PATCH 05/13] Isolate all PAT9125-specific code, fix build on !MK3 variants - Hide all prototypes related to PAT9125 to force all callers to check for the proper sensor, since the handling differences are substantial - Remove unneeded lenght accounting from the stepper isr as as consequence. - Keep detailed soft failure counts for the MK3 on the "last print failures" status screen, but fix build on variants without a PAT9125 by fixing the lcd stats function. --- Firmware/Marlin_main.cpp | 8 +++++++- Firmware/fsensor.cpp | 32 ++++++++++++++++++++------------ Firmware/fsensor.h | 15 ++++++++++----- Firmware/stepper.cpp | 14 +++++++------- Firmware/ultralcd.cpp | 25 ++++++++++++++++++------- 5 files changed, 62 insertions(+), 32 deletions(-) diff --git a/Firmware/Marlin_main.cpp b/Firmware/Marlin_main.cpp index 047b7082b..1dcb89389 100755 --- a/Firmware/Marlin_main.cpp +++ b/Firmware/Marlin_main.cpp @@ -634,13 +634,15 @@ void crashdet_cancel() void failstats_reset_print() { - fsensor_softfail = 0; eeprom_update_byte((uint8_t *)EEPROM_CRASH_COUNT_X, 0); eeprom_update_byte((uint8_t *)EEPROM_CRASH_COUNT_Y, 0); eeprom_update_byte((uint8_t *)EEPROM_FERROR_COUNT, 0); eeprom_update_byte((uint8_t *)EEPROM_POWER_COUNT, 0); eeprom_update_byte((uint8_t *)EEPROM_MMU_FAIL, 0); eeprom_update_byte((uint8_t *)EEPROM_MMU_LOAD_FAIL, 0); +#if defined(FILAMENT_SENSOR) && defined(PAT9125) + fsensor_softfail = 0; +#endif } @@ -6682,7 +6684,9 @@ Sigma_Exit: axis_steps_per_sqr_second[i] *= factor; } cs.axis_steps_per_unit[i] = value; +#if defined(FILAMENT_SENSOR) && defined(PAT9125) fsensor_set_axis_steps_per_unit(value); +#endif } else { cs.axis_steps_per_unit[i] = code_value(); @@ -8446,8 +8450,10 @@ Sigma_Exit: cs.axis_steps_per_unit[i] /= fac; position[i] /= fac; } +#if defined(FILAMENT_SENSOR) && defined(PAT9125) if (i == E_AXIS) fsensor_set_axis_steps_per_unit(cs.axis_steps_per_unit[i]); +#endif } } } diff --git a/Firmware/fsensor.cpp b/Firmware/fsensor.cpp index 297460d61..c1f9f27b0 100755 --- a/Firmware/fsensor.cpp +++ b/Firmware/fsensor.cpp @@ -47,25 +47,32 @@ const char ERRMSG_PAT9125_NOT_RESP[] PROGMEM = "PAT9125 not responding (%d)!\n"; #define FSENSOR_INT_PIN_PCMSK_BIT PCINT13 // PinChange Interrupt / PinChange Enable Mask @ PJ4 #define FSENSOR_INT_PIN_PCICR_BIT PCIE1 // PinChange Interrupt Enable / Flag @ PJ4 -//uint8_t fsensor_int_pin = FSENSOR_INT_PIN; -uint8_t fsensor_int_pin_old = 0; -int16_t fsensor_chunk_len = 0; - //! enabled = initialized and sampled every chunk event bool fsensor_enabled = true; //! runout watching is done in fsensor_update (called from main loop) bool fsensor_watch_runout = true; //! not responding - is set if any communication error occurred during initialization or readout bool fsensor_not_responding = false; + +#ifdef PAT9125 +uint8_t fsensor_int_pin_old = 0; +//! optical checking "chunk lenght" (already in steps) +int16_t fsensor_chunk_len = 0; //! enable/disable quality meassurement bool fsensor_oq_meassure_enabled = false; - //! number of errors, updated in ISR uint8_t fsensor_err_cnt = 0; //! variable for accumulating step count (updated callbacks from stepper and ISR) int16_t fsensor_st_cnt = 0; //! last dy value from pat9125 sensor (used in ISR) int16_t fsensor_dy_old = 0; +//! count of total sensor "soft" failures (filament status checks) +uint8_t fsensor_softfail = 0; +//! timestamp of last soft failure +unsigned long fsensor_softfail_last = 0; +//! count of soft failures within the configured time +uint8_t fsensor_softfail_ccnt = 0; +#endif //! log flag: 0=log disabled, 1=log enabled uint8_t fsensor_log = 1; @@ -78,6 +85,8 @@ uint8_t fsensor_log = 1; bool fsensor_autoload_enabled = true; //! autoload watching enable/disable flag bool fsensor_watch_autoload = false; + +#ifdef PAT9125 // uint16_t fsensor_autoload_y; // @@ -86,11 +95,8 @@ uint8_t fsensor_autoload_c; uint32_t fsensor_autoload_last_millis; // uint8_t fsensor_autoload_sum; -// -uint8_t fsensor_softfail = 0; -uint8_t fsensor_softfail_ccnt = 0; -unsigned long fsensor_softfail_last = 0; //! @} +#endif //! @name filament optical quality measurement variables @@ -136,7 +142,9 @@ void fsensor_restore_print_and_continue(void) { printf_P(PSTR("fsensor_restore_print_and_continue\n")); fsensor_watch_runout = true; +#ifdef PAT9125 fsensor_err_cnt = 0; +#endif restore_print_from_ram_and_continue(0); } @@ -368,6 +376,7 @@ bool fsensor_check_autoload(void) return false; } +#ifdef PAT9125 void fsensor_oq_meassure_set(bool State) { fsensor_oq_meassure_enabled = State; @@ -439,7 +448,7 @@ bool fsensor_oq_result(void) printf_P(_N("fsensor_oq_result %S\n"), (res?_OK:_NG)); return res; } -#ifdef PAT9125 + ISR(FSENSOR_INT_PIN_VECT) { if (mmu_enabled || ir_sensor_detected) return; @@ -538,8 +547,6 @@ void fsensor_setup_interrupt(void) PCICR |= bit(FSENSOR_INT_PIN_PCICR_BIT); // enable corresponding PinChangeInterrupt (set of pins) } -#endif //PAT9125 - void fsensor_st_block_chunk(int cnt) { if (!fsensor_enabled) return; @@ -551,6 +558,7 @@ void fsensor_st_block_chunk(int cnt) else {PIN_VAL(FSENSOR_INT_PIN, HIGH);} } } +#endif //PAT9125 //! Common code for enqueing M600 and supplemental codes into the command queue. diff --git a/Firmware/fsensor.h b/Firmware/fsensor.h index 0c791d412..373200dc8 100755 --- a/Firmware/fsensor.h +++ b/Firmware/fsensor.h @@ -6,16 +6,16 @@ #include "config.h" -//! minimum meassured chunk length in steps -extern int16_t fsensor_chunk_len; // enable/disable flag extern bool fsensor_enabled; // not responding flag extern bool fsensor_not_responding; -//enable/disable quality meassurement -extern bool fsensor_oq_meassure_enabled; +#ifdef PAT9125 +// optical checking "chunk lenght" (already in steps) +extern int16_t fsensor_chunk_len; +// count of soft failures extern uint8_t fsensor_softfail; - +#endif //! @name save restore printing //! @{ @@ -29,8 +29,10 @@ extern void fsensor_checkpoint_print(void); //! initialize extern void fsensor_init(void); +#ifdef PAT9125 //! update axis resolution extern void fsensor_set_axis_steps_per_unit(float u); +#endif //! @name enable/disable //! @{ @@ -56,8 +58,10 @@ extern void fsensor_autoload_check_stop(void); extern bool fsensor_check_autoload(void); //! @} +#ifdef PAT9125 //! @name optical quality measurement support //! @{ +extern bool fsensor_oq_meassure_enabled; extern void fsensor_oq_meassure_set(bool State); extern void fsensor_oq_meassure_start(uint8_t skip); extern void fsensor_oq_meassure_stop(void); @@ -74,6 +78,7 @@ extern void fsensor_st_block_chunk(int cnt); // to drain fsensor_st_cnt anyway at the beginning of the new block. #define fsensor_st_block_begin(rev) fsensor_st_block_chunk(0) //! @} +#endif //PAT9125 #if IR_SENSOR_ANALOG diff --git a/Firmware/stepper.cpp b/Firmware/stepper.cpp index 74625a633..93b3d0397 100644 --- a/Firmware/stepper.cpp +++ b/Firmware/stepper.cpp @@ -36,9 +36,9 @@ #include "tmc2130.h" #endif //TMC2130 -#ifdef FILAMENT_SENSOR +#if defined(FILAMENT_SENSOR) && defined(PAT9125) #include "fsensor.h" -int fsensor_counter = 0; //counter for e-steps +int fsensor_counter; //counter for e-steps #endif //FILAMENT_SENSOR #include "mmu.h" @@ -421,9 +421,9 @@ FORCE_INLINE void stepper_next_block() #endif /* LIN_ADVANCE */ count_direction[E_AXIS] = 1; } -#ifdef FILAMENT_SENSOR - fsensor_counter = 0; - fsensor_st_block_begin(count_direction[E_AXIS] < 0); +#if defined(FILAMENT_SENSOR) && defined(PAT9125) + fsensor_counter = 0; + fsensor_st_block_begin(count_direction[E_AXIS] < 0); #endif //FILAMENT_SENSOR } else { @@ -973,13 +973,13 @@ FORCE_INLINE void advance_isr_scheduler() { WRITE_NC(E0_STEP_PIN, !INVERT_E_STEP_PIN); e_steps += (rev? 1: -1); WRITE_NC(E0_STEP_PIN, INVERT_E_STEP_PIN); -#ifdef FILAMENT_SENSOR +#if defined(FILAMENT_SENSOR) && defined(PAT9125) fsensor_counter += (rev? -1: 1); #endif } while(--max_ticks); -#ifdef FILAMENT_SENSOR +#if defined(FILAMENT_SENSOR) && defined(PAT9125) if (abs(fsensor_counter) >= fsensor_chunk_len) { fsensor_st_block_chunk(fsensor_counter); diff --git a/Firmware/ultralcd.cpp b/Firmware/ultralcd.cpp index 0461ff803..fc39b1405 100755 --- a/Firmware/ultralcd.cpp +++ b/Firmware/ultralcd.cpp @@ -1796,14 +1796,23 @@ static void lcd_menu_fails_stats_print() uint8_t crashX = eeprom_read_byte((uint8_t*)EEPROM_CRASH_COUNT_X); uint8_t crashY = eeprom_read_byte((uint8_t*)EEPROM_CRASH_COUNT_Y); lcd_home(); +#ifndef PAT9125 + lcd_printf_P(failStatsFmt, + _i("Last print failures"), ////c=20 r=1 + _i("Power failures"), power, ////c=14 r=1 + _i("Filam. runouts"), filam, ////c=14 r=1 + _i("Crash"), crashX, crashY); ////c=7 r=1 +#else + // On the MK3 include detailed PAT9125 statistics about soft failures lcd_printf_P(PSTR("%S\n" - " %S %-3d\n" - " %S H %-3d S %-3d\n" - " %S X %-3d Y %-3d"), - _i("Last print failures"), - _i("Power failures"), power, - _i("Runouts"), filam, fsensor_softfail, - _i("Crash"), crashX, crashY); + " %-16.16S%-3d\n" + " %-7.7S H %-3d S %-3d\n" + " %-7.7S X %-3d Y %-3d"), + _i("Last print failures"), ////c=20 r=1 + _i("Power failures"), power, ////c=14 r=1 + _i("Runouts"), filam, fsensor_softfail, //c=7 r=1 + _i("Crash"), crashX, crashY); ////c=7 r=1 +#endif menu_back_if_clicked_fb(); } @@ -2234,10 +2243,12 @@ void lcd_set_filament_autoload() { fsensor_autoload_set(!fsensor_autoload_enabled); } +#if defined(FILAMENT_SENSOR) && defined(PAT9125) void lcd_set_filament_oq_meass() { fsensor_oq_meassure_set(!fsensor_oq_meassure_enabled); } +#endif FilamentAction eFilamentAction=FilamentAction::None; // must be initialized as 'non-autoLoad' From 5c4235b886b678335146bee4dca85ad7635ff497 Mon Sep 17 00:00:00 2001 From: Yuri D'Elia Date: Wed, 5 Feb 2020 15:38:39 +0100 Subject: [PATCH 06/13] Correctly reset the PAT9125 filament counters When the error count is cleared, the cumulative deltas as well as the segment lenght which is kept inside the stepper isr need to be reset. Introduce a helper function fsensor_reset_err_cnt to clear all the required variables in one place which can be used in most cases (the only exceptions being quality measument). Introduce a new function st_reset_fsensor to also clear the segment lenght within the isr. --- Firmware/fsensor.cpp | 38 ++++++++++++++++++++++---------------- Firmware/stepper.cpp | 12 ++++++++++-- Firmware/stepper.h | 5 ++++- 3 files changed, 36 insertions(+), 19 deletions(-) diff --git a/Firmware/fsensor.cpp b/Firmware/fsensor.cpp index c1f9f27b0..3c42a3424 100755 --- a/Firmware/fsensor.cpp +++ b/Firmware/fsensor.cpp @@ -138,12 +138,28 @@ void fsensor_stop_and_save_print(void) fsensor_watch_runout = false; } +#ifdef PAT9125 +// Reset all internal counters to zero, including stepper callbacks +void fsensor_reset_err_cnt() +{ + fsensor_err_cnt = 0; + pat9125_y = 0; + st_reset_fsensor(); +} + +void fsensor_set_axis_steps_per_unit(float u) +{ + fsensor_chunk_len = (int16_t)(FSENSOR_CHUNK_LEN * u); +} +#endif + + void fsensor_restore_print_and_continue(void) { printf_P(PSTR("fsensor_restore_print_and_continue\n")); fsensor_watch_runout = true; #ifdef PAT9125 - fsensor_err_cnt = 0; + fsensor_reset_err_cnt(); #endif restore_print_from_ram_and_continue(0); } @@ -157,11 +173,6 @@ void fsensor_checkpoint_print(void) restore_print_from_ram_and_continue(0); } -void fsensor_set_axis_steps_per_unit(float u) -{ - fsensor_chunk_len = (int16_t)(FSENSOR_CHUNK_LEN * u); -} - void fsensor_init(void) { #ifdef PAT9125 @@ -214,7 +225,7 @@ bool fsensor_enable(bool bUpdateEEPROM) fsensor_enabled = pat9125 ? true : false; fsensor_watch_runout = true; fsensor_oq_meassure = false; - fsensor_err_cnt = 0; + fsensor_reset_err_cnt(); fsensor_dy_old = 0; eeprom_update_byte((uint8_t*)EEPROM_FSENSOR, fsensor_enabled ? 0x01 : 0x00); FSensorStateMenu = fsensor_enabled ? 1 : 0; @@ -295,12 +306,11 @@ void fsensor_autoload_check_start(void) fsensor_autoload_last_millis = _millis(); fsensor_watch_runout = false; fsensor_watch_autoload = true; - fsensor_err_cnt = 0; } + void fsensor_autoload_check_stop(void) { - // puts_P(_N("fsensor_autoload_check_stop\n")); if (!fsensor_enabled) return; // puts_P(_N("fsensor_autoload_check_stop 1\n")); @@ -311,7 +321,7 @@ void fsensor_autoload_check_stop(void) fsensor_autoload_sum = 0; fsensor_watch_autoload = false; fsensor_watch_runout = true; - fsensor_err_cnt = 0; + fsensor_reset_err_cnt(); } #endif //PAT9125 @@ -410,7 +420,6 @@ void fsensor_oq_meassure_stop(void) printf_P(_N(" st_sum=%u yd_sum=%u er_sum=%u er_max=%hhu\n"), fsensor_oq_st_sum, fsensor_oq_yd_sum, fsensor_oq_er_sum, fsensor_oq_er_max); printf_P(_N(" yd_min=%u yd_max=%u yd_avg=%u sh_avg=%u\n"), fsensor_oq_yd_min, fsensor_oq_yd_max, (uint16_t)((uint32_t)fsensor_oq_yd_sum * fsensor_chunk_len / fsensor_oq_st_sum), (uint16_t)(fsensor_oq_sh_sum / fsensor_oq_samples)); fsensor_oq_meassure = false; - fsensor_err_cnt = 0; } const char _OK[] PROGMEM = "OK"; @@ -595,7 +604,7 @@ void fsensor_update(void) st_synchronize(); // check the filament in isolation - fsensor_err_cnt = 0; + fsensor_reset_err_cnt(); fsensor_oq_meassure_start(0); float e_tmp = current_position[E_AXIS]; current_position[E_AXIS] -= 3; @@ -603,13 +612,10 @@ void fsensor_update(void) current_position[E_AXIS] = e_tmp; plan_buffer_line_curposXYZE(200/60, active_extruder); st_synchronize(); - - uint8_t err_cnt = fsensor_err_cnt; fsensor_oq_meassure_stop(); bool err = false; - err |= (err_cnt > 1); - + err |= (fsensor_err_cnt > 1); err |= (fsensor_oq_er_sum > 2); err |= (fsensor_oq_yd_sum < (4 * FSENSOR_OQ_MIN_YD)); diff --git a/Firmware/stepper.cpp b/Firmware/stepper.cpp index 93b3d0397..93e4ad8fa 100644 --- a/Firmware/stepper.cpp +++ b/Firmware/stepper.cpp @@ -1357,8 +1357,6 @@ void quickStop() } #ifdef BABYSTEPPING - - void babystep(const uint8_t axis,const bool direction) { //MUST ONLY BE CALLED BY A ISR, it depends on that no other ISR interrupts this @@ -1594,3 +1592,13 @@ void microstep_readings() #endif } #endif //TMC2130 + + +#if defined(FILAMENT_SENSOR) && defined(PAT9125) +void st_reset_fsensor() +{ + CRITICAL_SECTION_START; + fsensor_counter = 0; + CRITICAL_SECTION_END; +} +#endif //FILAMENT_SENSOR diff --git a/Firmware/stepper.h b/Firmware/stepper.h index 7c41743cd..7fdf426f3 100644 --- a/Firmware/stepper.h +++ b/Firmware/stepper.h @@ -92,7 +92,10 @@ void microstep_readings(); #ifdef BABYSTEPPING void babystep(const uint8_t axis,const bool direction); // perform a short step with a single stepper motor, outside of any convention #endif - +#if defined(FILAMENT_SENSOR) && defined(PAT9125) +// reset the internal filament sensor state +void st_reset_fsensor(); +#endif #endif From e84f82a6757ec3bfe3a9473b71f145775f631bae Mon Sep 17 00:00:00 2001 From: Yuri D'Elia Date: Wed, 5 Feb 2020 15:46:45 +0100 Subject: [PATCH 07/13] Rewrite filament sensor PAT9125 error handling Rewrite the logic behind the "chunking"/error count behind the PAT9125. Basic idea: check the _direction_ of movement returned by the optical sensor and compare it to the direction of the stepper. To avoid doing this continuosly (and because the optical sensor doesn't necessarily have the accuracy to track small distances), do so in chunks. Each time a chunk doesn't match the expected direction, increase the error count. Several improvements were done to the previous code: - Increase the chunk window: this ensures that a filament with poor response returns an usable direction, while also moving the average return values from the sensor in the middle of the 12 bits available for maximum effectiveness. - Since the returned values are more reliable, reduce the error count (1.25mm*4 = ~5mm before runout detection) - Track _both_ positive and negative movement, although only trigger errors during extrusion (necessary due to several assumptions made in the mmu/unloading code) - Do not reset the counters for each block: accumulate distances correctly, allowing detection of any block lenght. --- Firmware/config.h | 3 +- Firmware/fsensor.cpp | 104 +++++++++++++++++++++---------------------- Firmware/stepper.cpp | 1 - 3 files changed, 53 insertions(+), 55 deletions(-) diff --git a/Firmware/config.h b/Firmware/config.h index 241a2f20b..5c8f7cd98 100644 --- a/Firmware/config.h +++ b/Firmware/config.h @@ -34,7 +34,8 @@ //#define PAT9125_I2C_ADDR 0x79 //ID=HI //#define PAT9125_I2C_ADDR 0x73 //ID=NC #define PAT9125_XRES 0 -#define PAT9125_YRES 240 +#define PAT9125_YRES 240 // maximum resolution (5*X cpi) +#define PAT9124_YRES_MM (5*PAT9125_YRES/25.4) // counts per mm //SM4 configuration #define SM4_DEFDELAY 500 //default step delay [us] diff --git a/Firmware/fsensor.cpp b/Firmware/fsensor.cpp index 3c42a3424..ea171906f 100755 --- a/Firmware/fsensor.cpp +++ b/Firmware/fsensor.cpp @@ -18,8 +18,8 @@ //! @name Basic parameters //! @{ -#define FSENSOR_CHUNK_LEN 0.64F //!< filament sensor chunk length 0.64mm -#define FSENSOR_ERR_MAX 9 //!< filament sensor maximum error count for runout detection +#define FSENSOR_CHUNK_LEN 1.25 //!< filament sensor chunk length (mm) +#define FSENSOR_ERR_MAX 4 //!< filament sensor maximum error/chunk count for runout detection #define FSENSOR_SOFTERR_CMAX 3 //!< number of contiguous soft failures before a triggering a runout #define FSENSOR_SOFTERR_DELTA 30000 //!< maximum interval (ms) to consider soft failures contiguous @@ -64,8 +64,6 @@ bool fsensor_oq_meassure_enabled = false; uint8_t fsensor_err_cnt = 0; //! variable for accumulating step count (updated callbacks from stepper and ISR) int16_t fsensor_st_cnt = 0; -//! last dy value from pat9125 sensor (used in ISR) -int16_t fsensor_dy_old = 0; //! count of total sensor "soft" failures (filament status checks) uint8_t fsensor_softfail = 0; //! timestamp of last soft failure @@ -226,7 +224,6 @@ bool fsensor_enable(bool bUpdateEEPROM) fsensor_watch_runout = true; fsensor_oq_meassure = false; fsensor_reset_err_cnt(); - fsensor_dy_old = 0; eeprom_update_byte((uint8_t*)EEPROM_FSENSOR, fsensor_enabled ? 0x01 : 0x00); FSensorStateMenu = fsensor_enabled ? 1 : 0; } @@ -477,53 +474,56 @@ ISR(FSENSOR_INT_PIN_VECT) fsensor_not_responding = true; printf_P(ERRMSG_PAT9125_NOT_RESP, 1); } + if (st_cnt != 0) - { //movement - if (st_cnt > 0) //positive movement - { - if (pat9125_y < 0) - { - fsensor_err_cnt++; - } - else if (pat9125_y > 0) - { - if (fsensor_err_cnt) - fsensor_err_cnt--; - } - else //(pat9125_y == 0) - if (((fsensor_dy_old <= 0) || (fsensor_err_cnt)) && (st_cnt > (fsensor_chunk_len >> 1))) - fsensor_err_cnt++; - if (fsensor_oq_meassure) - { - if (fsensor_oq_skipchunk) - { - fsensor_oq_skipchunk--; - fsensor_err_cnt = 0; - } - else - { - if (st_cnt == fsensor_chunk_len) - { - if (pat9125_y > 0) if (fsensor_oq_yd_min > pat9125_y) fsensor_oq_yd_min = (fsensor_oq_yd_min + pat9125_y) / 2; - if (pat9125_y >= 0) if (fsensor_oq_yd_max < pat9125_y) fsensor_oq_yd_max = (fsensor_oq_yd_max + pat9125_y) / 2; - } - fsensor_oq_samples++; - fsensor_oq_st_sum += st_cnt; - if (pat9125_y > 0) fsensor_oq_yd_sum += pat9125_y; - if (fsensor_err_cnt > old_err_cnt) - fsensor_oq_er_sum += (fsensor_err_cnt - old_err_cnt); - if (fsensor_oq_er_max < fsensor_err_cnt) - fsensor_oq_er_max = fsensor_err_cnt; - fsensor_oq_sh_sum += pat9125_s; - } - } - } - else //negative movement - { - } - } - else - { //no movement + { + // movement was planned, check for sensor movement + int8_t st_dir = st_cnt >= 0; + int8_t pat9125_dir = pat9125_y >= 0; + + if (pat9125_y == 0) + { + // no movement detected: increase error count only when extruding, since fast retracts + // cannot always be seen. We also need to ensure that no runout is generated while + // retracting as it's not currently handled everywhere + if (st_dir) ++fsensor_err_cnt; + } + else if (pat9125_dir != st_dir) + { + // detected direction opposite of motor movement + if (st_dir) ++fsensor_err_cnt; + } + else if (pat9125_dir == st_dir) + { + // direction agreeing with planned movement + if (fsensor_err_cnt) --fsensor_err_cnt; + } + + if (st_dir && fsensor_oq_meassure) + { + // extruding with quality assessment + if (fsensor_oq_skipchunk) + { + fsensor_oq_skipchunk--; + fsensor_err_cnt = 0; + } + else + { + if (st_cnt == fsensor_chunk_len) + { + if (pat9125_y > 0) if (fsensor_oq_yd_min > pat9125_y) fsensor_oq_yd_min = (fsensor_oq_yd_min + pat9125_y) / 2; + if (pat9125_y >= 0) if (fsensor_oq_yd_max < pat9125_y) fsensor_oq_yd_max = (fsensor_oq_yd_max + pat9125_y) / 2; + } + fsensor_oq_samples++; + fsensor_oq_st_sum += st_cnt; + if (pat9125_y > 0) fsensor_oq_yd_sum += pat9125_y; + if (fsensor_err_cnt > old_err_cnt) + fsensor_oq_er_sum += (fsensor_err_cnt - old_err_cnt); + if (fsensor_oq_er_max < fsensor_err_cnt) + fsensor_oq_er_max = fsensor_err_cnt; + fsensor_oq_sh_sum += pat9125_s; + } + } } #ifdef DEBUG_FSENSOR_LOG @@ -534,9 +534,7 @@ ISR(FSENSOR_INT_PIN_VECT) } #endif //DEBUG_FSENSOR_LOG - fsensor_dy_old = pat9125_y; pat9125_y = 0; - _lock = false; return; } diff --git a/Firmware/stepper.cpp b/Firmware/stepper.cpp index 93e4ad8fa..6e0937cfd 100644 --- a/Firmware/stepper.cpp +++ b/Firmware/stepper.cpp @@ -422,7 +422,6 @@ FORCE_INLINE void stepper_next_block() count_direction[E_AXIS] = 1; } #if defined(FILAMENT_SENSOR) && defined(PAT9125) - fsensor_counter = 0; fsensor_st_block_begin(count_direction[E_AXIS] < 0); #endif //FILAMENT_SENSOR } From 1eddc40ed46cb74192e792ba2326e9ac174aa157 Mon Sep 17 00:00:00 2001 From: Yuri D'Elia Date: Wed, 5 Feb 2020 22:05:04 +0100 Subject: [PATCH 08/13] Comment existing code --- Firmware/fsensor.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Firmware/fsensor.cpp b/Firmware/fsensor.cpp index ea171906f..2cf336a6e 100755 --- a/Firmware/fsensor.cpp +++ b/Firmware/fsensor.cpp @@ -460,12 +460,17 @@ ISR(FSENSOR_INT_PIN_VECT) if (mmu_enabled || ir_sensor_detected) return; if (!((fsensor_int_pin_old ^ FSENSOR_INT_PIN_PIN_REG) & FSENSOR_INT_PIN_MASK)) return; fsensor_int_pin_old = FSENSOR_INT_PIN_PIN_REG; + + // prevent isr re-entry static bool _lock = false; if (_lock) return; _lock = true; + + // fetch fsensor_st_cnt atomically int st_cnt = fsensor_st_cnt; fsensor_st_cnt = 0; sei(); + uint8_t old_err_cnt = fsensor_err_cnt; uint8_t pat9125_res = fsensor_oq_meassure?pat9125_update():pat9125_update_y(); if (!pat9125_res) From 3af35844e11c95147befa708c3f75f7546ef90dd Mon Sep 17 00:00:00 2001 From: Yuri D'Elia Date: Wed, 5 Feb 2020 22:05:24 +0100 Subject: [PATCH 09/13] Remove redundant check (always trigger the fsensor isr) Since fsensor_st_block_chunk is already called on fsensor_chunk_size boundaries, there's no need to check a second time. --- Firmware/fsensor.cpp | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/Firmware/fsensor.cpp b/Firmware/fsensor.cpp index 2cf336a6e..41d9a5065 100755 --- a/Firmware/fsensor.cpp +++ b/Firmware/fsensor.cpp @@ -563,12 +563,10 @@ void fsensor_st_block_chunk(int cnt) { if (!fsensor_enabled) return; fsensor_st_cnt += cnt; - if (abs(fsensor_st_cnt) >= fsensor_chunk_len) - { -// !!! bit toggling (PINxn <- 1) (for PinChangeInterrupt) does not work for some MCU pins - if (PIN_GET(FSENSOR_INT_PIN)) {PIN_VAL(FSENSOR_INT_PIN, LOW);} - else {PIN_VAL(FSENSOR_INT_PIN, HIGH);} - } + + // !!! bit toggling (PINxn <- 1) (for PinChangeInterrupt) does not work for some MCU pins + if (PIN_GET(FSENSOR_INT_PIN)) {PIN_VAL(FSENSOR_INT_PIN, LOW);} + else {PIN_VAL(FSENSOR_INT_PIN, HIGH);} } #endif //PAT9125 From cdc17e8da1b88a6fd10fec6e7a875f3e5caf30c5 Mon Sep 17 00:00:00 2001 From: Yuri D'Elia Date: Wed, 5 Feb 2020 22:06:56 +0100 Subject: [PATCH 10/13] Read all 12 bits correctly from PAT9125 DELTA_* registers Widen ucXL/ucYL/ucXYH types to 16 bits, since the following shifts will otherwise truncate the results. --- Firmware/pat9125.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Firmware/pat9125.c b/Firmware/pat9125.c index 7b7353dde..5b17345c2 100644 --- a/Firmware/pat9125.c +++ b/Firmware/pat9125.c @@ -183,9 +183,9 @@ uint8_t pat9125_update(void) if (pat9125_PID1 == 0xff) return 0; if (ucMotion & 0x80) { - uint8_t ucXL = pat9125_rd_reg(PAT9125_DELTA_XL); - uint8_t ucYL = pat9125_rd_reg(PAT9125_DELTA_YL); - uint8_t ucXYH = pat9125_rd_reg(PAT9125_DELTA_XYH); + uint16_t ucXL = pat9125_rd_reg(PAT9125_DELTA_XL); + uint16_t ucYL = pat9125_rd_reg(PAT9125_DELTA_YL); + uint16_t ucXYH = pat9125_rd_reg(PAT9125_DELTA_XYH); if (pat9125_PID1 == 0xff) return 0; int16_t iDX = ucXL | ((ucXYH << 4) & 0xf00); int16_t iDY = ucYL | ((ucXYH << 8) & 0xf00); @@ -207,8 +207,8 @@ uint8_t pat9125_update_y(void) if (pat9125_PID1 == 0xff) return 0; if (ucMotion & 0x80) { - uint8_t ucYL = pat9125_rd_reg(PAT9125_DELTA_YL); - uint8_t ucXYH = pat9125_rd_reg(PAT9125_DELTA_XYH); + uint16_t ucYL = pat9125_rd_reg(PAT9125_DELTA_YL); + uint16_t ucXYH = pat9125_rd_reg(PAT9125_DELTA_XYH); if (pat9125_PID1 == 0xff) return 0; int16_t iDY = ucYL | ((ucXYH << 8) & 0xf00); if (iDY & 0x800) iDY -= 4096; From 3be859ece951c6c4bdeb8ed112c7e14a439636d6 Mon Sep 17 00:00:00 2001 From: Yuri D'Elia Date: Thu, 6 Feb 2020 13:48:23 +0100 Subject: [PATCH 11/13] FS: Improve reliability on speeds with poor optical tracking Depending on the filament surface and moving speed, the PAT9125 sensor can stop being able to track movement. In such cases, instead of triggering false errors and/or relying on previous states, read and use the exposure data off the sensor and increase error counts only for poorly exposed images instead, which is a good indicator of a far-away (or missing!) tracking surface. --- Firmware/fsensor.cpp | 23 +++++++++++++++++++---- Firmware/pat9125.c | 14 ++++---------- Firmware/pat9125.h | 6 +++--- 3 files changed, 26 insertions(+), 17 deletions(-) diff --git a/Firmware/fsensor.cpp b/Firmware/fsensor.cpp index 41d9a5065..9e7b2f6cf 100755 --- a/Firmware/fsensor.cpp +++ b/Firmware/fsensor.cpp @@ -488,10 +488,25 @@ ISR(FSENSOR_INT_PIN_VECT) if (pat9125_y == 0) { - // no movement detected: increase error count only when extruding, since fast retracts - // cannot always be seen. We also need to ensure that no runout is generated while - // retracting as it's not currently handled everywhere - if (st_dir) ++fsensor_err_cnt; + if (st_dir) + { + // no movement detected: we might be within a blind sensor range, + // update the frame and shutter parameters we didn't earlier + if (!fsensor_oq_meassure) + pat9125_update_bs(); + + // increment the error count only if underexposed: filament likely missing + if ((pat9125_b < 80) && (pat9125_s > 10)) + { + // check for a dark frame (<30% avg brightness) with long exposure + ++fsensor_err_cnt; + } + else + { + // good frame, filament likely present + if(fsensor_err_cnt) --fsensor_err_cnt; + } + } } else if (pat9125_dir != st_dir) { diff --git a/Firmware/pat9125.c b/Firmware/pat9125.c index 5b17345c2..ae291c217 100644 --- a/Firmware/pat9125.c +++ b/Firmware/pat9125.c @@ -219,19 +219,13 @@ uint8_t pat9125_update_y(void) return 0; } -uint8_t pat9125_update_y2(void) +uint8_t pat9125_update_bs(void) { if ((pat9125_PID1 == 0x31) && (pat9125_PID2 == 0x91)) { - uint8_t ucMotion = pat9125_rd_reg(PAT9125_MOTION); - if (pat9125_PID1 == 0xff) return 0; //NOACK error - if (ucMotion & 0x80) - { - int8_t dy = pat9125_rd_reg(PAT9125_DELTA_YL); - if (pat9125_PID1 == 0xff) return 0; //NOACK error - pat9125_y -= dy; //negative number, because direction switching does not work - } - return 1; + pat9125_b = pat9125_rd_reg(PAT9125_FRAME); + pat9125_s = pat9125_rd_reg(PAT9125_SHUTTER); + if (pat9125_PID1 == 0xff) return 0; } return 0; } diff --git a/Firmware/pat9125.h b/Firmware/pat9125.h index 6d36a82e8..12f7fe949 100755 --- a/Firmware/pat9125.h +++ b/Firmware/pat9125.h @@ -19,9 +19,9 @@ extern uint8_t pat9125_b; extern uint8_t pat9125_s; extern uint8_t pat9125_init(void); -extern uint8_t pat9125_update(void); -extern uint8_t pat9125_update_y(void); -extern uint8_t pat9125_update_y2(void); +extern uint8_t pat9125_update(void); // update all sensor data +extern uint8_t pat9125_update_y(void); // update _y only +extern uint8_t pat9125_update_bs(void); // update _b/_s only #if defined(__cplusplus) From bd0544fe9e028ffb32d3696533340dc0319152b4 Mon Sep 17 00:00:00 2001 From: Yuri D'Elia Date: Thu, 6 Feb 2020 14:44:43 +0100 Subject: [PATCH 12/13] FS: Use two different speeds when checking for runout When doing a PAT9125 "soft check", use two different speeds between retraction and extrusion. This increases the chances that we can track the surface. --- Firmware/fsensor.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Firmware/fsensor.cpp b/Firmware/fsensor.cpp index 9e7b2f6cf..7f291d7a2 100755 --- a/Firmware/fsensor.cpp +++ b/Firmware/fsensor.cpp @@ -624,7 +624,7 @@ void fsensor_update(void) fsensor_oq_meassure_start(0); float e_tmp = current_position[E_AXIS]; current_position[E_AXIS] -= 3; - plan_buffer_line_curposXYZE(200/60, active_extruder); + plan_buffer_line_curposXYZE(250/60, active_extruder); current_position[E_AXIS] = e_tmp; plan_buffer_line_curposXYZE(200/60, active_extruder); st_synchronize(); From 853bb79b6b4ac485537fd410f432229a9c45817f Mon Sep 17 00:00:00 2001 From: Yuri D'Elia Date: Thu, 6 Feb 2020 16:39:08 +0100 Subject: [PATCH 13/13] Return correct status from pat9125_update_bs --- Firmware/pat9125.c | 1 + 1 file changed, 1 insertion(+) diff --git a/Firmware/pat9125.c b/Firmware/pat9125.c index ae291c217..de4693d29 100644 --- a/Firmware/pat9125.c +++ b/Firmware/pat9125.c @@ -226,6 +226,7 @@ uint8_t pat9125_update_bs(void) pat9125_b = pat9125_rd_reg(PAT9125_FRAME); pat9125_s = pat9125_rd_reg(PAT9125_SHUTTER); if (pat9125_PID1 == 0xff) return 0; + return 1; } return 0; }