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 k8 (rgb) converted to info.json style #291

Closed
wants to merge 0 commits into from
Closed

Add k8 (rgb) converted to info.json style #291

wants to merge 0 commits into from

Conversation

tbowmo
Copy link

@tbowmo tbowmo commented Oct 22, 2022 •
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).

@tbowmo
Copy link
Author

tbowmo commented Oct 22, 2022

Tested with K8 rgb ISO variant.

All other variants is verified by a small python test routine comparing the autogenerated mapping macro, with the one in the original header files (from old master)

@davecarrijo
Copy link

Is via working?

@tbowmo
Copy link
Author

tbowmo commented Oct 24, 2022 •
edited
Loading

Should be, but haven't tested yet. I only did a quick test of plain iso layout, which worked as expected.

@davecarrijo
Copy link

I have the k8 ansi variant, if you need a tester I will gladly help

@tbowmo
Copy link
Author

tbowmo commented Oct 25, 2022

Via is now verified as working (iso variant). Also updated default via keymap to be the same as windows iso layout.

dexter93
dexter93 reviewed Nov 2, 2022
View reviewed changes
keyboards/keychron/k8/rgb/ansi/config.h Outdated
*/

// RGB Matrix Config
#define DRIVER_LED_TOTAL 87
Copy link

Choose a reason for hiding this comment

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

these can now be 1 define

dexter93
dexter93 reviewed Nov 2, 2022
View reviewed changes
keyboards/keychron/k8/rgb/optical_iso/rules.mk Outdated
@@ -0,0 +1 @@
OPT_DEFS += -DOPTICAL_MATRIX
Copy link

Choose a reason for hiding this comment

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

need to figure out what will happen with this. we need something that works against the main qmk scan now

Copy link
Author

Choose a reason for hiding this comment

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

I left it here, as I was unsure and do not have an optical keyboard..

Copy link

Choose a reason for hiding this comment

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

Probably going to go with #299 once it gets the green light from qmk

Copy link

Choose a reason for hiding this comment

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

this has now been updated in qmk#19053
code is in upstream now, see the PR for usage refs

dexter93
dexter93 reviewed Nov 2, 2022
View reviewed changes
keyboards/keychron/k8/rgb/info.json Outdated
"dip_switch_enable": true
},
"rgb_matrix": {
"driver": "SN32F24xB"
Copy link

Choose a reason for hiding this comment

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

does this work?

Copy link
Author

Choose a reason for hiding this comment

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

Yup, it does show up in the autogenerated rules.mk file:
RGB_MATRIX_ENABLE ?= yes

Copy link

Choose a reason for hiding this comment

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

somehow it doesn't get picked up here. weird

Copy link

Choose a reason for hiding this comment

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

obj_keychron_k8_rgb_iso/src/info_rules.mk:RGB_MATRIX_ENABLE ?= yes

@dexter93 dexter93 force-pushed the sn32_develop branch 2 times, most recently from ce772b4 to 3323dfd Compare November 5, 2022 09:26
@github-actions github-actions bot added cli core dependencies documentation Improvements or additions to documentation python and removed python core cli dependencies documentation Improvements or additions to documentation labels Nov 6, 2022
@davecarrijo
Copy link

so... with this we can use VIA now?

@tbowmo
Copy link
Author

tbowmo commented Nov 7, 2022

@davecarrijo seems that via is temporarily broken in this PR (it worked initially, but a lot has changed since then)

@davecarrijo
Copy link

@davecarrijo seems that via is temporarily broken in this PR (it worked initially, but a lot has changed since then)

Did you changed the pid and the vid? What it changes?

@tbowmo
Copy link
Author

tbowmo commented Nov 10, 2022

I changed pid to the correct one for the iso variants, which also happens to fix via support :)

@vware
Copy link

vware commented Dec 15, 2022

this is looking quite nice actually, and it is the basis for the bluetooth pr, any chance this will get merged soonish? ;)

@davecarrijo
Copy link

I changed pid to the correct one for the iso variants, which also happens to fix via support :)

The man !!!🫡

@davecarrijo
Copy link

this is looking quite nice actually, and it is the basis for the bluetooth pr, any chance this will get merged soonish? ;)

That would made it just one of the best keyboards out there.

davecarrijo
davecarrijo reviewed Jan 25, 2023
View reviewed changes
Copy link

@davecarrijo davecarrijo left a comment

Choose a reason for hiding this comment

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

nice

dexter93
dexter93 reviewed Jan 26, 2023
View reviewed changes
.gitmodules Outdated
@@ -4,8 +4,8 @@
branch = master
[submodule "lib/chibios-contrib"]
path = lib/chibios-contrib
url = https://github.com/qmk/ChibiOS-Contrib
branch = master
url = https://github.com/SonixQMK/ChibiOS-Contrib
Copy link

Choose a reason for hiding this comment

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

no longer required. please remove modifications on .gitmodules

dexter93
dexter93 requested changes Jan 26, 2023
View reviewed changes
Copy link

@dexter93 dexter93 left a comment

Choose a reason for hiding this comment

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

suggestions for via v3 lighting

keyboards/keychron/k8/rgb/keymaps/via/k8_via_ansi.json Outdated
Comment on lines 5 to 184
"CYCLE_SPIRAL",
1
],
[
"DUAL_BEACON",
1
],
[
"RAINBOW_BEACON",
1
],
[
"RAINBOW_PINWHEELS",
1
],
[
"RAINDROPS",
1
],
[
"JELLYBEAN_RAINDROPS",
1
],
[
"HUE_BREATHING",
1
],
[
"HUE_PENDULUM",
1
],
[
"HUE_WAVE",
1
],
["PIXEL_FRACTAL", 1],
["PIXEL_RAIN", 1],
[
"TYPING_HEATMAP",
1
],
[
"DIGITAL_RAIN",
1
],
[
"SOLID_REACTIVE_SIMPLE",
1
],
[
"SOLID_REACTIVE",
1
],
[
"SOLID_REACTIVE_WIDE",
1
],
[
"SOLID_REACTIVE_MULTIWIDE",
1
],
[
"SOLID_REACTIVE_CROSS",
1
],
[
"SOLID_REACTIVE_MULTICROSS",
1
],
[
"SOLID_REACTIVE_NEXUS",
1
],
[
"SOLID_REACTIVE_MULTINEXUS",
1
],
[
"SPLASH",
1
],
[
"MULTISPLASH",
1
],
[
"SOLID_SPLASH",
1
],
[
"SOLID_MULTISPLASH",
1
]
],
"supportedLightingValues": [
128,
129,
131
]
},
Copy link

Choose a reason for hiding this comment

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

Suggested change
"lighting": {
"extends": "qmk_rgblight",
"underglowEffects": [
[
"None",
0
],
[
"SOLID_COLOR",
1
],
[
"ALPHAS_MODS",
1
],
[
"GRADIENT_UP_DOWN",
1
],
[
"GRADIENT_LEFT_RIGHT",
1
],
[
"BREATHING",
1
],
[
"BAND_SAT",
1
],
[
"BAND_VAL",
1
],
[
"BAND_PINWHEEL_SAT",
1
],
[
"BAND_PINWHEEL_VAL",
1
],
[
"BAND_SPIRAL_SAT",
1
],
[
"BAND_SPIRAL_VAL",
1
],
[
"CYCLE_ALL",
1
],
[
"CYCLE_LEFT_RIGHT",
1
],
[
"CYCLE_UP_DOWN",
1
],
[
"CYCLE_OUT_IN",
1
],
[
"CYCLE_OUT_IN_DUAL",
1
],
[
"RAINBOW_MOVING_CHEVRON",
1
],
[
"CYCLE_PINWHEEL",
1
],
[
"CYCLE_SPIRAL",
1
],
[
"DUAL_BEACON",
1
],
[
"RAINBOW_BEACON",
1
],
[
"RAINBOW_PINWHEELS",
1
],
[
"RAINDROPS",
1
],
[
"JELLYBEAN_RAINDROPS",
1
],
[
"HUE_BREATHING",
1
],
[
"HUE_PENDULUM",
1
],
[
"HUE_WAVE",
1
],
["PIXEL_FRACTAL", 1],
["PIXEL_RAIN", 1],
[
"TYPING_HEATMAP",
1
],
[
"DIGITAL_RAIN",
1
],
[
"SOLID_REACTIVE_SIMPLE",
1
],
[
"SOLID_REACTIVE",
1
],
[
"SOLID_REACTIVE_WIDE",
1
],
[
"SOLID_REACTIVE_MULTIWIDE",
1
],
[
"SOLID_REACTIVE_CROSS",
1
],
[
"SOLID_REACTIVE_MULTICROSS",
1
],
[
"SOLID_REACTIVE_NEXUS",
1
],
[
"SOLID_REACTIVE_MULTINEXUS",
1
],
[
"SPLASH",
1
],
[
"MULTISPLASH",
1
],
[
"SOLID_SPLASH",
1
],
[
"SOLID_MULTISPLASH",
1
]
],
"supportedLightingValues": [
128,
129,
131
]
},
"keycodes": ["qmk_lighting"],
"menus": ["qmk_rgb_matrix"],

This will make it work with via v3 defs

keyboards/keychron/k8/rgb/keymaps/via/k8_via_ansi_optical.json Outdated
Comment on lines 5 to 184
"CYCLE_SPIRAL",
1
],
[
"DUAL_BEACON",
1
],
[
"RAINBOW_BEACON",
1
],
[
"RAINBOW_PINWHEELS",
1
],
[
"RAINDROPS",
1
],
[
"JELLYBEAN_RAINDROPS",
1
],
[
"HUE_BREATHING",
1
],
[
"HUE_PENDULUM",
1
],
[
"HUE_WAVE",
1
],
["PIXEL_FRACTAL", 1],
["PIXEL_RAIN", 1],
[
"TYPING_HEATMAP",
1
],
[
"DIGITAL_RAIN",
1
],
[
"SOLID_REACTIVE_SIMPLE",
1
],
[
"SOLID_REACTIVE",
1
],
[
"SOLID_REACTIVE_WIDE",
1
],
[
"SOLID_REACTIVE_MULTIWIDE",
1
],
[
"SOLID_REACTIVE_CROSS",
1
],
[
"SOLID_REACTIVE_MULTICROSS",
1
],
[
"SOLID_REACTIVE_NEXUS",
1
],
[
"SOLID_REACTIVE_MULTINEXUS",
1
],
[
"SPLASH",
1
],
[
"MULTISPLASH",
1
],
[
"SOLID_SPLASH",
1
],
[
"SOLID_MULTISPLASH",
1
]
],
"supportedLightingValues": [
128,
129,
131
]
},
Copy link

Choose a reason for hiding this comment

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

Suggested change
"lighting": {
"extends": "qmk_rgblight",
"underglowEffects": [
[
"None",
0
],
[
"SOLID_COLOR",
1
],
[
"ALPHAS_MODS",
1
],
[
"GRADIENT_UP_DOWN",
1
],
[
"GRADIENT_LEFT_RIGHT",
1
],
[
"BREATHING",
1
],
[
"BAND_SAT",
1
],
[
"BAND_VAL",
1
],
[
"BAND_PINWHEEL_SAT",
1
],
[
"BAND_PINWHEEL_VAL",
1
],
[
"BAND_SPIRAL_SAT",
1
],
[
"BAND_SPIRAL_VAL",
1
],
[
"CYCLE_ALL",
1
],
[
"CYCLE_LEFT_RIGHT",
1
],
[
"CYCLE_UP_DOWN",
1
],
[
"CYCLE_OUT_IN",
1
],
[
"CYCLE_OUT_IN_DUAL",
1
],
[
"RAINBOW_MOVING_CHEVRON",
1
],
[
"CYCLE_PINWHEEL",
1
],
[
"CYCLE_SPIRAL",
1
],
[
"DUAL_BEACON",
1
],
[
"RAINBOW_BEACON",
1
],
[
"RAINBOW_PINWHEELS",
1
],
[
"RAINDROPS",
1
],
[
"JELLYBEAN_RAINDROPS",
1
],
[
"HUE_BREATHING",
1
],
[
"HUE_PENDULUM",
1
],
[
"HUE_WAVE",
1
],
["PIXEL_FRACTAL", 1],
["PIXEL_RAIN", 1],
[
"TYPING_HEATMAP",
1
],
[
"DIGITAL_RAIN",
1
],
[
"SOLID_REACTIVE_SIMPLE",
1
],
[
"SOLID_REACTIVE",
1
],
[
"SOLID_REACTIVE_WIDE",
1
],
[
"SOLID_REACTIVE_MULTIWIDE",
1
],
[
"SOLID_REACTIVE_CROSS",
1
],
[
"SOLID_REACTIVE_MULTICROSS",
1
],
[
"SOLID_REACTIVE_NEXUS",
1
],
[
"SOLID_REACTIVE_MULTINEXUS",
1
],
[
"SPLASH",
1
],
[
"MULTISPLASH",
1
],
[
"SOLID_SPLASH",
1
],
[
"SOLID_MULTISPLASH",
1
]
],
"supportedLightingValues": [
128,
129,
131
]
},
"keycodes": ["qmk_lighting"],
"menus": ["qmk_rgb_matrix"],

This will make it work with via v3 defs

@tbowmo tbowmo closed this Jan 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davecarrijo davecarrijo davecarrijo left review comments

@vware vware vware left review comments

@dexter93 dexter93 dexter93 requested changes

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

Successfully merging this pull request may close these issues.
4 participants
@tbowmo @davecarrijo @vware @dexter93