remap-keys / remap

Keymap Customization Web app for your keyboard.
https://remap-keys.app
Other
227 stars 27 forks source link

Data is not correct in TO keycode #746

Closed koktoh closed 1 year ago

koktoh commented 1 year ago

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 commented 1 year ago

The key code range of TO(n) has been changed by the https://github.com/qmk/qmk_firmware/pull/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 https://github.com/qmk/qmk_firmware/pull/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 commented 1 year ago

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

yoichiro commented 1 year ago

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

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

yoichiro commented 1 year ago

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 commented 1 year ago

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