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

dygma/raise: multiple fixes and improvements #18361

Merged
merged 3 commits into from
Sep 28, 2022

Conversation

justfalter
Copy link
Contributor

@justfalter justfalter commented Sep 14, 2022

This PR makes multiple fixes and improvements to the dygma raise implementation. Much of this centered around improving the accuracy and performance of LED light handling.

  • respect RGB_MATRIX_ENABLE = no
  • Only flush LED buffer when changes have occurred. This significantly improves typing accuracy by reducing traffic related to LED state, increasing the bandwidth available for matrix scans.
  • improve configurability of RGB matrix, reduce refresh rate
    • Reduce LED refresh rate from 60fps to 10fps, given the significant impact on typing accurate.
    • Allow enabling/disabling of reactive RGB matrix modes from rules.mk
  • fix right-hand ansi LED key map
    • The ansi layout right hand LED key mapping was seemingly flipped and offset by 1 key. For example, when the LED for "7" was to light, the "= +" LED would light, instead. Reversing each row of the led mapping resolved the issue.
    • LEDs for the "\ |" and "enter" keys were swapped.
  • fix color order to GRB
    • Colors were incorrectly mapped with red and green swapped. Actual color order is GRB, not RGB.
  • Add color-correction for red channel, as it apparently receives more voltage than the other color channels.

Description

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

- respect RGB_MATRIX_ENABLE = no
- Only flush LED buffer when changes have occurred. This significantly
improves typing accuracy by reducing traffic related to LED state,
increasing the bandwidth available for matrix scans.
- improve configurability of RGB matrix, reduce refresh rate
  - Reduce LED refresh rate from 60fps to 10fps, given the significant
impact on typing accuract.
  - Allow enabling/disabling of reactive RGB matrix modes from rules.mk
- fix right-hand ansi LED key map
  - The ansi layout right hand LED key mapping was seemingly flipped and
offset by 1 key. For example, when the LED for "7" was to light, the
"= +" LED would light, instead. Reversing each row of the led mapping
resolved the issue.
  - LEDs for the "\ |" and "enter" keys were swapped.
- fix color order to GRB
  - Colors were incorrectly mapped with red and green swapped. Actual
    color order is GRB, not RGB.
- Add color-correction for red channel, as it apparently receives more
  voltage than the other color channels.
@justfalter
Copy link
Contributor Author

@ibash you might be interested in this

@ibash
Copy link

ibash commented Sep 14, 2022

Awesome! Lgtm

Out of curiosity what are you using for a mcu?

@justfalter
Copy link
Contributor Author

Out of curiosity what are you using for a mcu?

Some random black pill clone I picked up off of amazon.

@drashna drashna requested a review from a team September 14, 2022 17:46
keyboards/handwired/dygma/raise/leds.c Outdated Show resolved Hide resolved
keyboards/handwired/dygma/raise/leds.c Outdated Show resolved Hide resolved
keyboards/handwired/dygma/raise/leds.h Outdated Show resolved Hide resolved
keyboards/handwired/dygma/raise/leds.h Outdated Show resolved Hide resolved
- Fix iso LED mapping (matches ansi)
- Fix g_led_config parsing complaints. Parser does not like non-numeric values.
- Eliminate ifdefs for RGB_MATRIX_ENABLE.
- add post_rules.mk to handle addition of leds.c sources when
  RGB_MATRIX_ENABLE is yes.
@justfalter
Copy link
Contributor Author

@fauxpark I got the changes in to use post_rules.mk instead of `#ifdef``'s .

I also fixed dygma/raise's q_led_config, which was failing to validate (#17105).

@justfalter
Copy link
Contributor Author

@drashna is this something you can take care of merging? I've addressed @fauxpark's requested changes.

@justfalter justfalter requested review from drashna and fauxpark and removed request for fauxpark and drashna September 28, 2022 01:41
@ibash
Copy link

ibash commented Sep 28, 2022

@justfalter thanks for all the work here! Hopefully this can get a second approval soon...

@drashna drashna merged commit d6f7763 into qmk:master Sep 28, 2022
@justfalter justfalter deleted the dygma-raise-fixes-and-improvements branch September 29, 2022 13:22
justfalter referenced this pull request in ibash/qmk_firmware Sep 29, 2022
Adds stm32 eeprom emulation for stm32F4 mcus. Specifically this:
1. Adds the stm32f4 flash hals.
2. Adds an eeprom shim to match the avr library.
3. Sets a custom memory map so that the first sector is used for the
   vector table, the next two sectors are used for eeprom emulation, and
   finally the rest is used for code.
   We do this because the first four sectors are 16kb, and the rest are
   much larger. If we used a larger sector for eeprom emulation it would
   be slower and more wasteful.
4. Adds via json for the dygma raise

Left to do:
Make this play well with the preexisting eeprom library.
It'd be best to move them all into eeprom_stm32 and then change the
filenames to be per mcu, and then selectively load them.
tacahiroy pushed a commit to tacahiroy/qmk_firmware that referenced this pull request Oct 2, 2022
aivalja pushed a commit to aivalja/qmk_firmware that referenced this pull request Nov 7, 2022
ramonimbao pushed a commit to ramonimbao/qmk_firmware that referenced this pull request Nov 28, 2022
nolanseaton pushed a commit to nolanseaton/qmk_firmware that referenced this pull request Jan 23, 2023
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.

4 participants