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

Tidy up SLEEP_LED_ENABLE rules #15362

Merged
merged 31 commits into from
Dec 1, 2021
Merged

Conversation

fauxpark
Copy link
Member

Description

This feature isn't used very much, and its rule in the template has a rather strange comment attached to it...

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout/userspace (addition or update)
  • Documentation

Issues Fixed or Closed by This PR

Checklist

  • My code follows the code style of this project: C, Python
  • I have read the PR Checklist document and have made the appropriate changes.
  • 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 fauxpark requested a review from a team November 30, 2021 18:14
@github-actions github-actions bot added via Adds via keymap and/or updates keyboard for via support and removed core labels Nov 30, 2021
Copy link
Contributor

@sigprof sigprof left a comment

Choose a reason for hiding this comment

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

The comment is at least partially true (on AVR this feature uses a timer specified by SLEEP_LED_TIMER, which is 1 by default, but can also be 3 — these are the same timers that can be used by the backlight or audio code, but there are no checks whether the specified timer is actually not used by those subsystems, except possibly a duplicate definition of the TIMERx_COMPA_vect ISR).

BTW, this feature also invokes led_set() from an interrupt handler, which might be unsafe, depending on what is done in led_update_kb() or even led_update_user().

@fauxpark fauxpark merged commit c12b997 into qmk:master Dec 1, 2021
@fauxpark fauxpark deleted the sleep-led-rules-remove branch December 1, 2021 10:13
cadusk pushed a commit to cadusk/qmk_firmware that referenced this pull request Dec 2, 2021
* qmk/develop: (76 commits)
  Update ReadMe for the Roopad (qmk#15380)
  Tidy up existing i2c_master implementations (qmk#15376)
  Remove pointless `CUSTOM_MATRIX = no` (qmk#15383)
  Remove manual enable of LTO within keyboards (qmk#15377)
  [Keyboard] Han60 QMK / VIA Support (qmk#15332)
  Convert ai03/orbit to SPLIT_KEYBOARD (qmk#15340)
  Convert not_so_minidox to SPLIT_KEYBOARD (qmk#15306)
  Tidy up `SLEEP_LED_ENABLE` rules (qmk#15362)
  [Keyboard] Add support for Mode SixtyFive M65HA and M65HI (qmk#14685)
  Implement MAGIC_TOGGLE_CONTROL_CAPSLOCK (qmk#15368)
  Rename Layout Macros for TKLs with F13 keys (qmk#15361)
  [Docs] Reorder functions in Understanding QMK (qmk#15357)
  [Keyboard] Fix up Endgame34 (qmk#15366)
  [Keyboard] Fix compilation issues for Ploopy Trackball classic (qmk#15364)
  [Keyboard] Add missng define for 4x6 Tractyl Manuform (qmk#15363)
  [Core] Added chconf.h for WB32 (qmk#15359)
  [Keyboard] kangaroo improvements (qmk#15350)
  [Keyboard] Convert ergoinu to SPLIT_KEYBOARD (qmk#15305)
  [Keymap] Keebio Sinc layout with macOS support (qmk#15324)
  Fixup paths for `ramonimbao/wete/v2`. (qmk#15360)
  ...
rudism pushed a commit to rudism/qmk_firmware that referenced this pull request Jan 16, 2022
coquizen pushed a commit to coquizen/qmk_firmware that referenced this pull request Jun 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement keyboard keymap via Adds via keymap and/or updates keyboard for via support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants