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 AVR set_pwm_duty unset default frequency #23463

Merged
merged 33 commits into from
Jan 12, 2022
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
15b243c
Fix AVR set_pwm_duty unset default frequency
descipher Jan 6, 2022
e363120
Set MOTOR_CURRENT_PWM_FREQUENCY default as 31400
descipher Jan 6, 2022
0ab6dcb
Merge 'bugfix-2.0.x' into bf2_weird_pwm_PR_23463
thinkyhead Jan 7, 2022
df4f122
defer to pinsformat.js
thinkyhead Jan 7, 2022
359415d
delete red highlighted trailing spaces
thinkyhead Jan 7, 2022
fe1f75d
clarify
thinkyhead Jan 7, 2022
eaef837
trust pins to retain freq after init ?
thinkyhead Jan 7, 2022
8cd296f
track init as a debug option
thinkyhead Jan 7, 2022
ffd9750
arg tweak, keep wrapper ?
thinkyhead Jan 7, 2022
dbd37f2
set_pwm_frequency with all HAL_CAN_SET_PWM_FREQ
thinkyhead Jan 7, 2022
8e10265
Add HAS_LCD_BRIGHTNESS Needs PWM condition check
descipher Jan 7, 2022
1953124
Corrected include and frequency init tracking
descipher Jan 7, 2022
ea74cdc
Cleanup condition check state
descipher Jan 8, 2022
8b334fd
Update fast_pwm.cpp
thinkyhead Jan 9, 2022
741078e
would other platforms ever use it?
thinkyhead Jan 9, 2022
e18008f
tweak wgm
thinkyhead Jan 9, 2022
aaa15d6
Init AVR timers 2,3,4,5 in HAL
descipher Jan 10, 2022
5b5b23b
Merge branch 'ultimate.pwm.fix' of https://github.com/descipher/Marli…
descipher Jan 10, 2022
4b01add
Add AT90 pins
descipher Jan 10, 2022
c1e1b75
kHz => KHz
thinkyhead Jan 10, 2022
e177429
unsigned PWM frequency
thinkyhead Jan 10, 2022
7ffc23a
32 bits may be better
thinkyhead Jan 10, 2022
092666e
fix formatting
thinkyhead Jan 10, 2022
1755e78
suppress warning
thinkyhead Jan 10, 2022
14795aa
Add more pins
descipher Jan 10, 2022
f95f606
unsigned enums
thinkyhead Jan 11, 2022
54501eb
move macros, tweak get_pwm_timer
thinkyhead Jan 11, 2022
78cb32f
tweak var names and maths
thinkyhead Jan 11, 2022
5aeac05
Merge branch 'bugfix-2.0.x' into pr/23463
thinkyhead Jan 11, 2022
90ba936
Timer check default return as non-PWM and not protected.
descipher Jan 11, 2022
0871ea3
return timer structs more directly
thinkyhead Jan 11, 2022
0f5fbe7
fallback to WGM2_PWM_PC
thinkyhead Jan 11, 2022
6a5eaec
style tweaks
thinkyhead Jan 11, 2022
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
3 changes: 2 additions & 1 deletion Marlin/src/HAL/AVR/HAL.h
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@ inline void HAL_adc_init() {
#define strtof strtod

#define HAL_CAN_SET_PWM_FREQ // This HAL supports PWM Frequency adjustment
#define PWM_FREQUENCY 1000 // Default PWM frequency when set_pwm_duty() is called without set_pwm_frequency()

/**
* set_pwm_frequency
Expand All @@ -217,7 +218,7 @@ inline void HAL_adc_init() {
* NOTE that the frequency is applied to all pins on the timer (Ex OC3A, OC3B and OC3B)
* NOTE that there are limitations, particularly if using TIMER2. (see Configuration_adv.h -> FAST FAN PWM Settings)
*/
void set_pwm_frequency(const pin_t pin, int f_desired);
void set_pwm_frequency(const pin_t pin, const int f_desired);

/**
* set_pwm_duty
Expand Down
20 changes: 12 additions & 8 deletions Marlin/src/HAL/AVR/fast_pwm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@
*/
#ifdef __AVR__

#include "../../inc/MarlinConfigPre.h"
#include "../../inc/MarlinConfig.h"
#include "HAL.h"

#if NEEDS_HARDWARE_PWM // Specific meta-flag for features that mandate PWM
static uint16_t timer_freq[5];

struct Timer {
volatile uint8_t* TCCRnQ[3]; // max 3 TCCR registers per timer
Expand Down Expand Up @@ -150,11 +150,13 @@ Timer get_pwm_timer(const pin_t pin) {
return timer;
}

void set_pwm_frequency(const pin_t pin, int f_desired) {
void set_pwm_frequency(const pin_t pin, const int f_desired) {
Timer timer = get_pwm_timer(pin);
if (timer.n == 0) return; // Don't proceed if protected timer or not recognized
uint16_t size;
if (timer.n == 2) size = 255; else size = 65535;

timer_freq[timer.n - 1] = f_desired;

const uint16_t size = (timer.n == 2) ? 255 : 65535;

uint16_t res = 255; // resolution (TOP value)
uint8_t j = 0; // prescaler index
Expand Down Expand Up @@ -228,8 +230,6 @@ void set_pwm_frequency(const pin_t pin, int f_desired) {
_SET_ICRn(timer.ICRn, res); // Set ICRn value (TOP) = res
}

#endif // NEEDS_HARDWARE_PWM

void set_pwm_duty(const pin_t pin, const uint16_t v, const uint16_t v_size/*=255*/, const bool invert/*=false*/) {
#if NEEDS_HARDWARE_PWM

Expand All @@ -242,7 +242,11 @@ void set_pwm_duty(const pin_t pin, const uint16_t v, const uint16_t v_size/*=255
else {
Timer timer = get_pwm_timer(pin);
if (timer.n == 0) return; // Don't proceed if protected timer or not recognized
// Set compare output mode to CLEAR -> SET or SET -> CLEAR (if inverted)

if (timer_freq[timer.n - 1] == 0) { // If the timer is unconfigured and no freq is set then default PWM_FREQUENCY
set_pwm_frequency(pin, PWM_FREQUENCY); // Set the frequency and save the value to the assigned index no.
}
thinkyhead marked this conversation as resolved.
Show resolved Hide resolved

_SET_COMnQ(timer.TCCRnQ, timer.q TERN_(HAS_TCCR2, + (timer.q == 2)), COM_CLEAR_SET + invert); // COM20 is on bit 4 of TCCR2, so +1 for q==2
const uint16_t top = timer.n == 2 ? TERN(USE_OCR2A_AS_TOP, *timer.OCRnQ[0], 255) : *timer.ICRn;
_SET_OCRnQ(timer.OCRnQ, timer.q, uint16_t(uint32_t(v) * top / v_size)); // Scale 8/16-bit v to top value
Expand Down
2 changes: 1 addition & 1 deletion Marlin/src/HAL/AVR/inc/SanityCheck.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
#elif NUM_SERVOS > 0 && defined(_useTimer3) && (WITHIN(SPINDLE_LASER_PWM_PIN, 2, 3) || SPINDLE_LASER_PWM_PIN == 5)
#error "Counter/Timer for SPINDLE_LASER_PWM_PIN is used by the servo system."
#endif
#elif defined(SPINDLE_LASER_FREQUENCY)
#elif SPINDLE_LASER_FREQUENCY
#error "SPINDLE_LASER_FREQUENCY requires SPINDLE_LASER_USE_PWM."
#endif

Expand Down
2 changes: 1 addition & 1 deletion Marlin/src/HAL/LPC1768/HAL.h
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ void flashFirmware(const int16_t);
* All Hardware PWM pins run at the same frequency and all
* Software PWM pins run at the same frequency
*/
void set_pwm_frequency(const pin_t pin, int f_desired);
void set_pwm_frequency(const pin_t pin, const int f_desired);

/**
* set_pwm_duty
Expand Down
10 changes: 3 additions & 7 deletions Marlin/src/HAL/LPC1768/fast_pwm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,8 @@ void set_pwm_duty(const pin_t pin, const uint16_t v, const uint16_t v_size/*=255
LPC176x::pwm_write_ratio(pin, invert ? 1.0f - (float)v / v_size : (float)v / v_size); // map 1-254 onto PWM range
}

#if NEEDS_HARDWARE_PWM // Specific meta-flag for features that mandate PWM

void set_pwm_frequency(const pin_t pin, int f_desired) {
LPC176x::pwm_set_frequency(pin, f_desired);
}

#endif
void set_pwm_frequency(const pin_t pin, const int f_desired) {
LPC176x::pwm_set_frequency(pin, f_desired);
}

#endif // TARGET_LPC1768
2 changes: 1 addition & 1 deletion Marlin/src/HAL/STM32/HAL.h
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ extern volatile uint32_t systick_uptime_millis;
* Set the frequency of the timer corresponding to the provided pin
* All Timer PWM pins run at the same frequency
*/
void set_pwm_frequency(const pin_t pin, int f_desired);
void set_pwm_frequency(const pin_t pin, const int f_desired);

/**
* set_pwm_duty
Expand Down
2 changes: 1 addition & 1 deletion Marlin/src/HAL/STM32/fast_pwm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ void set_pwm_duty(const pin_t pin, const uint16_t v, const uint16_t v_size/*=255
if (previousMode != TIMER_OUTPUT_COMPARE_PWM1) HT->resume();
}

void set_pwm_frequency(const pin_t pin, int f_desired) {
void set_pwm_frequency(const pin_t pin, const int f_desired) {
if (!PWM_PIN(pin)) return; // Don't proceed if no hardware timer
const PinName pin_name = digitalPinToPinName(pin);
TIM_TypeDef * const Instance = (TIM_TypeDef *)pinmap_peripheral(pin_name, PinMap_PWM); // Get HAL timer instance
Expand Down
2 changes: 1 addition & 1 deletion Marlin/src/HAL/STM32F1/HAL.h
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ void flashFirmware(const int16_t);
* Set the frequency of the timer corresponding to the provided pin
* All Timer PWM pins run at the same frequency
*/
void set_pwm_frequency(const pin_t pin, int f_desired);
void set_pwm_frequency(const pin_t pin, const int f_desired);

/**
* set_pwm_duty
Expand Down
2 changes: 1 addition & 1 deletion Marlin/src/HAL/STM32F1/fast_pwm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ void set_pwm_duty(const pin_t pin, const uint16_t v, const uint16_t v_size/*=255
timer_set_mode(timer, channel, TIMER_PWM); // PWM Output Mode
}

void set_pwm_frequency(const pin_t pin, int f_desired) {
void set_pwm_frequency(const pin_t pin, const int f_desired) {
if (!PWM_PIN(pin)) return; // Don't proceed if no hardware timer

timer_dev *timer; UNUSED(timer);
Expand Down
8 changes: 3 additions & 5 deletions Marlin/src/feature/spindle_laser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ void SpindleLaser::init() {
SET_PWM(SPINDLE_LASER_PWM_PIN);
set_pwm_duty(pin_t(SPINDLE_LASER_PWM_PIN), SPINDLE_LASER_PWM_OFF); // Set to lowest speed
#endif
#if ENABLED(HAL_CAN_SET_PWM_FREQ) && defined(SPINDLE_LASER_FREQUENCY)
#if ENABLED(HAL_CAN_SET_PWM_FREQ) && SPINDLE_LASER_FREQUENCY
set_pwm_frequency(pin_t(SPINDLE_LASER_PWM_PIN), SPINDLE_LASER_FREQUENCY);
TERN_(MARLIN_DEV_MODE, frequency = SPINDLE_LASER_FREQUENCY);
#endif
Expand All @@ -78,9 +78,7 @@ void SpindleLaser::init() {
#if ENABLED(AIR_ASSIST)
OUT_WRITE(AIR_ASSIST_PIN, !AIR_ASSIST_ACTIVE); // Init Air Assist OFF
#endif
#if ENABLED(I2C_AMMETER)
ammeter.init(); // Init I2C Ammeter
#endif
TERN_(I2C_AMMETER, ammeter.init()); // Init I2C Ammeter
}

#if ENABLED(SPINDLE_LASER_USE_PWM)
Expand All @@ -90,7 +88,7 @@ void SpindleLaser::init() {
* @param ocr Power value
*/
void SpindleLaser::_set_ocr(const uint8_t ocr) {
#if NEEDS_HARDWARE_PWM && SPINDLE_LASER_FREQUENCY
#if ENABLED(HAL_CAN_SET_PWM_FREQ) && SPINDLE_LASER_FREQUENCY
set_pwm_frequency(pin_t(SPINDLE_LASER_PWM_PIN), TERN(MARLIN_DEV_MODE, frequency, SPINDLE_LASER_FREQUENCY));
#endif
set_pwm_duty(pin_t(SPINDLE_LASER_PWM_PIN), ocr ^ SPINDLE_LASER_PWM_OFF);
Expand Down
5 changes: 0 additions & 5 deletions Marlin/src/inc/Conditionals_adv.h
Original file line number Diff line number Diff line change
Expand Up @@ -677,11 +677,6 @@
#define CUTTER_UNIT_IS(V) (_CUTTER_POWER(CUTTER_POWER_UNIT) == _CUTTER_POWER(V))
#endif

// Add features that need hardware PWM here
#if ANY(FAST_PWM_FAN, SPINDLE_LASER_USE_PWM)
#define NEEDS_HARDWARE_PWM 1
#endif

#if !defined(__AVR__) || !defined(USBCON)
// Define constants and variables for buffering serial data.
// Use only 0 or powers of 2 greater than 1
Expand Down
5 changes: 5 additions & 0 deletions Marlin/src/inc/Conditionals_post.h
Original file line number Diff line number Diff line change
Expand Up @@ -2860,6 +2860,11 @@
#define HAS_MOTOR_CURRENT_PWM 1
#endif

// Add features that need hardware PWM here
#if ANY(FAST_PWM_FAN, SPINDLE_LASER_USE_PWM, HAS_MOTOR_CURRENT_PWM, HAS_LCD_BRIGHTNESS)
#define NEEDS_HARDWARE_PWM 1
#endif

thinkyhead marked this conversation as resolved.
Show resolved Hide resolved
#if ANY(HAS_Z_MS_PINS, HAS_Z2_MS_PINS, HAS_Z3_MS_PINS, HAS_Z4_MS_PINS)
#define HAS_SOME_Z_MS_PINS 1
#endif
Expand Down
2 changes: 1 addition & 1 deletion Marlin/src/lcd/menu/menu_spindle_laser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@
ACTION_ITEM(MSG_LASER_FIRE_PULSE, cutter.test_fire_pulse);
#endif

#if BOTH(MARLIN_DEV_MODE, HAL_CAN_SET_PWM_FREQ) && defined(SPINDLE_LASER_FREQUENCY)
#if BOTH(MARLIN_DEV_MODE, HAL_CAN_SET_PWM_FREQ) && SPINDLE_LASER_FREQUENCY
EDIT_ITEM_FAST(CUTTER_MENU_FREQUENCY_TYPE, MSG_CUTTER_FREQUENCY, &cutter.frequency, 2000, 80000, cutter.refresh_frequency);
#endif

Expand Down
41 changes: 22 additions & 19 deletions Marlin/src/module/stepper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3257,33 +3257,33 @@ void Stepper::report_positions() {

#elif HAS_MOTOR_CURRENT_PWM

#define _WRITE_CURRENT_PWM(P) set_pwm_duty(pin_t(MOTOR_CURRENT_PWM_## P ##_PIN), 255L * current / (MOTOR_CURRENT_PWM_RANGE))
#define _WRITE_CURRENT_PWM_DUTY(P) set_pwm_duty(pin_t(MOTOR_CURRENT_PWM_## P ##_PIN), 255L * current / (MOTOR_CURRENT_PWM_RANGE))
switch (driver) {
case 0:
#if PIN_EXISTS(MOTOR_CURRENT_PWM_X)
_WRITE_CURRENT_PWM(X);
_WRITE_CURRENT_PWM_DUTY(X);
#endif
#if PIN_EXISTS(MOTOR_CURRENT_PWM_Y)
_WRITE_CURRENT_PWM(Y);
_WRITE_CURRENT_PWM_DUTY(Y);
#endif
#if PIN_EXISTS(MOTOR_CURRENT_PWM_XY)
_WRITE_CURRENT_PWM(XY);
_WRITE_CURRENT_PWM_DUTY(XY);
#endif
break;
case 1:
#if PIN_EXISTS(MOTOR_CURRENT_PWM_Z)
_WRITE_CURRENT_PWM(Z);
_WRITE_CURRENT_PWM_DUTY(Z);
#endif
break;
case 2:
#if PIN_EXISTS(MOTOR_CURRENT_PWM_E)
_WRITE_CURRENT_PWM(E);
_WRITE_CURRENT_PWM_DUTY(E);
#endif
#if PIN_EXISTS(MOTOR_CURRENT_PWM_E0)
_WRITE_CURRENT_PWM(E0);
_WRITE_CURRENT_PWM_DUTY(E0);
#endif
#if PIN_EXISTS(MOTOR_CURRENT_PWM_E1)
_WRITE_CURRENT_PWM(E1);
_WRITE_CURRENT_PWM_DUTY(E1);
#endif
break;
}
Expand All @@ -3302,34 +3302,37 @@ void Stepper::report_positions() {

#elif HAS_MOTOR_CURRENT_PWM

#ifdef __SAM3X8E__
#define _RESET_CURRENT_PWM_FREQ(P) NOOP
#else
#define _RESET_CURRENT_PWM_FREQ(P) set_pwm_frequency(pin_t(P), MOTOR_CURRENT_PWM_FREQUENCY)
#endif
#define INIT_CURRENT_PWM(P) do{ SET_PWM(MOTOR_CURRENT_PWM_## P ##_PIN); _RESET_CURRENT_PWM_FREQ(MOTOR_CURRENT_PWM_## P ##_PIN); }while(0)

#if PIN_EXISTS(MOTOR_CURRENT_PWM_X)
SET_PWM(MOTOR_CURRENT_PWM_X_PIN);
INIT_CURRENT_PWM(X);
#endif
#if PIN_EXISTS(MOTOR_CURRENT_PWM_Y)
SET_PWM(MOTOR_CURRENT_PWM_Y_PIN);
INIT_CURRENT_PWM(Y);
#endif
#if PIN_EXISTS(MOTOR_CURRENT_PWM_XY)
SET_PWM(MOTOR_CURRENT_PWM_XY_PIN);
INIT_CURRENT_PWM(XY);
#endif
#if PIN_EXISTS(MOTOR_CURRENT_PWM_Z)
SET_PWM(MOTOR_CURRENT_PWM_Z_PIN);
INIT_CURRENT_PWM(Z);
#endif
#if PIN_EXISTS(MOTOR_CURRENT_PWM_E)
SET_PWM(MOTOR_CURRENT_PWM_E_PIN);
INIT_CURRENT_PWM(E);
#endif
#if PIN_EXISTS(MOTOR_CURRENT_PWM_E0)
SET_PWM(MOTOR_CURRENT_PWM_E0_PIN);
INIT_CURRENT_PWM(E0);
#endif
#if PIN_EXISTS(MOTOR_CURRENT_PWM_E1)
SET_PWM(MOTOR_CURRENT_PWM_E1_PIN);
INIT_CURRENT_PWM(E1);
#endif

refresh_motor_power();

// Set Timer5 to 31khz so the PWM of the motor power is as constant as possible. (removes a buzzing noise)
#ifdef __AVR__
SET_CS5(PRESCALER_1);
#endif
#endif
}

Expand Down
4 changes: 4 additions & 0 deletions Marlin/src/module/stepper.h
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,10 @@ class Stepper {
#ifndef PWM_MOTOR_CURRENT
#define PWM_MOTOR_CURRENT DEFAULT_PWM_MOTOR_CURRENT
#endif
#ifndef MOTOR_CURRENT_PWM_FREQUENCY
#define MOTOR_CURRENT_PWM_FREQUENCY 31400
#endif

#define MOTOR_CURRENT_COUNT LINEAR_AXES
#elif HAS_MOTOR_CURRENT_SPI
static constexpr uint32_t digipot_count[] = DIGIPOT_MOTOR_CURRENT;
Expand Down