Update dynamis keyboard #17625

bbrfkr · 2022-07-11T16:47:28Z

Description

I updated the code of dynamis keyboard. In this work, the changes are the following:

support remapping encoder
update default keymap
update readme

Types of Changes

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

bbrfkr · 2022-07-11T16:53:28Z

@drashna Could you check my PR?

keyboard-magpie · 2022-07-11T20:11:28Z

These appear to be relatively significant changes to the matrix; does this represent new hardware that should in fact be managed in a rev2 folder?

drashna · 2022-07-11T21:03:37Z

@drashna Could you check my PR?

This PR is 4 hours old, and has not been reviewed at all, yet. There is no need to tag me, at this point.

In this context, this sort of ping is actually quite rude.

Pull Request reviews are something that is done in our free time. We are not paid nor compensated for the time we spend reviewing, as it is a labor of love. As such, this means that it can take time for us to get to your Pull Request. Things like family, or life can get in the way of us getting to PRs, and burnout is a serious consideration. The QMK firmware repository averages 200 PRs opened and 200 PRs merged every month, so please have patience.

And to be clear, I'm not angry here, but this is behavior that we want to nip in the bud before it becomes a problem.

keyboards/bbrfkr/dynamis/dynamis.c

keyboards/bbrfkr/dynamis/dynamis.h

keyboards/bbrfkr/dynamis/readme.md

keyboards/bbrfkr/dynamis/dynamis.c

bbrfkr · 2022-07-11T22:25:54Z

Dear drashna,

I apologize for my rude behavior. I mistaked the usage of @ tag...
I swear it will never happen again.

Thank you for pointing it out.

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

bbrfkr · 2022-07-12T00:31:48Z

These appear to be relatively significant changes to the matrix; does this represent new hardware that should in fact be managed in a rev2 folder?

No. This code is newly version firmware of dynamis keyboard, and support the same hardware.

drashna · 2022-07-12T00:37:05Z

Dear drashna,

I apologize for my rude behavior. I mistaked the usage of @ tag... I swear it will never happen again.

Thank you for pointing it out.

No worries. If it's been ... a while (eg, weeks), that's fine. Also, if you exclude the "@", it won't tag that user.

Eg, tagging a user is for getting their attention, but in this case, there is no need to get my attention (it's new, so I'll definitely see it when I go through PRs)

keyboards/bbrfkr/dynamis/dynamis.c

keyboards/bbrfkr/dynamis/keymaps/via/keymap.c

keyboards/bbrfkr/dynamis/dynamis.c

drashna · 2022-07-13T05:28:36Z

keyboards/bbrfkr/dynamis/dynamis.h

+        { C90, C91, C92, C93, C94, C95, C96 }, \
+        { KC_WH_D, KC_WH_U, KC_NO, KC_NO, KC_NO, KC_NO, KC_NO } \


Suggested change

{ C90, C91, C92, C93, C94, C95, C96 }, \

{ KC_WH_D, KC_WH_U, KC_NO, KC_NO, KC_NO, KC_NO, KC_NO } \

{ C90, C91, C92, C93, C94, C95, C96 } \

drashna · 2022-07-13T05:28:50Z

keyboards/bbrfkr/dynamis/dynamis.h

+        { C90, C91, C92, C93, C94, C95, C96 }, \
+        { KC_WH_D, KC_WH_U, KC_NO, KC_NO, KC_NO, KC_NO, KC_NO } \


Suggested change

{ C90, C91, C92, C93, C94, C95, C96 }, \

{ KC_WH_D, KC_WH_U, KC_NO, KC_NO, KC_NO, KC_NO, KC_NO } \

{ C90, C91, C92, C93, C94, C95, C96 } \

drashna · 2022-07-13T05:29:03Z

keyboards/bbrfkr/dynamis/config.h

@@ -26,9 +26,9 @@
 #define PRODUCT dynamis

 /* key matrix */
-#define MATRIX_ROWS 10
+#define MATRIX_ROWS 11


Suggested change

#define MATRIX_ROWS 11

#define MATRIX_ROWS 10

drashna · 2022-07-13T05:29:32Z

keyboards/bbrfkr/dynamis/config.h

 #define MATRIX_COLS 7
-#define MATRIX_ROW_PINS { B6, B4, D6, D5, D1, C6, B5, D7, D4, D0 }
+#define MATRIX_ROW_PINS { B6, B4, D6, D5, D1, C6, B5, D7, D4, D0, D2 }


Is D2 actually being used here? Or just being used as a place holder?

Suggested change

#define MATRIX_ROW_PINS { B6, B4, D6, D5, D1, C6, B5, D7, D4, D0, D2 }

#define MATRIX_ROW_PINS { B6, B4, D6, D5, D1, C6, B5, D7, D4, D0 }

D2 is for the encoder. I think that the default via keymap for encoder cannot be defined if this doesn't exist.

You can undefine and redefine the values for the via keymap, actually.

Which is the ideal option here, since you're creating blank spots in the layout that are ONLY used for the encoders.

Also, you should use NO_PIN instead of assigning a pin here.

I see. I replaced D2 to NO_PIN.

bbrfkr · 2022-08-11T16:12:00Z

I didn't notice that this pr conflicted. The conflict has been resolved.

drashna · 2022-08-14T02:28:47Z

keyboards/bbrfkr/dynamis/keymaps/via/keymap.c

+keyevent_t encoder_ccw = {
+    .key = (keypos_t){.row = 10, .col = 1},
+    .pressed = false
 };
+
+keyevent_t encoder_cw = {
+    .key = (keypos_t){.row = 10, .col = 0},
+    .pressed = false
+};
+
+void matrix_scan_user(void) {
+    if (IS_PRESSED(encoder_ccw)) {
+        encoder_ccw.pressed = false;
+        encoder_ccw.time = (timer_read() | 1);
+        action_exec(encoder_ccw);
+    }
+
+    if (IS_PRESSED(encoder_cw)) {
+        encoder_cw.pressed = false;
+        encoder_cw.time = (timer_read() | 1);
+        action_exec(encoder_cw);
+    }
+}
+
+bool encoder_update_user(uint8_t index, bool clockwise) {
+    if (clockwise) {
+        encoder_cw.pressed = true;
+        encoder_cw.time = (timer_read() | 1);
+        action_exec(encoder_cw);
+    } else {
+        encoder_ccw.pressed = true;
+        encoder_ccw.time = (timer_read() | 1);
+        action_exec(encoder_ccw);
+    }
+    return false;
+}


Suggested change

keyevent_t encoder_ccw = {

.key = (keypos_t){.row = 10, .col = 1},

.pressed = false

};

keyevent_t encoder_cw = {

.key = (keypos_t){.row = 10, .col = 0},

.pressed = false

};

void matrix_scan_user(void) {

if (IS_PRESSED(encoder_ccw)) {

encoder_ccw.pressed = false;

encoder_ccw.time = (timer_read() | 1);

action_exec(encoder_ccw);

}

if (IS_PRESSED(encoder_cw)) {

encoder_cw.pressed = false;

encoder_cw.time = (timer_read() | 1);

action_exec(encoder_cw);

}

}

bool encoder_update_user(uint8_t index, bool clockwise) {

if (clockwise) {

encoder_cw.pressed = true;

encoder_cw.time = (timer_read() | 1);

action_exec(encoder_cw);

} else {

encoder_ccw.pressed = true;

encoder_ccw.time = (timer_read() | 1);

action_exec(encoder_ccw);

}

return false;

}

drashna · 2022-08-14T02:30:29Z

keyboards/bbrfkr/dynamis/config.h

 #define MATRIX_COLS 7
-#define MATRIX_ROW_PINS { B6, B4, D6, D5, D1, C6, B5, D7, D4, D0 }
+#define MATRIX_ROW_PINS { B6, B4, D6, D5, D1, C6, B5, D7, D4, D0, NO_PIN }


To clarify, the NO_PIN is there to facilitate encoder in the matrix, correct?

If so, then it needs to be reverted. It's not the proper way to handle this, especially as VIA now supports the encoder map feature.

https://docs.qmk.fm/#/feature_encoders?id=encoder-map

This allows for proper handling of all keycodes, without special cases, and for it be configured dynamically, by VIA

Thank you for your comment. But, I cannot find the document that the via support encoder map... So, I don't understand to construct a json file for supporting encoder map feature...

https://www.caniusevia.com/docs/configuring_qmk

Thank you for your sharing! I'll try it...

Ok. Now, this code supported encoder map.

The json for via is the following;

{ "name": "dynamis", "vendorId": "0x6262", "productId": "0x0001", "lighting": "qmk_rgblight", "matrix": { "rows": 10, "cols": 7 }, "layouts": { "labels": [ "ISO Enter", "Split Space", "Split Left Shift", "Split Right Shift", "Split Backspace" ], "keymap": [ [ { "x": 15.5 }, "5,0\n\n\n4,1", "9,0\n\n\n4,1" ], [ { "y": 0.25, "x": 2.5 }, "0,0", "0,1", "0,2", "0,3", "0,4", "0,5", "0,6", "5,6", "5,5", "5,4", "5,3", "5,2", "5,1", { "w": 2 }, "5,0\n\n\n4,0" ], [ { "x": 2.5, "w": 1.5 }, "1,0", "1,1", "1,2", "1,3", "1,4", "1,5", "1,6", "6,6", "6,5", "6,4", "6,3", "6,2", "6,1", { "w": 1.5 }, "6,0\n\n\n0,0", { "x": 4, "w": 1.25, "h": 2, "w2": 1.5, "h2": 1, "x2": -0.25 }, "7,0\n\n\n0,1" ], [ { "x": 2.5, "w": 1.75 }, "2,0", "2,1", "2,2", "2,3", "2,4", "2,5", "2,6", "7,6", "7,5", "7,4", "7,3", "7,2", { "w": 2.25 }, "7,0\n\n\n0,0", { "x": 3 }, "7,1\n\n\n0,1" ], [ { "w": 1.25 }, "3,0\n\n\n2,1", "3,1\n\n\n2,1", { "x": 0.25, "w": 2.25 }, "3,1\n\n\n2,0", "3,2", "3,3", "3,4", "3,5", "3,6", "8,6", "8,5", "8,4", "8,3", "8,2", { "w": 2.75 }, "8,1\n\n\n3,0", { "x": 0.25 }, "9,1", { "a": 7 }, "e0", { "x": 0.25, "a": 4, "w": 1.75 }, "8,1\n\n\n3,1", "8,0\n\n\n3,1" ], [ { "x": 2.5, "w": 1.25 }, "4,0", { "w": 1.25 }, "4,1", { "w": 1.25 }, "4,2", { "w": 6.25 }, "4,3\n\n\n1,0", { "w": 1.25 }, "4,6", { "w": 1.25 }, "9,6", { "w": 1.25 }, "9,5", { "x": 0.5 }, "9,4", "9,3", "9,2" ], [ { "y": 0.25, "x": 6.25, "w": 2.25 }, "4,3\n\n\n1,1", { "w": 1.25 }, "4,4\n\n\n1,1", { "w": 2.75 }, "4,5\n\n\n1,1" ] ] } }

bbrfkr · 2022-08-29T05:19:44Z

Thank you for your review!

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

bbrfkr added 5 commits July 11, 2022 04:33

mod modifiers

3e9bc4e

mod initial layout

8ac14a8

mod readme

d77cc48

mod info.json

96dae11

support via for remapping encoder

ee8461e

github-actions bot added keyboard keymap via Adds via keymap and/or updates keyboard for via support labels Jul 11, 2022

add white space and last break line

388fda4

drashna requested changes Jul 11, 2022

View reviewed changes

keyboards/bbrfkr/dynamis/dynamis.c Outdated Show resolved Hide resolved

keyboards/bbrfkr/dynamis/dynamis.h Outdated Show resolved Hide resolved

keyboards/bbrfkr/dynamis/readme.md Outdated Show resolved Hide resolved

keyboards/bbrfkr/dynamis/dynamis.c Outdated Show resolved Hide resolved

bbrfkr and others added 3 commits July 12, 2022 08:23

Update keyboards/bbrfkr/dynamis/readme.md

c5322b6

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

mod usage encoder

8db05d3

move mute default key

523053a

drashna reviewed Jul 12, 2022

View reviewed changes

keyboards/bbrfkr/dynamis/dynamis.c Outdated Show resolved Hide resolved

drashna reviewed Jul 12, 2022

View reviewed changes

keyboards/bbrfkr/dynamis/keymaps/via/keymap.c Outdated Show resolved Hide resolved

mod encoder behavior

79c107c

drashna requested changes Jul 13, 2022

View reviewed changes

bbrfkr added 3 commits July 15, 2022 08:59

move the defaut behavior for encoder to .h file

cab64d6

replace actual pin to NO_PIN pin

f2272ef

mod conflict

df9866d

drashna requested changes Aug 14, 2022

View reviewed changes

bbrfkr added 2 commits August 16, 2022 04:26

Merge remote-tracking branch 'qmk/master' into update-dynamis-keyboard

84e7b30

try to enable encoder map

dca526c

Merge remote-tracking branch 'qmk/master' into update-dynamis-keyboard

4ab684d

drashna approved these changes Aug 28, 2022

View reviewed changes

drashna requested a review from a team August 28, 2022 22:11

fauxpark approved these changes Aug 29, 2022

View reviewed changes

drashna merged commit a2294bc into qmk:master Aug 29, 2022

bbrfkr mentioned this pull request Aug 29, 2022

add bbrfkr dynamis keyboard the-via/keyboards#1387

Merged

6 tasks

nolanseaton pushed a commit to nolanseaton/qmk_firmware that referenced this pull request Jan 23, 2023

[Keyboard] Update dynamis keyboard (qmk#17625)

f57e00d

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update dynamis keyboard #17625

Update dynamis keyboard #17625

bbrfkr commented Jul 11, 2022

bbrfkr commented Jul 11, 2022

keyboard-magpie commented Jul 11, 2022

drashna commented Jul 11, 2022 •

edited

Loading

bbrfkr commented Jul 11, 2022

bbrfkr commented Jul 12, 2022

drashna commented Jul 12, 2022

drashna Jul 13, 2022

drashna Jul 13, 2022

drashna Jul 13, 2022

drashna Jul 13, 2022

bbrfkr Jul 15, 2022

drashna Jul 16, 2022

bbrfkr Jul 17, 2022

bbrfkr commented Aug 11, 2022

drashna Aug 14, 2022

drashna Aug 14, 2022

bbrfkr Aug 16, 2022

drashna Aug 23, 2022

bbrfkr Aug 23, 2022

bbrfkr Aug 28, 2022 •

edited

Loading

bbrfkr commented Aug 29, 2022

		{ C90, C91, C92, C93, C94, C95, C96 }, \
		{ KC_WH_D, KC_WH_U, KC_NO, KC_NO, KC_NO, KC_NO, KC_NO } \

	{ C90, C91, C92, C93, C94, C95, C96 }, \
	{ KC_WH_D, KC_WH_U, KC_NO, KC_NO, KC_NO, KC_NO, KC_NO } \
	{ C90, C91, C92, C93, C94, C95, C96 } \

	#define MATRIX_ROW_PINS { B6, B4, D6, D5, D1, C6, B5, D7, D4, D0, D2 }
	#define MATRIX_ROW_PINS { B6, B4, D6, D5, D1, C6, B5, D7, D4, D0 }

Update dynamis keyboard #17625

Update dynamis keyboard #17625

Conversation

bbrfkr commented Jul 11, 2022

Description

Types of Changes

Issues Fixed or Closed by This PR

Checklist

bbrfkr commented Jul 11, 2022

keyboard-magpie commented Jul 11, 2022

drashna commented Jul 11, 2022 • edited Loading

bbrfkr commented Jul 11, 2022

bbrfkr commented Jul 12, 2022

drashna commented Jul 12, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bbrfkr commented Aug 11, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bbrfkr Aug 28, 2022 • edited Loading

Choose a reason for hiding this comment

bbrfkr commented Aug 29, 2022

drashna commented Jul 11, 2022 •

edited

Loading

bbrfkr Aug 28, 2022 •

edited

Loading