Knob Goblin add via support #11831

mrT1ddl3s · 2021-02-08T22:26:32Z

Description

Added Via support.
Minor OLED code modifications to allow for more supported layers.

Types of Changes

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

New macropad initial commit. Keyboard and keymap

per fauxpark review Co-authored-by: Ryan <fauxpark@gmail.com>

info file fix for wrong display on configurator

fix wrong graphic display on configurator

definitions moved from keymap .c file to keyboard .c file to allow for QMK Configurator to be used without losing encoder and OLED function.

Per drasha review Co-authored-by: Drashna Jaelre <drashna@live.com>

change render_logo to render_goblin_logo add attribute weak to oled_task_user

add space before "One" and "two" so that "T" in three doesn't stay on OLED

add text on OLED for up to 6 layers

IDs updated. Bootmagic lite defined for top left button

keyboards/knobgoblin/config.h

keyboards/knobgoblin/readme.md

Co-authored-by: Joel Challis <git@zvecr.com>

keyboards/knobgoblin/knobgoblin.c

per drashna review

drashna · 2021-02-14T21:13:19Z

Looks like you have some merge conflicts that need to be resolved, here.

mrT1ddl3s · 2021-02-15T15:55:04Z

fixed the merge conflicts

drashna

LGTM.

Thanks!

liyang · 2021-02-16T18:12:25Z

keyboards/knobgoblin/info.json

+				        {"Label": "F17", "x": 1, "y": 0},
+                {"Label": "F18", "x": 2, "y": 0},
+                {"Label": "F19", "x": 3, "y": 0},
+                {"Label": "F20", "x": 4, "y": 0},


Is the wonky indentation intentional? (I think the project indentation policy is 4 space, no hard-tabs.) If not could you filter this through something like underscore print --wrapwidth 72 please? (jq doesn't produce quite as nice-looking output unfortunately.) See https://github.com/ddopson/underscore-cli

liyang · 2021-02-16T18:17:13Z

keyboards/knobgoblin/knobgoblin.c

 	render_goblin_logo();
+
+    oled_set_cursor(0,11);

 	switch (get_highest_layer(layer_state)) {


As with the JSON, but qmk cformat keyboards/knobgoblin/knobgoblin.c will run clang-format for you. (In your keymap.c sections where you want to keep the formatting intact, you can wrap those in /* clang-format off */ and /* clang-format on */.

liyang · 2021-02-16T18:19:02Z

keyboards/knobgoblin/knobgoblin.c

 			break;
 		case 2:
-			oled_set_cursor(0,11);
 			oled_write_P(PSTR("THREE\n"), false);


Should there be a space before THREE here too? (Or is it for quasi centre-alignment on the display?)

liyang · 2021-02-16T18:21:16Z

keyboards/knobgoblin/keymaps/via/keymap.c

+    [0] = LAYOUT_ortho(
+                  KC_EQL, KC_PSLS, KC_PAST, KC_PMNS,
+                  KC_P7,  KC_P8,   KC_P9,   KC_PPLS,
+                  KC_P4,  KC_P5,   KC_P6,   KC_PPLS,
+         KC_MPLY, KC_P1,  KC_P2,   KC_P3,   KC_PENT,
+         KC_MUTE, MO(1),  KC_P0,   KC_PDOT, KC_PENT
+				 ),


I'd wrap these in /* clang-format off */ and /* clang-format on */ just to be safe, so they don't get mangled next time someone decides to run clang-format on the entire codebase. Seems to happen once every few months.

e.g.

[0] = LAYOUT_ortho(/* clang-format off */ KC_EQL, KC_PSLS, KC_PAST, KC_PMNS, KC_P7, KC_P8, KC_P9, KC_PPLS, KC_P4, KC_P5, KC_P6, KC_PPLS, KC_MPLY, KC_P1, KC_P2, KC_P3, KC_PENT, KC_MUTE, MO(1), KC_P0, KC_PDOT, KC_PENT ), /* clang-format on */

liyang · 2021-02-16T18:32:41Z

keyboards/knobgoblin/knobgoblin.c

 	switch (get_highest_layer(layer_state)) {
 		case 0:
-			oled_set_cursor(0,11);
-			oled_advance_char();
-			oled_write_P(PSTR("ONE\n"), false);
+			oled_write_P(PSTR(" ONE\n"), false);
 			break;
 		case 1:
-			oled_set_cursor(0,11);
-			oled_advance_char();
-			oled_write_P(PSTR("TWO\n"), false);
+			oled_write_P(PSTR(" TWO\n"), false);
 			break;
 		case 2:
-			oled_set_cursor(0,11);
 			oled_write_P(PSTR("THREE\n"), false);
 			break;
+		case 3:
+			oled_write_P(PSTR(" FOUR\n"), false);
+			break;
+		case 4:
+			oled_write_P(PSTR(" FIVE\n"), false);
+			break;
+		case 5:
+			oled_write_P(PSTR(" SIX\n"), false);
+			break;
+
 	}


FWIW I'd write this as:

const char PROGMEM* layer_text[] = {" ONE\n", " TWO\n", "THREE\n", " FOUR\n", " FIVE\n", " SIX\n"}; … uint8_t layer = get_highest_layer(layer_state); if(layer < 6) { oled_write_P(layer_text[layer], false); }

Might save a few bytes on the firmware and avoids unnecessary code duplication.

mrT1ddl3s and others added 27 commits January 22, 2021 10:58

Knobgoblin keyboard initial commit

09011af

New macropad initial commit. Keyboard and keymap

corrected bracket and layout name

c41bb8d

attempting info file fix again

2cfcfc1

info file line 33 hanging comma fix

4dd992e

Update keyboards/knobgoblin/config.h

b338586

per fauxpark review Co-authored-by: Ryan <fauxpark@gmail.com>

Update keyboards/knobgoblin/readme.md

3f65230

per fauxpark review Co-authored-by: Ryan <fauxpark@gmail.com>

Update keyboards/knobgoblin/readme.md

bbcd024

per fauxpark review Co-authored-by: Ryan <fauxpark@gmail.com>

Merge remote-tracking branch 'upstream/master' into knobgoblin_dev

d6f4eee

updated to include the two encoder push buttons

8c99221

info file fix for wrong display on configurator

info file fix to add encoder push buttons

fbcce1c

fix wrong graphic display on configurator

move encoder and oled definition

ceed454

definitions moved from keymap .c file to keyboard .c file to allow for QMK Configurator to be used without losing encoder and OLED function.

Update keyboards/knobgoblin/info.json

649879d

Per drasha review Co-authored-by: Drashna Jaelre <drashna@live.com>

Update keymap.c

ef8b36a

Delete keymap.c

9a55da1

update per erovia comment

96d890d

change render_logo to render_goblin_logo add attribute weak to oled_task_user

Update readme.md

13394f0

Update readme.md

35be2a7

fix layer text display

2480720

add space before "One" and "two" so that "T" in three doesn't stay on OLED

add more available layers to OLED display

12de699

add text on OLED for up to 6 layers

update IDs for Via

15d7b58

IDs updated. Bootmagic lite defined for top left button

add labels

e04c6b4

add Knob Goblin via keymap files

d72162b

bootmagic lite enabled

1b32ed2

add via keymap files

d7aa296

Update readme.md

40ee136

Delete keyboards/knobgoblin/via directory

d194a7c

Add files via upload

dca9f9d

github-actions bot added keyboard keymap via Adds via keymap and/or updates keyboard for via support labels Feb 8, 2021

zvecr requested changes Feb 9, 2021

View reviewed changes

keyboards/knobgoblin/config.h Outdated Show resolved Hide resolved

keyboards/knobgoblin/readme.md Outdated Show resolved Hide resolved

mrT1ddl3s and others added 2 commits February 9, 2021 09:16

Apply suggestions from zvecr review

20e761f

Co-authored-by: Joel Challis <git@zvecr.com>

Update info.json

0de45d8

mrT1ddl3s requested a review from zvecr February 11, 2021 19:20

drashna reviewed Feb 13, 2021

View reviewed changes

keyboards/knobgoblin/knobgoblin.c Outdated Show resolved Hide resolved

move oled set cursor out of switch

7af968c

per drashna review

mrT1ddl3s requested a review from drashna February 13, 2021 16:22

Merge branch 'master' into knobgoblin_dev

2475061

zvecr approved these changes Feb 16, 2021

View reviewed changes

zvecr requested a review from a team February 16, 2021 14:39

drashna approved these changes Feb 16, 2021

View reviewed changes

drashna merged commit 9a2b0a5 into qmk:master Feb 16, 2021

mrT1ddl3s deleted the knobgoblin_dev branch February 16, 2021 17:58

liyang reviewed Feb 16, 2021

View reviewed changes

This was referenced Feb 16, 2021

initial knob goblin json via commit the-via/keyboards#577

Closed

knob goblin json via commit. renamed file the-via/keyboards#578

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Knob Goblin add via support #11831

Knob Goblin add via support #11831

mrT1ddl3s commented Feb 8, 2021 •

edited

Loading

drashna commented Feb 14, 2021

mrT1ddl3s commented Feb 15, 2021

drashna left a comment

liyang Feb 16, 2021

liyang Feb 16, 2021

liyang Feb 16, 2021

liyang Feb 16, 2021

liyang Feb 16, 2021

liyang Feb 16, 2021

Knob Goblin add via support #11831

Knob Goblin add via support #11831

Conversation

mrT1ddl3s commented Feb 8, 2021 • edited Loading

Description

Types of Changes

Checklist

drashna commented Feb 14, 2021

mrT1ddl3s commented Feb 15, 2021

drashna left a comment

Choose a reason for hiding this comment

liyang Feb 16, 2021

Choose a reason for hiding this comment

liyang Feb 16, 2021

Choose a reason for hiding this comment

liyang Feb 16, 2021

Choose a reason for hiding this comment

liyang Feb 16, 2021

Choose a reason for hiding this comment

liyang Feb 16, 2021

Choose a reason for hiding this comment

liyang Feb 16, 2021

Choose a reason for hiding this comment

mrT1ddl3s commented Feb 8, 2021 •

edited

Loading