Fix breathing always on for soft PWM #5983

fauxpark · 2019-05-26T11:00:48Z

Description

For backlight systems that don't use a hardware PWM-capable pin, breathing_task() is always running! This is because for hardware PWM, breathing_interrupt_enable() and _disable() modify the TIMSK register to control the ISR, whereas the software PWM versions of these macros (they're not functions) set the static bool breathing... which is never checked anywhere!

So, before we run breathing_task(), we get the green light from is_breathing(). I can now control software PWM backlight properly with my test setup - though it is quite flickery at low brightness levels, but I think that is a separate issue.

This may (partially) resolve some of the recent issues people have been having with soft backlighting doing weird things. Eg:

#5425, #5953

Types of Changes

Issues Fixed or Closed by This PR

Checklist

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.
I have added tests to cover my changes.
I have tested the changes and verified that they work and don't break anything (as well as I can manage).

fauxpark · 2019-05-26T12:43:31Z

@FSund @HMarxen @al3ph could you guys test these changes and see if there is any improvement for you?

FSund · 2019-05-27T06:35:10Z

This has fixed most issues with backlight on my AMJ66 board, as long as I remember #define BACKLIGHT_ON_STATE 1.

al3ph · 2019-05-28T07:43:33Z

Yep, fixed my flicker as well, CHEERS!

al3ph · 2019-05-28T07:47:47Z

Hmm, no I'm mistaken still happening, (it's somewhat random).

fauxpark · 2019-05-28T07:51:04Z

Did you try playing with BACKLIGHT_ON_STATE?

fauxpark · 2019-05-29T10:33:39Z

The original porter of the XD87 claims (#4182 (comment)) that the backlight is controlled with a P-channel MOSFET rather than the usual N-channel, which is also the conclusion I've come to by tracing an image of the PCB. So, I'm pretty confident now that BACKLIGHT_ON_STATE 1 should do the trick.

Now, breathing will only be unsupported when Timers 1 and 3 are both used by Audio

* 'master' of https://github.com/qmk/qmk_firmware: (213 commits) [Keymap] New keymap for crkbd (qmk#6103) [Keyboard] fixes for issue with aanzee qmk port (qmk#6159) Add ergodash layout, update the backlight numbers for the rgb backlight to be the actual intended colors. [Keyboard] Aeboards Ext65 - New keyboard & Aegis Update (qmk#6127) Fix breathing always on for soft PWM (qmk#5983) [Keyboard] Added NK65 Picture in Readme as promised (qmk#6163) [Keymap] Added Deft layout (qmk#6153) [Keyboard] 1up60hse: Add RGBLIGHT_SLEEP To Default Config (qmk#6164) [Keymap] Actually swap space and left control in gaming mode (qmk#6162) Set default I2C clock speed to 100kHz for split_common (qmk#6161) [Keyboard] Planck Layout Macro Refactor, Part II (qmk#6156) [Keyboard] Fix incorrect RGBLED_NUM value (qmk#6148) [Keymap] Update Jotix keymap (qmk#6154) [Keymap] Add new mod tap dances to Hacker Dvorak (qmk#6155) [Keyboard] added custom keyboard (qmk#6141) [Keymap] Add keymap for keebio/nyquist (qmk#6144) [Keymap] Update to personal keymaps (qmk#6142) [Keymap] ortho_4x12: bredfield (qmk#6137) [Keymap] 40percent/gherkin Midi Layout (qmk#6130) Fix backlight breathing on C6 (qmk#6102) ...

* Fix breathing always on for soft PWM * Remove reference to hardware PWM pins in BACKLIGHT_BREATHING description Now, breathing will only be unsupported when Timers 1 and 3 are both used by Audio * Document BACKLIGHT_ON_STATE and its purpose

Partial fix for backlight breathing. - Requires qmk#5983 to fix fully (confirmed by FSund and fauxpark) Co-Authored-By: fauxpark <fauxpark@gmail.com> Co-Authored-By: Filip Sund <filip.sund@gmail.com>

* Added nearly perfect config for AMJ66, only missing top right key. * Correct the layout macro * Add layout mock-up to amj66.h * Update and comment out the backlight definitions in config.h The backlight pin was found to be D4, but there appears to be a bug in QMK that affects this keyboard. Commenting out for now. * Try to make a sensible default keymap * Add testing keymap for FSund Include the keymap that was being used for testing. Don't forget to refactor this later into an actually useful keymap. * Suggestions by fauxpark - uncomment the backlight configuration - fix the default keymap - remove commented MCU rule - specify the bootloader - make mental note to not try to write code at 3:30 in the morning * Add LAYOUT_66_ansi and LAYOUT_66_iso macros - include QMK Configurator data - enable Community Layout support * Add comments about layout variants to amj66.h * Add #define BACKLIGHT_ON_STATE 1 Partial fix for backlight breathing. - Requires #5983 to fix fully (confirmed by FSund and fauxpark) Co-Authored-By: fauxpark <fauxpark@gmail.com> Co-Authored-By: Filip Sund <filip.sund@gmail.com> * DEBOUNCING_DELAY -> DEBOUNCE * Move AMJ66 files into new AMJKeyboard directory * Correct Manufacturer in USB Device Descriptor * Remove comment regarding source fork * Correct the readme * Update default keymap to match the details given in its readme * White-space edit fsund_test keymap Makes its formatting more consistent with other 66% keymaps. No logic changes. * Linting info.json Debug-style linting (one key object per line) and minor edits to key labels. * Remove fsund_test keymap * Add FSund as a maintainer in info.json

* Added nearly perfect config for AMJ66, only missing top right key. * Correct the layout macro * Add layout mock-up to amj66.h * Update and comment out the backlight definitions in config.h The backlight pin was found to be D4, but there appears to be a bug in QMK that affects this keyboard. Commenting out for now. * Try to make a sensible default keymap * Add testing keymap for FSund Include the keymap that was being used for testing. Don't forget to refactor this later into an actually useful keymap. * Suggestions by fauxpark - uncomment the backlight configuration - fix the default keymap - remove commented MCU rule - specify the bootloader - make mental note to not try to write code at 3:30 in the morning * Add LAYOUT_66_ansi and LAYOUT_66_iso macros - include QMK Configurator data - enable Community Layout support * Add comments about layout variants to amj66.h * Add #define BACKLIGHT_ON_STATE 1 Partial fix for backlight breathing. - Requires qmk#5983 to fix fully (confirmed by FSund and fauxpark) Co-Authored-By: fauxpark <fauxpark@gmail.com> Co-Authored-By: Filip Sund <filip.sund@gmail.com> * DEBOUNCING_DELAY -> DEBOUNCE * Move AMJ66 files into new AMJKeyboard directory * Correct Manufacturer in USB Device Descriptor * Remove comment regarding source fork * Correct the readme * Update default keymap to match the details given in its readme * White-space edit fsund_test keymap Makes its formatting more consistent with other 66% keymaps. No logic changes. * Linting info.json Debug-style linting (one key object per line) and minor edits to key labels. * Remove fsund_test keymap * Add FSund as a maintainer in info.json