Helix: add split_common option

[mtei]

Description

This PR adds the option to use split_common for Helix keyboard compilation.

• Compiling as follows will compile with the current Helix custom code.

  make helix:default
  make helix/pico:default
  

• Compiling as follows will compile with split_common code.

  make helix/rev2/sc:default
  make helix/pico/sc:default
  

You can also specify the use of split_common by adding the following in rules.mk for each keymap:

SPLIT_KEYBOARD = yes

All keymaps have been tested and verified to work.

What is split_common different from current Helix custom code

• Due to the current limitation of split_common, the current layer information cannot be obtained on the slave side, so the layer information cannot be displayed correctly on OLED etc.

• split_common scans the matrix a bit slower.

  	scan cycle/second
  Helix custom code	1590
  split_common	1190

Keymaps source file compatibility

All keymaps 'keymap.c' and 'config.h' are unchanged. There is no need to change.
The 'rules.mk' of all keymaps except 'xulkal' have not changed.

In the 'xulkal' keymap rules.mk, SPLIT_KEYBOARD = yes was added to always use the split_common code, and the following code in 'users/xulkal/custom_oled.c' was removed.

#if KEYBOARD_helix_rev2
extern uint8_t is_master;
bool is_keyboard_master(void) { return is_master; }
#endif

All compile option patterns

• build helix/pico (HelixPico) with helix current codes

  $ make helix/pico:KEY_MAP
  $ make helix/pico/back:KEY_MAP
  $ make helix/pico/under:KEY_MAP
  

• build helix/rev2 (Helix or Helix beta) with helix current codes

  $ make helix:KEY_MAP
  $ make helix/rev2/back:KEY_MAP
  $ make helix/rev2/under:KEY_MAP
  $ make helix/rev2/oled:KEY_MAP
  $ make helix/rev2/oled/back:KEY_MAP
  $ make helix/rev2/oled/under:KEY_MAP
  

• build helix/pico (HelixPico) with split_common codes

  $ make helix/pico/sc:KEY_MAP
  $ make helix/pico/sc/back:KEY_MAP
  $ make helix/pico/sc/under:KEY_MAP
  

• build helix/rev2 (Helix) with split_common codes

  $ make helix/rev2/sc:KEY_MAP
  $ make helix/rev2/sc/back:KEY_MAP
  $ make helix/rev2/sc/under:KEY_MAP
  $ make helix/rev2/sc/oled:KEY_MAP
  $ make helix/rev2/sc/oledback:KEY_MAP
  $ make helix/rev2/sc/oledunder:KEY_MAP
  

Types of Changes

• Core
• Bugfix
• New feature
• Enhancement/optimization
• Keyboard (addition or update)
• Keymap/layout/userspace (addition or update)
• Documentation

Checklist

• My code follows the code style of this project.
• 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).

[mtei]

先日クローズした #24 のやりなおし版です。

今回は、OLED ドライバには触らず、split_common を使用するオプションの追加だけを行ないました。

上の英文で書いてあるように、split_common を使うと機能制限が 1つ(スレーブ側で現在のレイヤー情報が得られない)と 性能の若干の劣化があります。
(先日、お会いしたときもちょっと話したとおりです)

それもあって、split_common への無条件の切り替えはひかえて、お試し版のつもりで、split_common も使えるようにオプション追加ですませました。
今後おいおい split_common 側のコードを整備して全面切り替えできるように持っていきたいなと思っています。

テストとプルリクの処理手順

私の手元の PC で、以下のようにブランチを作りました。

$ cd qmk_firmware
$ git checkout -b helix_add_split_common_option 0.7.100

前回と同様に以下の手順を実行すれば、qmk へのpullreq が綺麗に出来るはずです。

まずないんさんの PC と Github リポジトリに qmk の最新をとりこみます。

次にないんさんの PC で、以下を実行

$ cd qmk_firmware
$ git checkout -b helix_add_split_common_option 0.7.100
$ git push origin helix_add_split_common_option

ないんさんの GitHub リポジトリで、この PR のタイトルの右にある編集ボタンを押して
マージ先のブランチを、helix_add_split_common_option に変更します。(まだマージはしません。)

ないんさんの PC で、以下を実行すれば、私のところの helix_add_split_common_option とまったく同じ内容のブランチが test_split_common という名前で出来上がるので、内容の確認と動作の確認をします。

$ cd qmk_firmware
$ git fetch origin pull/24/head:test_split_common
$ git checkout test_split_common

確認が終了したら、ないんさんの GitHub リポジトリで、マージをします。

最後に helix_add_split_common_option ブランチを qmk に PullRequest すれば、綺麗な PR になると思われます。

[MakotoKurauchi]

ありがとうございます。
今まで通りのオプションの時に

$ make helix/rev2/oled/back:avrdude

スレーブ側のOLEDのレイヤー表示が

Layer: Raiselt

となってしまうようです。ご確認頂けますか？

[mtei]

この PR のブランチと master の二つで、

$ make helix/rev2/oled/back:default:avrdude

をやってみましたが、正常に期待通り動いているようです。

今よく見ると、ないんさんのコマンドは、$ make helix/rev2/oled/back:avrdude で、defualt の指定が抜けているようですが、これは単なる書き間違いですか？ それともこの通りのコマンドを入れたのでしょうか？

[mtei]

キーマップ名を省略したコンパイル指定は、暗黙のうちに all が仮定されるようです。

$ make helix/rev2/oled/back:uso
QMK Firmware 0.7.100
Making helix/rev2/oled/back with keymap default and target uso          [ERRORS]
make[1]: *** No rule to make target `uso'.  Stop.
Making helix/rev2/oled/back with keymap edvorakjp and target uso        [ERRORS]
make[1]: *** No rule to make target `uso'.  Stop.
Making helix/rev2/oled/back with keymap five_rows and target uso        [ERRORS]
make[1]: *** No rule to make target `uso'.  Stop.
途中略
make: *** [helix/rev2/oled/back:uso] Error 1
Make finished with errors

$ make helix/rev2/oled/back:all:uso
QMK Firmware 0.7.100
Making helix/rev2/oled/back with keymap default and target uso          [ERRORS]
make[1]: *** No rule to make target `uso'.  Stop.
Making helix/rev2/oled/back with keymap edvorakjp and target uso        [ERRORS]
make[1]: *** No rule to make target `uso'.  Stop.
Making helix/rev2/oled/back with keymap five_rows and target uso        [ERRORS]
make[1]: *** No rule to make target `uso'.  Stop.
途中略
make: *** [helix/rev2/oled/back:all:uso] Error 1
Make finished with errors

なので、

$ make helix/rev2/oled/back:avrdude

は、以下と同じになり、

$ make helix/rev2/oled/back:all:avrdude

最初の書き込みで default キーマップ、2番目の書き込みで、edvorakjp キーマップを書き込んでいる可能性がありそうです。

[MakotoKurauchi]

混乱させてしまいすみません。コマンドはコピペミスで間違いです。
現状のmasterに戻して正常に動作していることは確認済みです。

[mtei]

うーん、、不思議です。この PR では、キーマップ系のソースは変更がないので変わらないとおもうのです。実際、手元では、master でも、このブランチでも同じ動作をしています。

もういちど、master, このブランチの helix original、このブランチの split_common 使用、の3つを default キーマップで試してみましたが、前2者は同じ動作、最後のものは、slave 側が 'default' 表示のまま、と、期待したとおりの動きでした。

どうすれば、報告の挙動を再現できるのだろう。

[mtei]

ごめんなさい、上で、以下のように書いたところが間違っていました。

$ cd qmk_firmware
$ git fetch origin pull/24/head:test_split_common
$ git checkout test_split_common

正しくは、以下のように、24 を 26 にしなければなりませんでした。

$ cd qmk_firmware
$ git fetch origin pull/26/head:test_split_common
$ git checkout test_split_common

一旦 test_split_common ブランチを消してやり直してからテストしてどうなるか確認お願いします。

(こちらでも、報告の現象再現できました。これは、#24 に有って見逃していたバグですね。ssd1306.c と drivers/oled/oled_driver.c の挙動に違いがあるようです。)

[MakotoKurauchi]

ありがとうございます！
こちらも確認不足でした。

[mtei]

追加の、PR #27 を作りました。マージしてもらえば、自動的に qmk/qmk_firmware への PR に追加で反映されます。

qmk 本家の方で、PR7915 が出ていて、それに伴って、quantum/matrix.c か quantum/split_common/matrix.c を利用するキーボードは、config.h に以下が必須になったそうです。
Helix では、make helix/rev2/sc:map のように sc をつけた時が該当します。

#define DIODE_DIRECTION COL2ROW

[mtei]

追加の、PR #28 を作りました。

readme にほんのちょっと追記を行いました。

ひと月経過してもqmk 本家にいっこうにマージされてない現状です。
みると All checks have failed になっていますが、さらに詳しく見てみると、他のキーボードのコンパイルがエラーになっていて、helix の問題ではなさそうです。
コメントでそれを指摘しても良いのですが、readme をちょっと直すネタを思いついたのでそれを追加することでテストを再度起動してみようかと思います