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

Feature request/implementation: LED pattern responsive to key input #4846

Closed
2 of 4 tasks
valen214 opened this issue Jan 14, 2019 · 1 comment
Closed
2 of 4 tasks

Comments

@valen214
Copy link
Contributor

valen214 commented Jan 14, 2019

Feature Request Type

  • New behavior
  • Core Functionality
  • Add-on hardware support (e.g. audio, RGB, OLED screen, etc.)
  • Alteration (enhancement/optimization) of existing Feature(s)

Description

related (pull request)
source project

This issue focuses on Implementing LED effect by modifying keymap.c only.
Effect showcase is available in video linked in source project (more patterns are added since then)

Ideas

  1. This feature is quite complete in the source project, which is however forked from Massdrop's base, thus not compatible with this mother qmk project (they have additional feature like led_instructions). I am working on porting it to qmk (several methods are possible, more below), which is the main point of this issue.
  2. I failed to get <rgblight.h> working, it is why I try to use led_map->rgb (in qmk) or led_instruction_t led_instructions[] (in Massdrop) instead.
  3. "Why so stubborn to only use keymap.c?": I thought it would be less intrusive to other part of the project. It is welcome to be offered any better solutions.

What I am working on currently

Porting from source project to be compatible to this base project (as mentioned)
several ways are proposed (tested):

  1. setting led_per_run = 1 and modify led rgb values in keymap.c: matrix_scan_user() (in each and every invocation)
    • potential problem: performance, (might, although unlikely, not working on other occasion)
    • The reason setting led_per_run = 1 is because modifying rgb in matrix_scan_user() causes racing with arm_atsam/led_matrix.c: led_matrix_run(), which makes some led's rgb is modified again in led_matrix_run() after matrix_scan_user() if led_per_run is 15 (default value).
  2. set led_lighting_mode = LED_MODE_KEYS_ONLY and modify led_cur->scan
    • in tmk_core/protocol/arm_atsam/led_matrix.c: led_matrix_run(): line 311 doing so would skip certain led, so the overriding and racing problem as mentioned above can be avoided.
    • problem: Fn + Z toggle led mode would not behave normally.
  3. modify led_matrix_run() by modifying tmk_core/protocol/arm_atsam/led_matrix.c, or overriding such function in keymap.c (which is possible because led_matrix_run() has __attribute__((weak)) reference)
    • open up many possibilities, able to cover the problems of above methods.
    • problem: need to submit pull request / merge branch
@drashna
Copy link
Member

drashna commented Feb 8, 2019

I believe that #4856 should have closed this issue.

I'm closing it now. Reopen if i'm incorrect.

@drashna drashna closed this as completed Feb 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants