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

Enable Support for more than 1 IS31FL3733 #9743

Closed
wants to merge 3 commits into from

Conversation

Xelus22
Copy link
Contributor

@Xelus22 Xelus22 commented Jul 17, 2020

Description

Enable a second IS31FL3733 in rgb_matrix as it is mostly already supported elsewhere in the code base.

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

  • Enable second IS31FL3733

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

Copy link
Member

@fauxpark fauxpark left a comment

Choose a reason for hiding this comment

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

The DZtech boards have only one 3733 and DRIVER_ADDR_2 is set to the same value as DRIVER_LED_1, I assume because it doesn't compile without it being defined, even if it isn't used. I wonder if that breaks anything, and whether we should add checks to only run stuff for driver 2 when DRIVER_COUNT == 2.

@fauxpark
Copy link
Member

Also, this should probably target develop.

@Xelus22 Xelus22 changed the base branch from master to develop July 17, 2020 10:10
@Xelus22
Copy link
Contributor Author

Xelus22 commented Jul 17, 2020

The DZtech boards have only one 3733 and DRIVER_ADDR_2 is set to the same value as DRIVER_LED_1, I assume because it doesn't compile without it being defined, even if it isn't used. I wonder if that breaks anything, and whether we should add checks to only run stuff for driver 2 when DRIVER_COUNT == 2.

I would assume we would need checks for all our supported drivers then. i.e. 3731, 3733, 3736, 3737 and 3741

@drashna
Copy link
Member

drashna commented Oct 11, 2020

For reference (non-comprehensive), the boards that use the 3733 controller are:

  • the DZTech boards, all of them.
  • Exclusive e6_rgb
  • hs60 v2 * uses WilbaTech LED
  • k_type (ignore, is abomination that uses i2c1 and i2c2), lists different addresses
  • kbdfans/kbdmini
  • (mass)drop(ped) boards
  • miller/gm862
  • nebula68 * uses WilbaTech LED
  • nk65 * uses WilbaTech LED
  • wilba tech boards, or anything that uses their per key rgb (not RGB Matrix feature).

So the only boards that would need verification/change are the DZTech boards, the e6_rgb, (probably the K_TYPE), the kbdmini, and the gm862. The drop boards .... not sure

@stale
Copy link

stale bot commented Jan 15, 2021

Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

@stale
Copy link

stale bot commented Feb 14, 2021

Thank you for your contribution!
This pull request has been automatically closed because it has not had activity in the last 30 days. Please feel free to give a status update now, ping for review, or re-open when it's ready.

@stale stale bot closed this Feb 14, 2021
@tzarc tzarc reopened this Feb 14, 2021
@tzarc tzarc closed this Feb 27, 2021
@tzarc tzarc deleted the branch qmk:develop February 27, 2021 20:27
@tzarc
Copy link
Member

tzarc commented Feb 27, 2021

Sorry about that, GitHub decided to delete the develop branch from upstream when we merged it, despite being told otherwise. Reopened.

@tzarc tzarc reopened this Feb 27, 2021
@stale stale bot removed the awaiting changes label Feb 27, 2021
@drashna
Copy link
Member

drashna commented Mar 14, 2021

got a small merge conflict here.

@Xelus22
Copy link
Contributor Author

Xelus22 commented May 17, 2021

Closing in favour of #12342

@Xelus22 Xelus22 closed this May 17, 2021
@Xelus22 Xelus22 deleted the is31fl3733_update branch June 2, 2021 11:39
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.

None yet

5 participants