Skip to content

Enhancement of PWM, with dithering #6100

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Mar 24, 2017
Merged

Enhancement of PWM, with dithering #6100

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2aed66a
temperature: Fix SOFT_PWM off by one
StefanBruens Mar 12, 2017
35a55d5
SOFT_PWM: Implement dithering if SOFT_PWM_SCALE is 1 or more
StefanBruens Mar 12, 2017
0a74774
soft_pwm: avoid useless refetches of pwm_count
StefanBruens Mar 14, 2017
6a040a6
SOFT_PWM: Do not switch off heaters twice on pwm_count wraparound
StefanBruens Mar 18, 2017
043be28
Use "& 0x3F" instead of "% 64"
thinkyhead Mar 23, 2017
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
6 changes: 6 additions & 0 deletions Marlin/Configuration.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1424,6 +1424,12 @@
// at zero value, there are 128 effective control positions.
#define SOFT_PWM_SCALE 0

// If SOFT_PWM_SCALE is set to a value higher than 0, dithering can
// be used to mitigate the associated resolution loss. If enabled,
// some of the PWM cycles are stretched so on average the wanted
// duty cycle is attained.
//#define SOFT_PWM_DITHER

// Temperature status LEDs that display the hotend and bed temperature.
// If all hotends and bed temperature and temperature setpoint are < 54C then the BLUE led is on.
// Otherwise the RED led is on. There is 1C hysteresis.
Expand Down
118 changes: 65 additions & 53 deletions Marlin/temperature.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1512,6 +1512,8 @@ void Temperature::isr() {
static uint8_t temp_count = 0;
static TempState temp_state = StartupDelay;
static uint8_t pwm_count = _BV(SOFT_PWM_SCALE);
// avoid multiple loads of pwm_count
uint8_t pwm_count_tmp = pwm_count;

// Static members for each heater
#if ENABLED(SLOW_PWM_HEATERS)
Expand All @@ -1521,7 +1523,7 @@ void Temperature::isr() {
static uint8_t state_heater_ ## n = 0; \
static uint8_t state_timer_heater_ ## n = 0
#else
#define ISR_STATICS(n) static uint8_t soft_pwm_ ## n
#define ISR_STATICS(n) static uint8_t soft_pwm_ ## n = 0
#endif

// Statics per heater
Expand All @@ -1544,72 +1546,82 @@ void Temperature::isr() {
#endif

#if DISABLED(SLOW_PWM_HEATERS)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not:
constexpr uint8_t pwm_mask = ENABLED(SOFT_PWM_DITHER) ? _BV(SOFT_PWM_SCALE) - 1 : 0;

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately ENABLED() cannot be used that way. Only in preprocessor expressions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you tried it?
after the preprocessor run the code reads as follows:
constexpr uint8_t pwm_mask = 1 ? 1 : 0;
which is both syntactically and semantically correct.

Copy link
Member Author

@thinkyhead thinkyhead Apr 2, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you tried it?

I have indeed. The ENABLED macro uses preprocessor voodoo that only works in preprocessor expressions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The preprocessor is always run! "ENABLED(...)" by itself is a preprocessor expression, which is expanded prior to the compilation itself.

If you want to see the output of the preprocessor, pass "-E" to gcc.

I tried it, checked the created preprocessor output, and checked the generated assembly. If it does not work for you, whats the error message?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code:

  int x = ENABLED(THINGS);
  SERIAL_ECHO(x);

Error:

error: 'SWITCH_ENABLED_THINGS' was not declared in this scope

Copy link
Member Author

@thinkyhead thinkyhead Apr 2, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nonexistent macros are expanded as their literal text. This is fine in a preprocessor expression, because non-existent macros (in this case SWITCH_ENABLED_THINGS) are treated as a value of 0, and the preprocessor doesn't throw an error.

"ENABLED(...)" by itself is a preprocessor expression

Well, it is a macro that will be expanded. "Preprocessor expressions" (sorry if I get the terminology wrong) are evaluated in lines beginning with #if and other such directives. Hence you can't say:

bool q = defined(YOMAMA);

constexpr uint8_t pwm_mask =
#if ENABLED(SOFT_PWM_DITHER)
_BV(SOFT_PWM_SCALE) - 1
#else
0
#endif
;

/**
* Standard PWM modulation
*/
if (pwm_count == 0) {
soft_pwm_0 = soft_pwm[0];
WRITE_HEATER_0(soft_pwm_0 > 0 ? HIGH : LOW);
if (pwm_count_tmp >= 127) {
pwm_count_tmp -= 127;
soft_pwm_0 = (soft_pwm_0 & pwm_mask) + soft_pwm[0];
WRITE_HEATER_0(soft_pwm_0 > pwm_mask ? HIGH : LOW);
#if HOTENDS > 1
soft_pwm_1 = soft_pwm[1];
WRITE_HEATER_1(soft_pwm_1 > 0 ? HIGH : LOW);
soft_pwm_1 = (soft_pwm_1 & pwm_mask) + soft_pwm[1];
WRITE_HEATER_1(soft_pwm_1 > pwm_mask ? HIGH : LOW);
#if HOTENDS > 2
soft_pwm_2 = soft_pwm[2];
WRITE_HEATER_2(soft_pwm_2 > 0 ? HIGH : LOW);
soft_pwm_2 = (soft_pwm_2 & pwm_mask) + soft_pwm[2];
WRITE_HEATER_2(soft_pwm_2 > pwm_mask ? HIGH : LOW);
#if HOTENDS > 3
soft_pwm_3 = soft_pwm[3];
WRITE_HEATER_3(soft_pwm_3 > 0 ? HIGH : LOW);
soft_pwm_3 = (soft_pwm_3 & pwm_mask) + soft_pwm[3];
WRITE_HEATER_3(soft_pwm_3 > pwm_mask ? HIGH : LOW);
#endif
#endif
#endif

#if HAS_HEATER_BED
soft_pwm_BED = soft_pwm_bed;
WRITE_HEATER_BED(soft_pwm_BED > 0 ? HIGH : LOW);
soft_pwm_BED = (soft_pwm_BED & pwm_mask) + soft_pwm_bed;
WRITE_HEATER_BED(soft_pwm_BED > pwm_mask ? HIGH : LOW);
#endif

#if ENABLED(FAN_SOFT_PWM)
#if HAS_FAN0
soft_pwm_fan[0] = fanSpeedSoftPwm[0] >> 1;
WRITE_FAN(soft_pwm_fan[0] > 0 ? HIGH : LOW);
soft_pwm_fan[0] = (soft_pwm_fan[0] & pwm_mask) + fanSpeedSoftPwm[0] >> 1;
WRITE_FAN(soft_pwm_fan[0] > pwm_mask ? HIGH : LOW);
#endif
#if HAS_FAN1
soft_pwm_fan[1] = fanSpeedSoftPwm[1] >> 1;
WRITE_FAN1(soft_pwm_fan[1] > 0 ? HIGH : LOW);
soft_pwm_fan[1] = (soft_pwm_fan[1] & pwm_mask) + fanSpeedSoftPwm[1] >> 1;
WRITE_FAN1(soft_pwm_fan[1] > pwm_mask ? HIGH : LOW);
#endif
#if HAS_FAN2
soft_pwm_fan[2] = fanSpeedSoftPwm[2] >> 1;
WRITE_FAN2(soft_pwm_fan[2] > 0 ? HIGH : LOW);
soft_pwm_fan[2] = (soft_pwm_fan[2] & pwm_mask) + fanSpeedSoftPwm[2] >> 1;
WRITE_FAN2(soft_pwm_fan[2] > pwm_mask ? HIGH : LOW);
#endif
#endif
}

if (soft_pwm_0 < pwm_count) WRITE_HEATER_0(0);
#if HOTENDS > 1
if (soft_pwm_1 < pwm_count) WRITE_HEATER_1(0);
else {
if (soft_pwm_0 <= pwm_count_tmp) WRITE_HEATER_0(0);
#if HOTENDS > 1
if (soft_pwm_1 <= pwm_count_tmp) WRITE_HEATER_1(0);
#endif
#if HOTENDS > 2
if (soft_pwm_2 < pwm_count) WRITE_HEATER_2(0);
#if HOTENDS > 3
if (soft_pwm_3 < pwm_count) WRITE_HEATER_3(0);
#endif
if (soft_pwm_2 <= pwm_count_tmp) WRITE_HEATER_2(0);
#endif
#endif

#if HAS_HEATER_BED
if (soft_pwm_BED < pwm_count) WRITE_HEATER_BED(0);
#endif

#if ENABLED(FAN_SOFT_PWM)
#if HAS_FAN0
if (soft_pwm_fan[0] < pwm_count) WRITE_FAN(0);
#if HOTENDS > 3
if (soft_pwm_3 <= pwm_count_tmp) WRITE_HEATER_3(0);
#endif
#if HAS_FAN1
if (soft_pwm_fan[1] < pwm_count) WRITE_FAN1(0);

#if HAS_HEATER_BED
if (soft_pwm_BED <= pwm_count_tmp) WRITE_HEATER_BED(0);
#endif
#if HAS_FAN2
if (soft_pwm_fan[2] < pwm_count) WRITE_FAN2(0);

#if ENABLED(FAN_SOFT_PWM)
#if HAS_FAN0
if (soft_pwm_fan[0] <= pwm_count_tmp) WRITE_FAN(0);
#endif
#if HAS_FAN1
if (soft_pwm_fan[1] <= pwm_count_tmp) WRITE_FAN1(0);
#endif
#if HAS_FAN2
if (soft_pwm_fan[2] <= pwm_count_tmp) WRITE_FAN2(0);
#endif
#endif
#endif
}

// SOFT_PWM_SCALE to frequency:
//
Expand All @@ -1619,8 +1631,7 @@ void Temperature::isr() {
// 3: / 16 = 61.0352 Hz
// 4: / 8 = 122.0703 Hz
// 5: / 4 = 244.1406 Hz
pwm_count += _BV(SOFT_PWM_SCALE);
pwm_count &= 0x7F;
pwm_count = pwm_count_tmp + _BV(SOFT_PWM_SCALE);

#else // SLOW_PWM_HEATERS

Expand Down Expand Up @@ -1694,7 +1705,8 @@ void Temperature::isr() {
#endif

#if ENABLED(FAN_SOFT_PWM)
if (pwm_count == 0) {
if (pwm_count_tmp >= 127) {
pwm_count_tmp = 0;
#if HAS_FAN0
soft_pwm_fan[0] = fanSpeedSoftPwm[0] >> 1;
WRITE_FAN(soft_pwm_fan[0] > 0 ? HIGH : LOW);
Expand All @@ -1709,15 +1721,15 @@ void Temperature::isr() {
#endif
}
#if HAS_FAN0
if (soft_pwm_fan[0] < pwm_count) WRITE_FAN(0);
if (soft_pwm_fan[0] <= pwm_count_tmp) WRITE_FAN(0);
#endif
#if HAS_FAN1
if (soft_pwm_fan[1] < pwm_count) WRITE_FAN1(0);
if (soft_pwm_fan[1] <= pwm_count_tmp) WRITE_FAN1(0);
#endif
#if HAS_FAN2
if (soft_pwm_fan[2] < pwm_count) WRITE_FAN2(0);
if (soft_pwm_fan[2] <= pwm_count_tmp) WRITE_FAN2(0);
#endif
#endif //FAN_SOFT_PWM
#endif // FAN_SOFT_PWM

// SOFT_PWM_SCALE to frequency:
//
Expand All @@ -1727,13 +1739,13 @@ void Temperature::isr() {
// 3: / 16 = 61.0352 Hz
// 4: / 8 = 122.0703 Hz
// 5: / 4 = 244.1406 Hz
pwm_count += _BV(SOFT_PWM_SCALE);
pwm_count &= 0x7F;
pwm_count = pwm_count_tmp + _BV(SOFT_PWM_SCALE);

// increment slow_pwm_count only every 64 pwm_count (e.g., every 8s)
if ((pwm_count % 64) == 0) {
// increment slow_pwm_count only every 64th pwm_count,
// i.e. yielding a PWM frequency of 16/128 Hz (8s).
if (((pwm_count >> SOFT_PWM_SCALE) & 0x3F) == 0) {
slow_pwm_count++;
slow_pwm_count &= 0x7f;
slow_pwm_count &= 0x7F;

// EXTRUDER 0
if (state_timer_heater_0 > 0) state_timer_heater_0--;
Expand All @@ -1749,7 +1761,7 @@ void Temperature::isr() {
#if HAS_HEATER_BED
if (state_timer_heater_BED > 0) state_timer_heater_BED--;
#endif
} // (pwm_count % 64) == 0
} // ((pwm_count >> SOFT_PWM_SCALE) & 0x3F) == 0

#endif // SLOW_PWM_HEATERS

Expand Down