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.

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

Data is not correct in TO keycode #746

Closed
koktoh opened this issue Oct 6, 2022 · 5 comments · Fixed by #747
Closed

Data is not correct in TO keycode #746

koktoh opened this issue Oct 6, 2022 · 5 comments · Fixed by #747

Comments

@koktoh
Copy link

koktoh commented Oct 6, 2022

The layer index in TO keycode is seems to be not correct.

TO keycode has layer index in lower 8bits.
But, remap reads/writes only lower 4bits.

If you write TO(0) keycode, raw data should be 0x5000, but raw data is 0x5010.
Similarly, remap writes TO(2) = 0x5002 as TO(18) = 0x5012, TO(4) = 0x5004 as TO(20) = 0x5014, and so on...

And, remap reads TO(16) = 0x5010 as TO(0) = 0x5000.
Similarly, remap reads TO(18) = 0x5012 as TO(2) = 0x5002, TO(20) = 0x5014 as TO(4) = 0x5004, and so on...

In below code, TO(0) is defined 20496 = 0x5010.

https://github.com/remap-keys/remap/blob/main/src/services/hid/KeycodeInfoList.ts#L3329-L3339

In QMK, TO code is defined here.

    QK_TO                   = 0x5000,

https://github.com/qmk/qmk_firmware/blob/master/quantum/quantum_keycodes.h#L42

And, TO(layer) macro is defined here.

#define TO(layer) (QK_TO | ((layer)&0xFF))

https://github.com/qmk/qmk_firmware/blob/master/quantum/quantum_keycodes.h#L803

LM, LT use lower 4bits as layer index.
Other layer keycodes use lower 8bits.
I think these layer keycodes has same issue.

My firmware targets RP2040.
I can send firmware to you if you need it to verify.

Some of configs are below.

/* VIA */
#define DYNAMIC_KEYMAP_LAYER_COUNT 30

/* RP2040 Flash Config */
#define PICO_FLASH_SIZE_BYTES (8 * 1024 * 1024)

/* EEPROM */
#define EEPROM_SIZE (1024)
#define WEAR_LEVELING_LOGICAL_SIZE	(8 * 1024)
#define WEAR_LEVELING_BACKING_SIZE	(WEAR_LEVELING_LOGICAL_SIZE * 2)
@yoichiro
Copy link
Collaborator

The key code range of TO(n) has been changed by the qmk/qmk_firmware#17989. It seems that the report from @koktoh was written by only checking the latest source code of the QMK Firmware.

That is, by the qmk/qmk_firmware#17989, Remap and VIA app are currently supporting firmwares which were built using the source code set before merging the pull request. And they currently can't support firmwares which are built using the source code set after merging the pull request.

Because there is no method to know what the version number and the revision number is the connected keyboard, we have a policy that we follow the latest (= master branch) source code set of QMK Firmware. As the result, we intend to fix this issue to follow the behavior of latest source code of QMK Firmware. That is, we will change the TO(n) key code range.

@Ftakao
Copy link

Ftakao commented Oct 19, 2022

日本語での記載失礼します。
具体的な症状としては、TO(1)を割り当てキーボード側で操作すると、レイヤー17が読み込まれる、といった具合になります。
現状、レイヤー30枚まで対応しているのですが、その後は(2)=レイヤー18、といった具合で増えていき、実際の割当レイヤーが30を超えると動作を停止してしまいます。
TO(0)がレイヤー16だったので、おそらく16スタートとしてずれて認識されてるのだと思われます。
MOなど他のコマンド?でも同じ挙動でした。
お手数をおかけしますがよろしくお願いいたします。

@yoichiro
Copy link
Collaborator

MOなど他のコマンド?でも同じ挙動でした。

@Ftakao TO(n) は原因が判明しているので対応可能ですが、MO(n) については QMK Firmware と Remap ではキーコードに差がない( 0x5100 - 0x51FF )ために問題は生じない、という認識でいます。TO(n) の不具合に影響して MO(n) も不具合となってしまっているのか、電源投入後に TO(n) を一切使わずに MO(n) だけを使った際でも問題が発生するのか、今一度確認をお願いできますでしょうか?

@yoichiro
Copy link
Collaborator

I recognized that the TO(n) key codes are different from them of QMK Firmware. But, I think that other key codes are same as QMK Firmware. Therefore, I will fix the TO(n) problem against this issue.

@yoichiro
Copy link
Collaborator

@koktoh @Ftakao Could you confirm whether this issue was fixed or not with the latest Remap (its build number is 202)? If you find some problem, you can reopen this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants