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

Add KPRepublic/BM40hsrgb rev2 #16689

Merged
merged 37 commits into from
Sep 21, 2023
Merged

Add KPRepublic/BM40hsrgb rev2 #16689

merged 37 commits into from
Sep 21, 2023

Conversation

rgoulter
Copy link
Contributor

@rgoulter rgoulter commented Mar 19, 2022

For KPRepublic's BM40 rev2.

The code in this PR is similar to #15060 (@peepeetee's PR for KPRepublic's BM65 rev2) and #15177 (peepeetee's PR on behalf of bdtc123 for BM60 rev2). -- And especially similar to #16452 (which also aims to add KPRepublic BM40 rev2), except this PR copies the BM65 rev2's custom RGB matrix driver for the IS31FL (used for per-key RGB) and WS2821B (used for underglow RGB).

This is a breaking change.

Instead of:

qmk compile --keyboard kprepublic/bm40hsrgb --keymap defaultr

The older rev1 PCBs now have /rev1 added; thus should be built with:

qmk compile --keyboard kprepublic/bm40hsrgb/rev2 --keymap default

(fyi people with bm40 keymaps: @gabustoledo
gabustoledo, @lmlask
lmlask).

Description

  • Move kprepublic/bm40hsrgb code to rev1 directory.
  • Add code for the BM40 rev2 (copy-pasting the custom RGB matrix driver IS31FL3733 + WS2812 from aforementioned PRs).

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

@github-actions github-actions bot added keyboard keymap via Adds via keymap and/or updates keyboard for via support labels Mar 19, 2022
@rgoulter rgoulter changed the title Bm40hsrgb rev2 Add KPRepublic/BM40hsrgb rev2 Mar 19, 2022
@drashna drashna requested a review from a team March 19, 2022 15:13
@peepeetee
Copy link
Contributor

Question:
Is this a duplicate of #16452?

@rgoulter
Copy link
Contributor Author

rgoulter commented Mar 20, 2022

@peepeetee it is! I'd searched the github issues for "BM40" and "kprepublic" and couldn't see a PR for it 😅 :

search

("bm40hsrgb" would've found it).

The code is a bit different. This PR handles the WS2812 underglow LEDs whereas #16452 does not uses the underglow LEDs with RGB Light instead of RGB Matrix.

keyboards/kprepublic/bm40hsrgb/rev1/readme.md Outdated Show resolved Hide resolved
keyboards/kprepublic/bm40hsrgb/rev2/readme.md Outdated Show resolved Hide resolved
@rgoulter rgoulter mentioned this pull request Apr 8, 2022
14 tasks
keyboards/kprepublic/bm40hsrgb/readme.md Outdated Show resolved Hide resolved
keyboards/kprepublic/bm40hsrgb/rev1/readme.md Outdated Show resolved Hide resolved
@drashna drashna requested a review from a team April 18, 2022 07:51
@spider9375
Copy link

Is there anything that prevents this PR from merging?

@SyreeseOfficial
Copy link

Hello all! I'm trying to build a BM40 build. My PCB is the rev2 model. I've never built a keeb or flashed a keeb until this (my first attempt). I'm also completely new to GitHub. Is there any way I can use this info to get my keyboard working? All the keys are wrong and most of them don't work. Thanks for any help. Pls lmk if there's any other info I can provide as well. I'll also link to my reddit post asking for help that has some images attached along with more info on the problem (HERE).

@rgoulter
Copy link
Contributor Author

@SyreeseOfficial you'll probably get quicker help asking on discord.

For getting your qmk_firmware to have the code in this PR, this comment is useful

LAYOUTS = ortho_4x12 planck_mit

which gives the following git commands to run in qmk_firmware:

git fetch origin pull/<PR_NUMBER>/head:review/<PR_NUMBER>
git checkout review/<PR_NUMBER>
git rebase develop

@drashna drashna requested a review from a team May 20, 2022 04:41
@hienvd
Copy link

hienvd commented May 21, 2022

Any new updates guys?
I'm waiting for this PR to configure my BM40 rev2

@rafael-malmegrin
Copy link

Any new updates guys? I'm waiting for this PR to configure my BM40 rev2

I managed to flash my BM40v2 with the files from this pull, if you have a local QMK environment setup you can do it. I got a warning saying the firmware had almost no bytes left but it worked, I was able to duplicate and edit the default keymap and flash it, it's working fine on both of my v2 pcb's.

@rgoulter
Copy link
Contributor Author

I got a warning saying the firmware had almost no bytes left but it worked

Adding

LTO_ENABLE = yes

to rules.mk in the keymap helps. https://docs.qmk.fm/#/squeezing_avr

@rafael-malmegrin
Copy link

LTO_ENABLE = yes

That did help, compiled my keymap again and did not get the warning, thanks.

@seth-rah
Copy link

Just to understand the system

The outdated change requests, do they require approval before the PR can be merged or is it just a case of needing enough approvals?

As one has been there for more than a year and might just be a blocker if those rules exist.

@drashna drashna removed the stale Issues or pull requests that have become inactive without resolution. label Aug 29, 2023
Co-authored-by: Drashna Jaelre <drashna@live.com>
@rgoulter
Copy link
Contributor Author

rgoulter commented Sep 2, 2023

I see that PRs targetting develop are merged again. I think that was the last thing blocking this PR?

@fauxpark
Copy link
Member

fauxpark commented Sep 2, 2023

kprepublic/bm40hsrgb/rev2:via seems to be failing CI. User keymaps are no issue, but the default and via keymaps at least must compile, in order to merge.

@MattNemeth
Copy link

MattNemeth commented Sep 3, 2023

I don't currently have another keyboard to test this but I have experienced a bug when combining TT(layer x) + TO(layer y). I imagine this is not specific to this PR but cannot confirm and will wait until this is merged before opening an issue on the main qmk repo. The issue is as follows:

  1. Hold TT(layer x) key
  2. When on layer x, tap a TO(layer y) key
  3. Resulting functionality is activating layer x (as if it was pressed enough times to enable the tap toggle)

When layer x is toggled on, TO(layer y) works as expected. Using MO(layer y) instead of TT(layer y) works as expected and goes to layer y.

@rgoulter
Copy link
Contributor Author

rgoulter commented Sep 4, 2023

I see the QMK CI build finished, I see that default and via keymaps are passing for kprepublic/bm40hsrgb/rev2.

@MattNemeth
Copy link

If anyone else was struggling to find up to date info on how to get firmware built and working for their new BM40 Rev 2 using QMK this is the way to do it as of the time of this comment. Hopefully this will be merged soon and be on the official repo soon but if it still isn't, use this PR. Lots of outdated info across the web about this board over the past few years with broken links. Hopefully this comment will save someone the hours I spent trying to figure out how to flash this "QMK Fully Compatible" board as they claim on KPRepublic...

@dornyika
Copy link

If anyone else was struggling to find up to date info on how to get firmware built and working for their new BM40 Rev 2 using QMK this is the way to do it as of the time of this comment. Hopefully this will be merged soon and be on the official repo soon but if it still isn't, use this PR. Lots of outdated info across the web about this board over the past few years with broken links. Hopefully this comment will save someone the hours I spent trying to figure out how to flash this "QMK Fully Compatible" board as they claim on KPRepublic...

Sorry I'm missing something. How do I suppose to do it?

@seth-rah
Copy link

@waffle87 & @zvecr

Does this MR require your approval to move forward? As the old comments appear to be outdated

@MattNemeth
Copy link

@dornyika if you havent got it working yet you'll need to clone the original QMK repo and checkout this pull request. You can make/modify/create your own keymap starting with the default and get it working this way

@drashna drashna merged commit 629ac9a into qmk:develop Sep 21, 2023
@rgoulter
Copy link
Contributor Author

🎉

@tristanware
Copy link

Huge thanks to @rgoulter for all the effort!

mechlovin pushed a commit to mechlovin/qmk_firmware that referenced this pull request Oct 25, 2023
Co-authored-by: Drashna Jaelre <drashna@live.com>
Co-authored-by: Ryan <fauxpark@gmail.com>
Co-authored-by: Joel Challis <git@zvecr.com>
Co-authored-by: jack <0x6a73@protonmail.com>
christrotter pushed a commit to christrotter/qmk_firmware that referenced this pull request Nov 28, 2023
Co-authored-by: Drashna Jaelre <drashna@live.com>
Co-authored-by: Ryan <fauxpark@gmail.com>
Co-authored-by: Joel Challis <git@zvecr.com>
Co-authored-by: jack <0x6a73@protonmail.com>
zgagnon pushed a commit to zgagnon/qmk_firmware_waterfowl that referenced this pull request Dec 15, 2023
Co-authored-by: Drashna Jaelre <drashna@live.com>
Co-authored-by: Ryan <fauxpark@gmail.com>
Co-authored-by: Joel Challis <git@zvecr.com>
Co-authored-by: jack <0x6a73@protonmail.com>
future-figs pushed a commit to future-figs/qmk_firmware that referenced this pull request Dec 27, 2023
Co-authored-by: Drashna Jaelre <drashna@live.com>
Co-authored-by: Ryan <fauxpark@gmail.com>
Co-authored-by: Joel Challis <git@zvecr.com>
Co-authored-by: jack <0x6a73@protonmail.com>
@rgoulter rgoulter deleted the bm40hsrgb_rev2 branch June 18, 2024 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
keyboard keymap via Adds via keymap and/or updates keyboard for via support
Projects
None yet
Development

Successfully merging this pull request may close these issues.