Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix BLTouch PWM reliability in HAL/STM32 #18702

Merged
merged 2 commits into from
Jul 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Marlin/src/HAL/STM32/HAL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ void HAL_init() {
while (!LL_PWR_IsActiveFlag_BRR()); // Wait until backup regulator is initialized
#endif

SetSoftwareSerialTimerInterruptPriority();
SetTimerInterruptPriorities();

TERN_(EMERGENCY_PARSER, USB_Hook_init());
}
Expand Down
24 changes: 22 additions & 2 deletions Marlin/src/HAL/STM32/Servo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,18 @@ 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.");

// Initialize to the default timer priority. This will be overridden by a call from timers.cpp.
// This allows all timer interrupt priorities to be managed from a single location in the HAL.
static uint32_t servo_interrupt_priority = NVIC_EncodePriority(NVIC_GetPriorityGrouping(), TIM_IRQ_PRIO, TIM_IRQ_SUBPRIO);

// This must be called after the STM32 Servo class has intialized the timer.
// It may only be needed after the first call to attach(), but it is possible
// that is is necessary after every detach() call. To be safe this is currently
// called after every call to attach().
static void fixServoTimerInterruptPriority() {
NVIC_SetPriority(getTimerUpIrq(TIMER_SERVO), servo_interrupt_priority);
}

libServo::libServo()
: delay(servoDelay[servoCount]),
was_attached_before_pause(false),
Expand All @@ -44,13 +56,17 @@ libServo::libServo()
int8_t libServo::attach(const int pin) {
if (servoCount >= MAX_SERVOS) return -1;
if (pin > 0) servo_pin = pin;
return stm32_servo.attach(servo_pin);
auto result = stm32_servo.attach(servo_pin);
fixServoTimerInterruptPriority();
return result;
}

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 stm32_servo.attach(servo_pin, min, max);
auto result = stm32_servo.attach(servo_pin, min, max);
fixServoTimerInterruptPriority();
return result;
}

void libServo::move(const int value) {
Expand Down Expand Up @@ -86,5 +102,9 @@ void libServo::resume_all_servos() {
if (servo) servo->resume();
}

void libServo::setInterruptPriority(uint32_t preemptPriority, uint32_t subPriority) {
servo_interrupt_priority = NVIC_EncodePriority(NVIC_GetPriorityGrouping(), preemptPriority, subPriority);
}

#endif // HAS_SERVOS
#endif // ARDUINO_ARCH_STM32 && !STM32GENERIC
1 change: 1 addition & 0 deletions Marlin/src/HAL/STM32/Servo.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ class libServo {

static void pause_all_servos();
static void resume_all_servos();
static void setInterruptPriority(uint32_t preemptPriority, uint32_t subPriority);

private:
Servo stm32_servo;
Expand Down
44 changes: 30 additions & 14 deletions Marlin/src/HAL/STM32/timers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,26 +29,41 @@

#define NUM_HARDWARE_TIMERS 2

// Default timer priorities. Override by specifying alternate priorities in the board pins file.
// The TONE timer is not present here, as it currently cannot be set programmatically. It is set
// by defining TIM_IRQ_PRIO in the variant.h or platformio.ini file, which adjusts the default
// priority for STM32 HardwareTimer objects.
#define SWSERIAL_TIMER_IRQ_PRIO_DEFAULT 1 // Requires tight bit timing to communicate reliably with TMC drivers
#define SERVO_TIMER_IRQ_PRIO_DEFAULT 1 // Requires tight PWM timing to control a BLTouch reliably
#define STEP_TIMER_IRQ_PRIO_DEFAULT 2
sjasonsmith marked this conversation as resolved.
Show resolved Hide resolved
#define TEMP_TIMER_IRQ_PRIO_DEFAULT 14 // Low priority avoids interference with other hardware and timers

#ifndef STEP_TIMER_IRQ_PRIO
#define STEP_TIMER_IRQ_PRIO 2
#define STEP_TIMER_IRQ_PRIO STEP_TIMER_IRQ_PRIO_DEFAULT
#endif
#ifndef TEMP_TIMER_IRQ_PRIO
#define TEMP_TIMER_IRQ_PRIO 14 // 14 = after hardware ISRs
#endif

// Ensure the default timer priority is somewhere between the STEP and TEMP priorities.
// The STM32 framework defaults to interrupt 14 for all timers. This should be increased so that
// timing-sensitive operations such as speaker output are note impacted by the long-running
// temperature ISR. This must be defined in the platformio.ini file or the board's variant.h,
// so that it will be consumed by framework code.
#if !(TIM_IRQ_PRIO > STEP_TIMER_IRQ_PRIO && TIM_IRQ_PRIO < TEMP_TIMER_IRQ_PRIO)
#error "Default timer interrupt priority is unspecified or set to a value which may degrade performance."
#define TEMP_TIMER_IRQ_PRIO TEMP_TIMER_IRQ_PRIO_DEFAULT
#endif

#if HAS_TMC_SW_SERIAL
#include <SoftwareSerial.h>
#ifndef SWSERIAL_TIMER_IRQ_PRIO
#define SWSERIAL_TIMER_IRQ_PRIO 1
#define SWSERIAL_TIMER_IRQ_PRIO SWSERIAL_TIMER_IRQ_PRIO_DEFAULT
#endif
#endif
#if HAS_SERVOS
#include "Servo.h"
#ifndef SERVO_TIMER_IRQ_PRIO
#define SERVO_TIMER_IRQ_PRIO SERVO_TIMER_IRQ_PRIO_DEFAULT
#endif
#endif
#if ENABLED(SPEAKER)
// Ensure the default timer priority is somewhere between the STEP and TEMP priorities.
// The STM32 framework defaults to interrupt 14 for all timers. This should be increased so that
// timing-sensitive operations such as speaker output are not impacted by the long-running
// temperature ISR. This must be defined in the platformio.ini file or the board's variant.h,
// so that it will be consumed by framework code.
#if !(TIM_IRQ_PRIO > STEP_TIMER_IRQ_PRIO && TIM_IRQ_PRIO < TEMP_TIMER_IRQ_PRIO)
#error "Default timer interrupt priority is unspecified or set to a value which may degrade performance."
#endif
#endif

Expand Down Expand Up @@ -189,8 +204,9 @@ TIM_TypeDef * HAL_timer_device(const uint8_t timer_num) {
return nullptr;
}

void SetSoftwareSerialTimerInterruptPriority() {
void SetTimerInterruptPriorities() {
TERN_(HAS_TMC_SW_SERIAL, SoftwareSerial::setInterruptPriority(SWSERIAL_TIMER_IRQ_PRIO, 0));
TERN_(HAS_SERVOS, libServo::setInterruptPriority(SERVO_TIMER_IRQ_PRIO, 0));
}

#endif // ARDUINO_ARCH_STM32 && !STM32GENERIC
3 changes: 2 additions & 1 deletion Marlin/src/HAL/STM32/timers.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,9 @@ void HAL_timer_enable_interrupt(const uint8_t timer_num);
void HAL_timer_disable_interrupt(const uint8_t timer_num);
bool HAL_timer_interrupt_enabled(const uint8_t timer_num);

// Configure timer priorities for peripherals such as Software Serial or Servos.
// Exposed here to allow all timer priority information to reside in timers.cpp
void SetSoftwareSerialTimerInterruptPriority();
void SetTimerInterruptPriorities();

//TIM_TypeDef* HAL_timer_device(const uint8_t timer_num); no need to be public for now. not public = not used externally

Expand Down