Refactor the crkbd/via keymap and fix OLED bugs #10354

foostan · 2020-09-18T14:04:24Z

Description

Currently the via keymap has OLED bugs, so this PR is fix it.
The via keymap should keep simplify as default keymap.
Remove unnecessary functions
Update default keymap based the via keymap
Remove foostan keymap because it will be legacy

@drashna What do you think about this PR? Would you confirm it please?

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

drashna

If the issue is with the OLED not rendering correctly, I just ran into the issue last night.

https://github.com/foostan/qmk_firmware/blob/refactor_crkbd_via_keymap/keyboards/crkbd/rev1/common/common.h#L24

This line should be changed to #define is_master is_keyboard_master()

https://github.com/foostan/qmk_firmware/blob/refactor_crkbd_via_keymap/keyboards/crkbd/rev1/common/common.c#L21

This line should be removed.

keyboards/crkbd/rev1/common/keymaps/via/keymap.c

keyboards/crkbd/rev1/common/keymaps/via/rules.mk

foostan · 2020-09-19T22:56:05Z

If the issue is with the OLED not rendering correctly, I just ran into the issue last night.
https://github.com/foostan/qmk_firmware/blob/refactor_crkbd_via_keymap/keyboards/crkbd/rev1/common/common.h#L24

This line should be changed to #define is_master is_keyboard_master()
https://github.com/foostan/qmk_firmware/blob/refactor_crkbd_via_keymap/keyboards/crkbd/rev1/common/common.c#L21

This line should be removed.

Thank you for your suggestion.
I'll fix them and push.

drashna · 2020-09-20T02:01:52Z

You're very welcome!

This reverts commit dc9db02.

…keymap

foostan · 2020-09-23T01:54:04Z

@drashna Would you confirm it again please?

drashna

LGTM!

con-f-use · 2020-10-08T22:15:17Z

error: conflicting types for 'is_keyboard_master'

can we get that fixed?

drashna · 2020-10-13T11:07:44Z

Specifically, the default keymap needs the extern uint8_t is_master removed.

foostan · 2020-10-14T17:04:49Z

extern uint8_t is_master removed

drashna · 2020-10-15T10:42:48Z

It looks like the default keymap is not compiling for the "common" revision.

The best solution may be to change the default to use the OLED Driver feature. But for now, it may be simplest to just turn off the oled for the default keymap.

Eg, add:

#ifdef KEYBOARD_crkbd_rev1_common
#    undef SSD1306OLED
#endif

The alternative is to copy the OLED code from the via keymap to the default keymap, too.

If you want I can edit the code to address this, though, if you'd like.

foostan · 2020-10-16T09:02:20Z

I know, somebody want to use the default keymap to use the OLED Driver feature.

The alternative is to copy the OLED code from the via keymap to the default keymap, too.

So that I think it’s best solution about this problem.
I'm going to update the default keymap.

foostan · 2020-10-17T16:52:04Z

It's okey compiling the default keymap with "common" and "legacy" revision.
But some keymaps to be error. Should we fix all of keymaps?

drashna · 2020-10-18T20:34:40Z

But some keymaps to be error. Should we fix all of keymaps?

That's fine. As long as the default and via keymaps compile, that's the important part. People can clean up their keymaps on their own. And because it will be simpler for them, in the long term.
Also, it will take longer to get merged if user keymaps are touched.

foostan · 2020-10-24T13:44:18Z

@drashna Thank you helping! I reverted the commit for personal keymaps without mine and yours.

foostan · 2020-10-25T08:22:43Z

@drashna When will it be merged? Need a review from someone else?

foostan · 2020-11-09T01:40:57Z

@drashna Hi, what should I do for it?

lpwisniewski · 2020-11-11T18:34:15Z

Thank you very much ! Since the last update I wasn't able to make the OLED and matrix leds working at the same time but thanks this PR it is now fixed.
I tested the default keymap, it's working correctly.

keyboards/crkbd/keymaps/default/keymap.c

lpwisniewski · 2020-11-12T12:09:52Z

Actually the LEDs on the right side don't light up

…keymap

foostan · 2020-11-13T16:21:48Z

@fauxpark Thank you for reviewing. I updated according to your suggestion.

foostan · 2020-11-13T16:24:15Z

@lpwisniewski You use crkbd/rev1/common:default, right? common is not compatible to matrix LED. Please use legacy.

lpwisniewski · 2020-11-13T16:32:03Z

Thanks @foostan, it fixed the issue, everything is working. Is there some documentation that explain the difference between legacy and common ?

foostan · 2020-11-13T17:23:27Z

@lpwisniewski Sorry it's hard to understand. Please see #6001 (comment)

lpwisniewski · 2020-11-13T23:00:31Z

Thank you very much for the link, and BTW thank you for everything else, your work on this keyboard is awesome

drashna · 2020-11-17T05:18:20Z

Thank you very much for the link, and BTW thank you for everything else, your work on this keyboard is awesome

Sorry, that's my fault. I'll see about opening a PR that makes it more/explicitly clear.

* Refactor the crkbd/via keymap and fix OLED bugs * Revert "Refactor the crkbd/via keymap and fix OLED bugs" This reverts commit dc9db02. * Fix OLED display bugs * Remove unused functions * Remove uncessary the key logger * Add new lines at end of file * Remove unnecesary extern for is_master * Remove extern is_master from rev1.h * Remove ssd1306 from common dir because its the legacy lib * Update default keymap based the via keymap * Remove foostan keymap because it will be legacy * Revert Remove unnecesary extern for is_master * Remove unnecessary backslashes.

Refactor the crkbd/via keymap and fix OLED bugs

dc9db02

drashna reviewed Sep 19, 2020

View reviewed changes

drashna self-assigned this Sep 19, 2020

drashna added via Adds via keymap and/or updates keyboard for via support keymap labels Sep 19, 2020

2Moons-JP mentioned this pull request Sep 22, 2020

Add Slice RGB VIA Support #10387

Closed

14 tasks

foostan added 6 commits September 23, 2020 10:15

Revert "Refactor the crkbd/via keymap and fix OLED bugs"

fb03330

This reverts commit dc9db02.

Merge remote-tracking branch 'origin/master' into refactor_crkbd_via_…

b26f708

…keymap

Fix OLED display bugs

51b4d35

Remove unused functions

257ddd6

Remove uncessary the key logger

b1f5099

Add new lines at end of file

396486f

drashna approved these changes Oct 4, 2020

View reviewed changes

drashna requested a review from a team October 4, 2020 19:36

con-f-use mentioned this pull request Oct 12, 2020

Problem with OLED displays foostan/crkbd#66

Closed

foostan added 3 commits October 15, 2020 00:35

Remove unnecesary extern for is_master

cc374dc

Remove extern is_master from rev1.h

d445ff3

Remove ssd1306 from common dir because its the legacy lib

b5c6d71

foostan added 2 commits October 17, 2020 01:09

Update default keymap based the via keymap

17ad3eb

Remove foostan keymap because it will be legacy

573a1b1

Revert Remove unnecesary extern for is_master

226e83d

drashna approved these changes Oct 24, 2020

View reviewed changes

drashna requested a review from a team October 25, 2020 00:00

fauxpark requested changes Nov 11, 2020

View reviewed changes

foostan added 2 commits November 14, 2020 01:13

Merge remote-tracking branch 'origin/master' into refactor_crkbd_via_…

16d3a96

…keymap

Remove unnecessary backslashes.

dcceaac

github-actions bot added the keyboard label Nov 13, 2020

drashna requested a review from a team November 15, 2020 18:29

fauxpark approved these changes Nov 15, 2020

View reviewed changes

drashna merged commit 503b762 into qmk:master Nov 17, 2020

foostan deleted the refactor_crkbd_via_keymap branch November 17, 2020 11:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor the crkbd/via keymap and fix OLED bugs #10354

Refactor the crkbd/via keymap and fix OLED bugs #10354

foostan commented Sep 18, 2020 •

edited

Loading

drashna left a comment

foostan commented Sep 19, 2020

drashna commented Sep 20, 2020

foostan commented Sep 23, 2020

drashna left a comment

con-f-use commented Oct 8, 2020

drashna commented Oct 13, 2020

foostan commented Oct 14, 2020

drashna commented Oct 15, 2020

foostan commented Oct 16, 2020

foostan commented Oct 17, 2020

drashna commented Oct 18, 2020

foostan commented Oct 24, 2020 •

edited

Loading

foostan commented Oct 25, 2020

foostan commented Nov 9, 2020

lpwisniewski commented Nov 11, 2020

lpwisniewski commented Nov 12, 2020

foostan commented Nov 13, 2020

foostan commented Nov 13, 2020

lpwisniewski commented Nov 13, 2020

foostan commented Nov 13, 2020

lpwisniewski commented Nov 13, 2020

drashna commented Nov 17, 2020

Refactor the crkbd/via keymap and fix OLED bugs #10354

Refactor the crkbd/via keymap and fix OLED bugs #10354

Conversation

foostan commented Sep 18, 2020 • edited Loading

Description

Types of Changes

Issues Fixed or Closed by This PR

Checklist

drashna left a comment

Choose a reason for hiding this comment

foostan commented Sep 19, 2020

drashna commented Sep 20, 2020

foostan commented Sep 23, 2020

drashna left a comment

Choose a reason for hiding this comment

con-f-use commented Oct 8, 2020

drashna commented Oct 13, 2020

foostan commented Oct 14, 2020

drashna commented Oct 15, 2020

foostan commented Oct 16, 2020

foostan commented Oct 17, 2020

drashna commented Oct 18, 2020

foostan commented Oct 24, 2020 • edited Loading

foostan commented Oct 25, 2020

foostan commented Nov 9, 2020

lpwisniewski commented Nov 11, 2020

lpwisniewski commented Nov 12, 2020

foostan commented Nov 13, 2020

foostan commented Nov 13, 2020

lpwisniewski commented Nov 13, 2020

foostan commented Nov 13, 2020

lpwisniewski commented Nov 13, 2020

drashna commented Nov 17, 2020

foostan commented Sep 18, 2020 •

edited

Loading

foostan commented Oct 24, 2020 •

edited

Loading