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.

Sign up for GitHub

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

Jump to bottom

Set the correct RGB LED count on YD60MQ #8629

Merged
merged 6 commits into from
Apr 9, 2020
Merged

Set the correct RGB LED count on YD60MQ #8629

merged 6 commits into from
Apr 9, 2020

Conversation

vomindoraan
Copy link
Contributor

@vomindoraan vomindoraan commented Mar 31, 2020 •
edited
Loading

Description

The YD60MQ PCB comes with 16 addressable RGB LEDs, but it's currently configured to only use 12. This PR fixes that.

Tested and confirmed working by EliaLikeYou#2670 on Discord, who owns one of the boards.
Reported on Reddit, here.

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

@vomindoraan vomindoraan changed the title Fix RGB LED count on YD60MQ Set the correct RGB LED count on YD60MQ Mar 31, 2020
@mtei mtei requested a review from a team March 31, 2020 20:37
zvecr
zvecr reviewed Mar 31, 2020
View reviewed changes
Copy link
Member

@zvecr zvecr left a comment

Choose a reason for hiding this comment

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

A quick google shows a image result of a PCB with 12 LEDs https://candykeys.com/product/yd60mq-pcb-60-iso-ansi-backlit. If there are multiple revisions of the board (and i didn't google wrong....), with different amounts of LEDs, then I don't think this should go in as is.

@drashna
Copy link
Member

drashna commented Mar 31, 2020

If that's the case, it may be better as revisions, then.

@vomindoraan
Copy link
Contributor Author

Interesting, I wonder if that's the only difference between the two versions. I'll need to look into whether the board has revision numbers or other distinguishing features (other than the number of LEDs) by which we could label the revisions.

@vomindoraan
Copy link
Contributor Author

I couldn't find any distinguishing features other than the number of LEDs, so I've split the board into two revisions based on that. Let me know if I missed anything.

Note to self: yd60mq and yd68 should probably be moved into a ydkb parent directory; leaving that for another PR, though.

Erovia
Erovia requested changes Apr 8, 2020
View reviewed changes
Copy link
Member

@Erovia Erovia left a comment

Choose a reason for hiding this comment

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

  • Move the rules.mk back to the keyboard level
  • Add the DEFAULT_FOLDER directive.
  • Create empty rules.mk files on the "revision" levels.

keyboards/yd60mq/16led/rules.mk Outdated Show resolved Hide resolved
@vomindoraan
Copy link
Contributor Author

Good idea, I've made the changes you requested.

@Erovia Erovia requested review from a team and zvecr April 8, 2020 18:10
@noroadsleft
Copy link
Member

For the record, I hate having to use the DEFAULT_FOLDER directive, but here I'd consider it required until we come up with a solution for renaming/moving keyboards that works for the whole project.

@noroadsleft noroadsleft merged commit 4b10235 into qmk:master Apr 9, 2020
@noroadsleft
Copy link
Member

Thanks!

Shinichi-Ohki added a commit to Shinichi-Ohki/qmk_firmware that referenced this pull request Apr 9, 2020
@Shinichi-Ohki
Merge branch 'master' of https://github.com/qmk/qmk_firmware
e7edb70 
* 'master' of https://github.com/qmk/qmk_firmware: (53 commits)
  Set the correct RGB LED count on YD60MQ (qmk#8629)
  [Keymap] Updates to personal keymaps (qmk#8665)
  Fix compile issues related to NO_ACTION_MACRO/FUNCTION and LTO_ENABLE (qmk#8663)
  [Keymap] Userspace update Rgb layers code (qmk#8659)
  Add Choconum (qmk#8709)
  Add Via keymap for BM16-A (qmk#8681)
  Fix edge-case with config
  Don't hide for devs...
  Apply @skullydazed's suggestions, move 'import milc'
  Make dedicated sections for user/dev commands in docs
  Rebase on master, hide some other subcommands
  Use milc for config check, requirements fixes
  CLI: Add development mode support
  Update info.json (qmk#8723)
  format code according to conventions [skip ci]
  spi_master for AVR (qmk#8299)
  DennyTom's buttery_engine (qmk#8138)
  add via support for kira80 (qmk#8677)
  [Keyboard] Wheatfield Split75 (qmk#8511)
  Correctly handle json keymaps with ANY()
  ...
loadedsith added a commit to loadedsith/qmk_firmware that referenced this pull request Apr 9, 2020
@loadedsith
Merge branch 'master' into grahampheath
dfdbdca 
* master: (3035 commits)
  [Keymap] Update personal userspace and keymaps (qmk#8747)
  Add PS2_MOUSE_ROTATE to compensate for device orientation (qmk#8650)
  Add RGB support in via to launchpad (qmk#8621)
  VIA support for the KBDFans KBD6x (qmk#8680)
  Set the correct RGB LED count on YD60MQ (qmk#8629)
  [Keymap] Updates to personal keymaps (qmk#8665)
  Fix compile issues related to NO_ACTION_MACRO/FUNCTION and LTO_ENABLE (qmk#8663)
  [Keymap] Userspace update Rgb layers code (qmk#8659)
  Add Choconum (qmk#8709)
  Add Via keymap for BM16-A (qmk#8681)
  Fix edge-case with config
  Don't hide for devs...
  Apply @skullydazed's suggestions, move 'import milc'
  Make dedicated sections for user/dev commands in docs
  Rebase on master, hide some other subcommands
  Use milc for config check, requirements fixes
  CLI: Add development mode support
  Update info.json (qmk#8723)
  format code according to conventions [skip ci]
  spi_master for AVR (qmk#8299)
  ...

# Conflicts:
#	layouts/community/ortho_4x12/grahampheath/keymap.c
#	lib/lufa
loadedsith added a commit to loadedsith/qmk_firmware that referenced this pull request Apr 9, 2020
@loadedsith
Merge branch 'master' into grahampheath-iris
c625827 
* master: (208 commits)
  [Keymap] Update personal userspace and keymaps (qmk#8747)
  Add PS2_MOUSE_ROTATE to compensate for device orientation (qmk#8650)
  Add RGB support in via to launchpad (qmk#8621)
  VIA support for the KBDFans KBD6x (qmk#8680)
  Set the correct RGB LED count on YD60MQ (qmk#8629)
  [Keymap] Updates to personal keymaps (qmk#8665)
  Fix compile issues related to NO_ACTION_MACRO/FUNCTION and LTO_ENABLE (qmk#8663)
  [Keymap] Userspace update Rgb layers code (qmk#8659)
  Add Choconum (qmk#8709)
  Add Via keymap for BM16-A (qmk#8681)
  Fix edge-case with config
  Don't hide for devs...
  Apply @skullydazed's suggestions, move 'import milc'
  Make dedicated sections for user/dev commands in docs
  Rebase on master, hide some other subcommands
  Use milc for config check, requirements fixes
  CLI: Add development mode support
  Update info.json (qmk#8723)
  format code according to conventions [skip ci]
  spi_master for AVR (qmk#8299)
  ...
loadedsith added a commit to loadedsith/qmk_firmware that referenced this pull request Apr 9, 2020
@loadedsith
Merge branch 'master' into mygithub/launchpad
497cef9 
* master: (208 commits)
  [Keymap] Update personal userspace and keymaps (qmk#8747)
  Add PS2_MOUSE_ROTATE to compensate for device orientation (qmk#8650)
  Add RGB support in via to launchpad (qmk#8621)
  VIA support for the KBDFans KBD6x (qmk#8680)
  Set the correct RGB LED count on YD60MQ (qmk#8629)
  [Keymap] Updates to personal keymaps (qmk#8665)
  Fix compile issues related to NO_ACTION_MACRO/FUNCTION and LTO_ENABLE (qmk#8663)
  [Keymap] Userspace update Rgb layers code (qmk#8659)
  Add Choconum (qmk#8709)
  Add Via keymap for BM16-A (qmk#8681)
  Fix edge-case with config
  Don't hide for devs...
  Apply @skullydazed's suggestions, move 'import milc'
  Make dedicated sections for user/dev commands in docs
  Rebase on master, hide some other subcommands
  Use milc for config check, requirements fixes
  CLI: Add development mode support
  Update info.json (qmk#8723)
  format code according to conventions [skip ci]
  spi_master for AVR (qmk#8299)
  ...
@zvecr zvecr mentioned this pull request Apr 12, 2020
Merged
13 tasks
violet-fish pushed a commit to violet-fish/qmk_firmware that referenced this pull request Apr 12, 2020
@vomindoraan @violet-fish
Set the correct RGB LED count on YD60MQ (qmk#8629)
d415972 
* Fix RGB LED count on YD60MQ

* Split YD60MQ into 12-LED and 16-LED revisions

* Update readmes

* Make 12led the default version

* Readd base rules.mk, version→variant in readme

* Add syntax highlighting to code blocks in readme
@sigprof sigprof mentioned this pull request Apr 12, 2020
Merged
13 tasks
Quarren42 pushed a commit to Quarren42/qmk_firmware that referenced this pull request Apr 15, 2020
@vomindoraan @Quarren42
Set the correct RGB LED count on YD60MQ (qmk#8629)
a91c801 
* Fix RGB LED count on YD60MQ

* Split YD60MQ into 12-LED and 16-LED revisions

* Update readmes

* Make 12led the default version

* Readd base rules.mk, version→variant in readme

* Add syntax highlighting to code blocks in readme
kylekuj pushed a commit to kylekuj/qmk_firmware that referenced this pull request Apr 21, 2020
@vomindoraan @kujawakk
Set the correct RGB LED count on YD60MQ (qmk#8629)
2862d26 
* Fix RGB LED count on YD60MQ

* Split YD60MQ into 12-LED and 16-LED revisions

* Update readmes

* Make 12led the default version

* Readd base rules.mk, version→variant in readme

* Add syntax highlighting to code blocks in readme
@vomindoraan vomindoraan deleted the yd60mq_led_fix branch May 7, 2020 14:19
bitherder pushed a commit to bitherder/qmk_firmware that referenced this pull request May 15, 2020
@vomindoraan @lorenzoernesto
Set the correct RGB LED count on YD60MQ (qmk#8629)
2984954 
* Fix RGB LED count on YD60MQ

* Split YD60MQ into 12-LED and 16-LED revisions

* Update readmes

* Make 12led the default version

* Readd base rules.mk, version→variant in readme

* Add syntax highlighting to code blocks in readme
sowbug pushed a commit to sowbug/qmk_firmware that referenced this pull request May 24, 2020
@vomindoraan @sowbug
Set the correct RGB LED count on YD60MQ (qmk#8629)
7d5a9c4 
* Fix RGB LED count on YD60MQ

* Split YD60MQ into 12-LED and 16-LED revisions

* Update readmes

* Make 12led the default version

* Readd base rules.mk, version→variant in readme

* Add syntax highlighting to code blocks in readme
turky pushed a commit to turky/qmk_firmware that referenced this pull request Jun 13, 2020
@vomindoraan @turky
Set the correct RGB LED count on YD60MQ (qmk#8629)
8591504 
* Fix RGB LED count on YD60MQ

* Split YD60MQ into 12-LED and 16-LED revisions

* Update readmes

* Make 12led the default version

* Readd base rules.mk, version→variant in readme

* Add syntax highlighting to code blocks in readme
jakobaa pushed a commit to jakobaa/qmk_firmware that referenced this pull request Jul 7, 2020
@vomindoraan @jakobaa
Set the correct RGB LED count on YD60MQ (qmk#8629)
fc3d687 
* Fix RGB LED count on YD60MQ

* Split YD60MQ into 12-LED and 16-LED revisions

* Update readmes

* Make 12led the default version

* Readd base rules.mk, version→variant in readme

* Add syntax highlighting to code blocks in readme
thorstenweber83 pushed a commit to thorstenweber83/qmk_firmware that referenced this pull request Sep 2, 2020
@vomindoraan @thorstenweber83
Set the correct RGB LED count on YD60MQ (qmk#8629)
119005e 
* Fix RGB LED count on YD60MQ

* Split YD60MQ into 12-LED and 16-LED revisions

* Update readmes

* Make 12led the default version

* Readd base rules.mk, version→variant in readme

* Add syntax highlighting to code blocks in readme
@rbange rbange mentioned this pull request Sep 14, 2020
Closed
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@noroadsleft noroadsleft noroadsleft approved these changes

@Erovia Erovia Erovia approved these changes

@zvecr zvecr Awaiting requested review from zvecr

Assignees
No one assigned
Labels
bug keyboard
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@vomindoraan @drashna @noroadsleft @Erovia @zvecr @mtei