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

Only increment Enable status for RGB Matrix if it supports it #5664

Closed
wants to merge 3 commits into from

Conversation

drashna
Copy link
Member

@drashna drashna commented Apr 20, 2019

Description

Right now, the RGB Matrix toggle code increments the variable. That's fine, if RGB_MATRIX_EXTRA_TOG is defined. But it's not normally. So, on non-massdrop boards, it's creating an issue where you have to hit the toggle multiple times.

This should fix the issue.

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

@XScorpion2
Copy link
Contributor

Also fixed by #5619

Copy link
Contributor

@yanfali yanfali left a comment

Choose a reason for hiding this comment

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

Lgtm

quantum/rgb_matrix.c Outdated Show resolved Hide resolved
Copy link
Contributor

@XScorpion2 XScorpion2 left a comment

Choose a reason for hiding this comment

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

Approved, even though it will make me rebase my pr later =P

@mechmerlin
Copy link
Contributor

This worked on my doro67, would like to see confirmation that this doesn't break functionality on massdrop boards.

@drashna
Copy link
Member Author

drashna commented Apr 22, 2019

@mechmerlin The Massdrop Boards are the only ones with the "extra toggle" defined. So, yeah, it shouldn't break anything. It just wasn't included in the rgb matrix code, accidently.

https://github.com/qmk/qmk_firmware/blob/master/keyboards/massdrop/alt/config.h#L126
https://github.com/qmk/qmk_firmware/blob/master/keyboards/massdrop/ctrl/config.h#L126

Also, #5619 addresses this, and removes the need for it altogether. But this is a smaller change.

@drashna
Copy link
Member Author

drashna commented Apr 30, 2019

superseded.

@drashna drashna closed this Apr 30, 2019
@drashna drashna deleted the fix/rgb_matrix_toggle branch April 30, 2019 01:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants