From 5814b316ffc08bb46d1d526b08233ee401ceb900 Mon Sep 17 00:00:00 2001 From: Jason Smith Date: Sun, 10 May 2020 01:03:18 -0700 Subject: [PATCH 1/7] Clear STM32F4 flash error bits prior to writing to flash --- Marlin/src/HAL/STM32/eeprom_flash.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Marlin/src/HAL/STM32/eeprom_flash.cpp b/Marlin/src/HAL/STM32/eeprom_flash.cpp index 49862957e8a3..f91db2e1b2d3 100644 --- a/Marlin/src/HAL/STM32/eeprom_flash.cpp +++ b/Marlin/src/HAL/STM32/eeprom_flash.cpp @@ -139,6 +139,11 @@ bool PersistentStore::access_start() { bool PersistentStore::access_finish() { if (eeprom_data_written) { + #ifdef STM32F4xx + // MCU may come up with flash error bits which prevent some flash operations. + // Clear flags prior to flash operations to prevent errors. + __HAL_FLASH_CLEAR_FLAG(FLASH_FLAG_OPERR | FLASH_FLAG_WRPERR | FLASH_FLAG_PGAERR | FLASH_FLAG_PGPERR | FLASH_FLAG_PGSERR); + #endif #if ENABLED(FLASH_EEPROM_LEVELING) From 78caeac5e78751ea59aa50e7520f67c96804bf13 Mon Sep 17 00:00:00 2001 From: Jason Smith Date: Sun, 10 May 2020 11:02:48 -0700 Subject: [PATCH 2/7] Add STM32F4 PRINTCOUNTER sanity check FLASH_EEPROM_EMULATION takes about a second to erase the flash page on STM32F4 hardware, which will interfere with any ongoing activities. Disallow PRINTCOUNTER and FLASH_EEPROM_EMULATION to co-exist for STM32F4 hardware using HAL/STM32, to avoid issues during prints. --- Marlin/src/HAL/STM32/inc/SanityCheck.h | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Marlin/src/HAL/STM32/inc/SanityCheck.h b/Marlin/src/HAL/STM32/inc/SanityCheck.h index 9cd8db81f426..aad173df441d 100644 --- a/Marlin/src/HAL/STM32/inc/SanityCheck.h +++ b/Marlin/src/HAL/STM32/inc/SanityCheck.h @@ -43,3 +43,8 @@ #endif #error "SDCARD_EEPROM_EMULATION requires SDSUPPORT. Enable SDSUPPORT or choose another EEPROM emulation." #endif + +#if defined(STM32F4xx) && BOTH(PRINTCOUNTER, FLASH_EEPROM_EMULATION) + #warning "FLASH_EEPROM_EMULATION may cause long delays when writing and should not be used while printing." + #error "Disable PRINTCOUNTER or choose another EEPROM emulation." +#endif From 49ea203f8f1ae53c07b6a0abcfe8e9cc37b78b09 Mon Sep 17 00:00:00 2001 From: Jason Smith Date: Sun, 10 May 2020 12:02:28 -0700 Subject: [PATCH 3/7] Allow pausing and resuming on libServo class in HAL/STM32 --- Marlin/src/HAL/STM32/Servo.cpp | 44 ++++++++++++++++++++++++++++++---- Marlin/src/HAL/STM32/Servo.h | 18 +++++++++++--- 2 files changed, 54 insertions(+), 8 deletions(-) diff --git a/Marlin/src/HAL/STM32/Servo.cpp b/Marlin/src/HAL/STM32/Servo.cpp index d192da5d28dd..918f58cad390 100644 --- a/Marlin/src/HAL/STM32/Servo.cpp +++ b/Marlin/src/HAL/STM32/Servo.cpp @@ -29,32 +29,66 @@ #include "Servo.h" static uint_fast8_t servoCount = 0; +static libServo *servos[NUM_SERVOS] = {0}; constexpr millis_t servoDelay[] = SERVO_DELAY; static_assert(COUNT(servoDelay) == NUM_SERVOS, "SERVO_DELAY must be an array NUM_SERVOS long."); libServo::libServo() -: delay(servoDelay[servoCount++]) -{} +: delay(servoDelay[servoCount]), + was_attached_before_pause(false), + value_before_pause(0) +{ + servos[servoCount++] = this; +} int8_t libServo::attach(const int pin) { if (servoCount >= MAX_SERVOS) return -1; if (pin > 0) servo_pin = pin; - return super::attach(servo_pin); + return stm32_servo.attach(servo_pin); } int8_t libServo::attach(const int pin, const int min, const int max) { if (servoCount >= MAX_SERVOS) return -1; if (pin > 0) servo_pin = pin; - return super::attach(servo_pin, min, max); + return stm32_servo.attach(servo_pin, min, max); } void libServo::move(const int value) { if (attach(0) >= 0) { - write(value); + stm32_servo.write(value); safe_delay(delay); TERN_(DEACTIVATE_SERVOS_AFTER_MOVE, detach()); } } + +void libServo::pause() +{ + was_attached_before_pause = stm32_servo.attached(); + if (was_attached_before_pause) { + value_before_pause = stm32_servo.read(); + stm32_servo.detach(); + } +} + +void libServo::resume() +{ + if (was_attached_before_pause) { + attach(); + move(value_before_pause); + } +} + +void libServo::pause_all_servos() +{ + for (auto& servo : servos) + if (servo) servo->pause(); +} + +void libServo::resume_all_servos() +{ + for (auto& servo : servos) + if (servo) servo->resume(); +} #endif // HAS_SERVOS #endif // ARDUINO_ARCH_STM32 && !STM32GENERIC diff --git a/Marlin/src/HAL/STM32/Servo.h b/Marlin/src/HAL/STM32/Servo.h index e8b3c4b100b4..50ae1a9b946e 100644 --- a/Marlin/src/HAL/STM32/Servo.h +++ b/Marlin/src/HAL/STM32/Servo.h @@ -27,15 +27,27 @@ #include "../../core/millis_t.h" // Inherit and expand on the official library -class libServo : public Servo { +class libServo { public: libServo(); - int8_t attach(const int pin); + int8_t attach(const int pin = 0); // pin == 0 uses value from previous call int8_t attach(const int pin, const int min, const int max); + void detach() { stm32_servo.detach(); } + int read() { return stm32_servo.read(); } void move(const int value); + + void pause(); + void resume(); + + static void pause_all_servos(); + static void resume_all_servos(); + private: - typedef Servo super; + Servo stm32_servo; int servo_pin = 0; millis_t delay = 0; + + bool was_attached_before_pause; + int value_before_pause; }; From 49ce0fba57d1459ec59f87745c3d4222ea44f600 Mon Sep 17 00:00:00 2001 From: Jason Smith Date: Sun, 10 May 2020 11:25:07 -0700 Subject: [PATCH 4/7] Pause servos and disable interrupts while erasing flash --- Marlin/src/HAL/STM32/eeprom_flash.cpp | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/Marlin/src/HAL/STM32/eeprom_flash.cpp b/Marlin/src/HAL/STM32/eeprom_flash.cpp index f91db2e1b2d3..8da62978e8d5 100644 --- a/Marlin/src/HAL/STM32/eeprom_flash.cpp +++ b/Marlin/src/HAL/STM32/eeprom_flash.cpp @@ -27,6 +27,7 @@ #if ENABLED(FLASH_EEPROM_EMULATION) #include "../shared/eeprom_api.h" +#include "Servo.h" // Only STM32F4 can support wear leveling at this time @@ -164,7 +165,11 @@ bool PersistentStore::access_finish() { current_slot = EEPROM_SLOTS - 1; UNLOCK_FLASH(); + libServo::pause_all_servos(); + DISABLE_ISRS(); status = HAL_FLASHEx_Erase(&EraseInitStruct, &SectorError); + ENABLE_ISRS(); + libServo::resume_all_servos(); if (status != HAL_OK) { DEBUG_ECHOLNPAIR("HAL_FLASHEx_Erase=", status); DEBUG_ECHOLNPAIR("GetError=", HAL_FLASH_GetError()); @@ -209,7 +214,18 @@ bool PersistentStore::access_finish() { return success; #else + // The following was written for the STM32F4 but may work with other MCUs as well. + // Most STM32F4 flash does not allow reading from flash during erase operations. + // This takes about a second on a STM32F407 with a 128kB sector used as EEPROM. + // Interrupts during this time can have unpredictable results, such as killing Servo + // output. Servo output still glitches with interrupts disabled, but recovers after the + // erase. + libServo::pause_all_servos(); + DISABLE_ISRS(); eeprom_buffer_flush(); + ENABLE_ISRS(); + libServo::resume_all_servos(); + eeprom_data_written = false; #endif } From 8810dcfd037699d0535a05cbcc7bec494571f8dc Mon Sep 17 00:00:00 2001 From: Jason Smith Date: Sun, 10 May 2020 12:25:44 -0700 Subject: [PATCH 5/7] Change silent undef of FLASH_EEPROM_LEVELING to sanity check --- Marlin/src/HAL/STM32/eeprom_flash.cpp | 6 ------ Marlin/src/HAL/STM32/inc/SanityCheck.h | 4 ++++ 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/Marlin/src/HAL/STM32/eeprom_flash.cpp b/Marlin/src/HAL/STM32/eeprom_flash.cpp index 8da62978e8d5..d9fa42f96346 100644 --- a/Marlin/src/HAL/STM32/eeprom_flash.cpp +++ b/Marlin/src/HAL/STM32/eeprom_flash.cpp @@ -29,12 +29,6 @@ #include "../shared/eeprom_api.h" #include "Servo.h" - -// Only STM32F4 can support wear leveling at this time -#ifndef STM32F4xx - #undef FLASH_EEPROM_LEVELING -#endif - /** * The STM32 HAL supports chips that deal with "pages" and some with "sectors" and some that * even have multiple "banks" of flash. diff --git a/Marlin/src/HAL/STM32/inc/SanityCheck.h b/Marlin/src/HAL/STM32/inc/SanityCheck.h index aad173df441d..7734fc0e83c2 100644 --- a/Marlin/src/HAL/STM32/inc/SanityCheck.h +++ b/Marlin/src/HAL/STM32/inc/SanityCheck.h @@ -48,3 +48,7 @@ #warning "FLASH_EEPROM_EMULATION may cause long delays when writing and should not be used while printing." #error "Disable PRINTCOUNTER or choose another EEPROM emulation." #endif + +#if !defined(STM32F4xx) && ENABLED(FLASH_EEPROM_LEVELING) + #error "FLASH_EEPROM_LEVELING is currently only supported on STM32F4 hardware." +#endif From 1d14d43723c3aa6d1215d0d383ff09a423d7017c Mon Sep 17 00:00:00 2001 From: Jason Smith Date: Sun, 10 May 2020 13:39:26 -0700 Subject: [PATCH 6/7] Bypass servo pause/resume if !HAS_SERVOS --- Marlin/src/HAL/STM32/eeprom_flash.cpp | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/Marlin/src/HAL/STM32/eeprom_flash.cpp b/Marlin/src/HAL/STM32/eeprom_flash.cpp index d9fa42f96346..309c5eea9f86 100644 --- a/Marlin/src/HAL/STM32/eeprom_flash.cpp +++ b/Marlin/src/HAL/STM32/eeprom_flash.cpp @@ -27,7 +27,15 @@ #if ENABLED(FLASH_EEPROM_EMULATION) #include "../shared/eeprom_api.h" -#include "Servo.h" + +#if HAS_SERVOS + #include "Servo.h" + #define PAUSE_SERVO_OUTPUT() libServo::pause_all_servos() + #define RESUME_SERVO_OUTPUT() libServo::resume_all_servos() +#else + #define PAUSE_SERVO_OUTPUT() + #define RESUME_SERVO_OUTPUT() +#endif /** * The STM32 HAL supports chips that deal with "pages" and some with "sectors" and some that @@ -159,11 +167,11 @@ bool PersistentStore::access_finish() { current_slot = EEPROM_SLOTS - 1; UNLOCK_FLASH(); - libServo::pause_all_servos(); + PAUSE_SERVO_OUTPUT(); DISABLE_ISRS(); status = HAL_FLASHEx_Erase(&EraseInitStruct, &SectorError); ENABLE_ISRS(); - libServo::resume_all_servos(); + RESUME_SERVO_OUTPUT(); if (status != HAL_OK) { DEBUG_ECHOLNPAIR("HAL_FLASHEx_Erase=", status); DEBUG_ECHOLNPAIR("GetError=", HAL_FLASH_GetError()); @@ -214,11 +222,11 @@ bool PersistentStore::access_finish() { // Interrupts during this time can have unpredictable results, such as killing Servo // output. Servo output still glitches with interrupts disabled, but recovers after the // erase. - libServo::pause_all_servos(); + PAUSE_SERVO_OUTPUT(); DISABLE_ISRS(); eeprom_buffer_flush(); ENABLE_ISRS(); - libServo::resume_all_servos(); + RESUME_SERVO_OUTPUT(); eeprom_data_written = false; #endif From 6016608c1a4ceae830e8f9a61e204dbc1d2498ff Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Sun, 10 May 2020 21:30:26 -0500 Subject: [PATCH 7/7] Update Servo.cpp --- Marlin/src/HAL/STM32/Servo.cpp | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/Marlin/src/HAL/STM32/Servo.cpp b/Marlin/src/HAL/STM32/Servo.cpp index 918f58cad390..5fb8e3cd6ae7 100644 --- a/Marlin/src/HAL/STM32/Servo.cpp +++ b/Marlin/src/HAL/STM32/Servo.cpp @@ -61,8 +61,7 @@ void libServo::move(const int value) { } } -void libServo::pause() -{ +void libServo::pause() { was_attached_before_pause = stm32_servo.attached(); if (was_attached_before_pause) { value_before_pause = stm32_servo.read(); @@ -70,25 +69,22 @@ void libServo::pause() } } -void libServo::resume() -{ +void libServo::resume() { if (was_attached_before_pause) { attach(); move(value_before_pause); } } -void libServo::pause_all_servos() -{ +void libServo::pause_all_servos() { for (auto& servo : servos) if (servo) servo->pause(); } -void libServo::resume_all_servos() -{ +void libServo::resume_all_servos() { for (auto& servo : servos) if (servo) servo->resume(); } -#endif // HAS_SERVOS +#endif // HAS_SERVOS #endif // ARDUINO_ARCH_STM32 && !STM32GENERIC