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

Add rk750 keyboard #24048

Closed
wants to merge 3 commits into from
Closed

Add rk750 keyboard #24048

wants to merge 3 commits into from

Conversation

yangzheng20003
Copy link

@yangzheng20003 yangzheng20003 commented Jul 4, 2024 •
edited
Loading

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

@github-actions github-actions bot added keyboard keymap via Adds via keymap and/or updates keyboard for via support labels Jul 4, 2024
zvecr
zvecr requested changes Jul 4, 2024
View reviewed changes
keyboards/rk/rk750/config.h Outdated
Copy link
Member

Choose a reason for hiding this comment

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

Please remove this file

keyboards/rk/rk750/keyboard.json
"mousekey": true,
"nkro": true,
"rgb_matrix": true
},
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
},
},
"dynamic_keymap": {
"layer_count": 3
},

waffle87
waffle87 reviewed Jul 4, 2024
View reviewed changes
keyboards/rk/rk750/rk750.c
Comment on lines +5 to +8
#define LED_ENABLE_PIN A5
void keyboard_pre_init_kb(void) {
gpio_set_pin_output(LED_ENABLE_PIN);
gpio_write_pin_high(LED_ENABLE_PIN);
Copy link
Member

@waffle87 waffle87 Jul 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

What exactly is this for? All the results I'm seeing for this keyboard on Google indicate that this is a wireless keyboard. If this is initialisation code for a wireless module, this PR will not be accepted without the accompanying wireless code.
Edit: https://device.report/m/8886e15ae57293b1e6ddf52174c52b050531b390232629370dbc807a0d5722b1

166-75001-06 RK750-...

The PDF linked here appears to show this is a wireless keyboard.

@tzarc
Copy link
Member

tzarc commented Jul 10, 2024

This PR is on hold until #24085 is rectified.

@tzarc tzarc marked this pull request as draft July 10, 2024 07:52
@tzarc
Copy link
Member

tzarc commented Aug 26, 2024

As of August 26, 2024, qmk/qmk_firmware is no longer accepting VIA-enabled keymaps as these have now transitioned to a repository under the VIA team's control.

As you've submitted a PR containing via or VIA-enabled keymap(s), this is your notice that they should be removed from this PR. You should now submit a secondary PR to the VIA QMK Userspace repository with your associated via or VIA-enabled keymaps instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@waffle87 waffle87 waffle87 left review comments

@zvecr zvecr zvecr requested changes

Assignees
No one assigned
Labels
keyboard keymap needs-wireless-code via Adds via keymap and/or updates keyboard for via support
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@yangzheng20003 @tzarc @zvecr @waffle87