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

Add 'capslock backlight' feature to Iron180 #15462

Merged
merged 3 commits into from
Dec 27, 2021

Conversation

Gondolindrim
Copy link
Contributor

Description

Some Iron180 users have shown interest in only having the caps lock LED soldered and for it to work as a caps lock indicator.

This PR adds a CAPSLOCK_BACKLIGHT define in config.h which in turn enables a led state function in iron180.c that toggles backlight and breathing according to caps lock. The PR also contains README instructions.

Types of Changes

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

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).

@Gondolindrim Gondolindrim marked this pull request as ready for review December 12, 2021 15:55
@zvecr
Copy link
Member

zvecr commented Dec 12, 2021

Any reason to go custom over cores BACKLIGHT_CAPS_LOCK implementation?

@Gondolindrim
Copy link
Contributor Author

Gondolindrim commented Dec 12, 2021

Any reason to go custom over cores BACKLIGHT_CAPS_LOCK implementation?

As far as I tested, BACKLIGHT_CAPS_LOCK does not play well with breathing: it works fine for static light but if breathing is on, the LEDs keep on breathing no matter the state of caps lock. This is also reproduced using the controls in VIA. The code of the PR handles breathing properly (then again, as far as I tested).

@drashna drashna requested a review from a team December 17, 2021 16:11
@fauxpark
Copy link
Member

That sounds like something that should be fixed in core code, rather than reimplemented here.

@Gondolindrim
Copy link
Contributor Author

Gondolindrim commented Dec 17, 2021

That sounds like something that should be fixed in core code, rather than reimplemented here.

I DM'd @zvecr at length about this. The thing is, the core code implementation's intent is to treat breathing as a different state than backlight itself; in other words, breathing is not supposed to be turned on and off when backlight is respectively enabled or disabled because they are supposed to be two different features.

Now since zvecr is a developer and a key member I took his statement as a somewhat "official" QMK opinion on it. However, the intended effect I want for Iron180 is for breathing to be turned on and off with the backlight, hence why the keyboard-level implementation in favour of the core-level implementation.

So in this discussion me and zvecr agreed that maybe we ought to leave the core implementation as it is; the discussion now is what would be the best way to implement this feature in the keyboard-level. According to zvecr the functions I used, videlicet, backlight_enable(), backlight_disable(), breathing_enable() and breathing_disable() will write to EEPROM, hence the way it is implemented now will operate EEPROM every time caps lock is pressed which would not be a particular problem were Iron180 not powered by STM32F072 which uses the EEPROM simulation in flash, potentially wearing flash. Maybe I should, instead of enable-disable, just operate with backlight and breathing brightness or levels?

EDIT: typos

@tzarc tzarc merged commit 52b53cc into qmk:master Dec 27, 2021
tiktuk pushed a commit to tiktuk/qmk_firmware that referenced this pull request Jan 1, 2022
* Add 'capslock backlight' capability to Iron180

* Update readme

* Revers CAPSLOCK_BACKLIGHT back to default
ryphon pushed a commit to ryphon/qmk_firmware that referenced this pull request Jan 12, 2022
* Add 'capslock backlight' capability to Iron180

* Update readme

* Revers CAPSLOCK_BACKLIGHT back to default
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants