usb_host keyboard, keys f1-f12, ctrl-left, ctrl-right, ctrl-up, ctrl-down, page down, page up, insert, delete, pause in VT100 CSI #8807

iyalosovetsky · 2024-01-17T01:40:21Z

I am very impressed with how the USB host keyboard works. For my CNC project, I `ve to added the ability to work with the function keys f1-f12, ctrl-left, ctrl-right, ctrl-up, ctrl-down, page down, page up, insert, delete, pause in the format of command VT 100. Offer my modified code "supervisor/shared/usb/host_keyboard.c"

Added the ability to work with the function keys f1-f12, ctrl-left, ctrl-right, ctrl-up, ctrl-down, page down, page up, insert, delete, pause. in the format of command VT 100

tannewt · 2024-01-17T18:17:54Z

Please run pre-commit over your code: https://learn.adafruit.com/improve-your-code-with-pylint/check-your-code-with-pre-commit It should automatically fix these issue before commit.

tannewt

Thanks for the improvements! Just a couple suggestions.

tannewt · 2024-01-17T18:18:42Z

supervisor/shared/usb/host_keyboard.c

    { HID_KEY_ENTER, HID_KEY_SLASH, 0, 0, "\r\x1b\10\t -=[]\\#;'`,./" },

-    { HID_KEY_F1, HID_KEY_F1, 0x1e, 0, }, // help key on xerox 820 kbd
+    // { HID_KEY_F1, HID_KEY_F1, 0x1e, 0, }, // help key on xerox 820 kbd


Suggested change

// { HID_KEY_F1, HID_KEY_F1, 0x1e, 0, }, // help key on xerox 820 kbd

supervisor/shared/usb/host_keyboard.c

iyalosovetsky · 2024-01-18T11:34:22Z

Thank you for the suggestion 1. About HID_KEY_F1. I`ve changed it to going t0 VT100 CSI standards. if it`s important to leave compatibility with xerox 820 keyboard I can change to the previous version. 2. About HID_KEY_ARROW_RIGHT, HID_KEY_ARROW_UP. I spent a couple of hours getting the arrows to work with ctr and without ctrl, and this option worked. Perhaps the cause of the error was resolved by the code below. I can return everything back and test again. if (!(mapper->flags & FLAG_CTRL) && ctrl) { continue; } Best regards! ср, 17 янв. 2024 г. в 20:20, Scott Shawcroft ***@***.***>:

…

***@***.**** requested changes on this pull request. Thanks for the improvements! Just a couple suggestions. ------------------------------ In supervisor/shared/usb/host_keyboard.c <#8807 (comment)> : > @@ -90,13 +110,43 @@ STATIC struct keycode_mapper keycode_to_ascii[] = { { HID_KEY_ENTER, HID_KEY_SLASH, 0, FLAG_SHIFT, "\n\x1b\177\t _+{}|~:\"~<>?" }, { HID_KEY_ENTER, HID_KEY_SLASH, 0, 0, "\r\x1b\10\t -=[]\\#;'`,./" }, - { HID_KEY_F1, HID_KEY_F1, 0x1e, 0, }, // help key on xerox 820 kbd + // { HID_KEY_F1, HID_KEY_F1, 0x1e, 0, }, // help key on xerox 820 kbd ⬇️ Suggested change - // { HID_KEY_F1, HID_KEY_F1, 0x1e, 0, }, // help key on xerox 820 kbd ------------------------------ In supervisor/shared/usb/host_keyboard.c <#8807 (comment)> : > { HID_KEY_KEYPAD_DIVIDE, HID_KEY_KEYPAD_DECIMAL, 0, FLAG_NUMLOCK | FLAG_STRING, "/\0" "*\0" "-\0" "+\0" "\n\0" CURSOR_END SEP CURSOR_DOWN SEP CURSOR_PGDN SEP CURSOR_LEFT SEP NOTHING SEP CURSOR_RIGHT SEP CURSOR_HOME SEP CURSOR_UP SEP CURSOR_PGDN SEP CURSOR_INS SEP CURSOR_DEL}, { HID_KEY_KEYPAD_DIVIDE, HID_KEY_KEYPAD_DECIMAL, 0, 0, "/*-+\n1234567890." }, - { HID_KEY_ARROW_RIGHT, HID_KEY_ARROW_UP, 0, FLAG_STRING, CURSOR_RIGHT SEP CURSOR_LEFT SEP CURSOR_DOWN SEP CURSOR_UP }, + // { HID_KEY_ARROW_RIGHT, HID_KEY_ARROW_UP, 0, FLAG_STRING, CURSOR_RIGHT SEP CURSOR_LEFT SEP CURSOR_DOWN SEP CURSOR_UP }, Why remove this? — Reply to this email directly, view it on GitHub <#8807 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACPGUT5PARKEEKDB474ETQLYPAI6BAVCNFSM6AAAAABB5XV67CVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTQMRXHEYDMMZTHE> . You are receiving this because you authored the thread.Message ID: ***@***.***>

jepler · 2024-01-18T16:28:40Z

. if it`s important to leave compatibility with xerox 820 keyboard I can change to the previous version.

No, this is not a priority for me.

jepler · 2024-01-18T16:34:59Z

supervisor/shared/usb/host_keyboard.c

+                if (!(mapper->flags & FLAG_CTRL) && ctrl) {
+                    continue;
+                }                


I suspect this change doesn't have the desired effect.

When ctrl+c is pressed, the result needs to be the interrupt character with value 3.

This happens through the keycode mapper element { HID_KEY_A, HID_KEY_Z, 'a', 0, NULL}, which does not have FLAG_CTRL set, so the initial code is the ASCII value 'c'. Then this is modified (at line 258 before this change) with a bitmask:

if (ctrl) { code &= 0x1f; }

I believe, but didn't test, that with your change the rule for KEY_A..KEY_Z will "continue" at new line 297 rather than sending the interrupt character.

I am guessing that you made this change so that your new code will send the desired code for ctrl+ARROW_UP and so on, but merely putting the entries in the correct order (with the FLAG_CTRL entry first) should suffice.

You're right!
I've changed to

if (!(mapper->flags & FLAG_CTRL) && ctrl && (mapper->flags & FLAG_STRING)) { continue; }

and it works fine.

The best way would be to set a range of special keys, but no idea if that's possible or not.

jepler · 2024-01-18T16:35:53Z

supervisor/shared/usb/host_keyboard.c

+    { HID_KEY_F12, HID_KEY_F12, 0, FLAG_STRING, F12 },
+    { HID_KEY_PRINT_SCREEN, HID_KEY_PRINT_SCREEN, 0, FLAG_STRING, PRINT_SCREEN },
+
+    { HID_KEY_ARROW_UP, HID_KEY_ARROW_UP, 0 , FLAG_STRING+FLAG_CTRL,CTRL_UP  },


I'm more accustomed to the style where bit masks are combined with "|" instead of "+" and would prefer it that way here.

jepler · 2024-01-18T16:42:21Z

supervisor/shared/usb/host_keyboard.c

+    { HID_KEY_INSERT, HID_KEY_INSERT, 0, FLAG_STRING, CURSOR_INS },
+    { HID_KEY_DELETE, HID_KEY_DELETE, 0, FLAG_STRING, CURSOR_DEL },
+
+    { HID_KEY_F1, HID_KEY_F1, 0, FLAG_STRING, F1 },


When USB keycodes are consecutive, which F1..F12 and RIGHT...UP are, it saves a small amount (maybe 8 bytes per entry) of flash storage to use the form which accommodates multiple strings within one entry, { HID_KEY_ARROW_RIGHT, HID_KEY_ARROW_UP, 0, FLAG_STRING, CURSOR_RIGHT SEP CURSOR_LEFT SEP CURSOR_DOWN SEP CURSOR_UP },

This is done somewhat inconsistently in the original (for instance, INSERT..ARROW_UP are consecutive but the table does not use this fact) but it would be nice if it were done here for these 12 consecutive items, it might save about 100 bytes of flash storage.

jepler · 2024-01-18T16:51:27Z

supervisor/shared/usb/host_keyboard.c

+#define F10     "\e[21~"
+#define F11     "\e[23~"
+#define F12     "\e[24~"
+#define PRINT_SCREEN     "\e[i"


[Just a note for myself: yes there really is a code defined for the printscreen key! xterm and rxvt-unicode both use this value]

jepler · 2024-01-18T17:00:14Z

supervisor/shared/usb/host_keyboard.c

+#define F1      "\eOP"
+#define F2      "\eOQ"
+#define F3      "\eOR"
+#define F4      "\eOS"
+#define F5      "\e[15~"


Oddly, my documentation ("man 7 urxvt") has "\e[11~" through "\e14~" for F1..F4. "\eOP" .. "\eOS" are for KP_F1..KP_F4, which do not exist on traditional PC keyboards. however both xterm and xfce4-terminal are behaving like you've implemented here, so I'm sure it's fine.

done. I've change to "\e[11~" through "\e14~"

iyalosovetsky · 2024-01-18T21:27:41Z

* if (!(mapper->flags & FLAG_CTRL) && ctrl) { continue; } * Without this, CTRL- UP, DOWN, LEFT, RIGHT do not work. But you are right about *{ HID_KEY_A, HID_KEY_Z, 'a', 0, NULL} * *...* *if (ctrl) {* * code &= 0x1f; }* that may work badly. I need some time to change the code and test it again. I think the best way is for a..z not to use that construction. About binary Or (|) you are right, I`ll correct it. About F1 ... F4 I will need time to find out more about that problem. Thank you for your time, good luck! Sorry for my english чт, 18 янв. 2024 г. в 19:00, Jeff Epler ***@***.***>:

…

***@***.**** requested changes on this pull request. ------------------------------ In supervisor/shared/usb/host_keyboard.c <#8807 (comment)> : > + if (!(mapper->flags & FLAG_CTRL) && ctrl) { + continue; + } I suspect this change doesn't have the desired effect. When ctrl+c is pressed, the result needs to be the interrupt character with value 3. This happens through the keycode mapper element { HID_KEY_A, HID_KEY_Z, 'a', 0, NULL}, which does not have FLAG_CTRL set, so the initial code is the ASCII value 'c'. Then this is modified (at line 258 before this change) with a bitmask: if (ctrl) { code &= 0x1f; } I believe, but didn't test, that with your change the rule for KEY_A..KEY_Z will "continue" at new line 297 rather than sending the interrupt character. I am guessing that you made this change so that your new code will send the desired code for ctrl+ARROW_UP and so on, but merely putting the entries in the correct order (with the FLAG_CTRL entry first) should suffice. ------------------------------ In supervisor/shared/usb/host_keyboard.c <#8807 (comment)> : > + + { HID_KEY_F1, HID_KEY_F1, 0, FLAG_STRING, F1 }, + { HID_KEY_F2, HID_KEY_F2, 0, FLAG_STRING, F2 }, + { HID_KEY_F3, HID_KEY_F3, 0, FLAG_STRING, F3 }, + { HID_KEY_F4, HID_KEY_F4, 0, FLAG_STRING, F4 }, + { HID_KEY_F5, HID_KEY_F5, 0, FLAG_STRING, F5 }, + { HID_KEY_F6, HID_KEY_F6, 0, FLAG_STRING, F6 }, + { HID_KEY_F7, HID_KEY_F7, 0, FLAG_STRING, F7 }, + { HID_KEY_F8, HID_KEY_F8, 0, FLAG_STRING, F8 }, + { HID_KEY_F9, HID_KEY_F9, 0, FLAG_STRING, F9 }, + { HID_KEY_F10, HID_KEY_F10, 0, FLAG_STRING, F10 }, + { HID_KEY_F11, HID_KEY_F11, 0, FLAG_STRING, F11 }, + { HID_KEY_F12, HID_KEY_F12, 0, FLAG_STRING, F12 }, + { HID_KEY_PRINT_SCREEN, HID_KEY_PRINT_SCREEN, 0, FLAG_STRING, PRINT_SCREEN }, + + { HID_KEY_ARROW_UP, HID_KEY_ARROW_UP, 0 , FLAG_STRING+FLAG_CTRL,CTRL_UP }, I'm more accustomed to the style where bit masks are combined with "|" instead of "+" and would prefer it that way here. ------------------------------ In supervisor/shared/usb/host_keyboard.c <#8807 (comment)> : > { HID_KEY_KEYPAD_DIVIDE, HID_KEY_KEYPAD_DECIMAL, 0, FLAG_NUMLOCK | FLAG_STRING, "/\0" "*\0" "-\0" "+\0" "\n\0" CURSOR_END SEP CURSOR_DOWN SEP CURSOR_PGDN SEP CURSOR_LEFT SEP NOTHING SEP CURSOR_RIGHT SEP CURSOR_HOME SEP CURSOR_UP SEP CURSOR_PGDN SEP CURSOR_INS SEP CURSOR_DEL}, { HID_KEY_KEYPAD_DIVIDE, HID_KEY_KEYPAD_DECIMAL, 0, 0, "/*-+\n1234567890." }, - { HID_KEY_ARROW_RIGHT, HID_KEY_ARROW_UP, 0, FLAG_STRING, CURSOR_RIGHT SEP CURSOR_LEFT SEP CURSOR_DOWN SEP CURSOR_UP }, + // { HID_KEY_ARROW_RIGHT, HID_KEY_ARROW_UP, 0, FLAG_STRING, CURSOR_RIGHT SEP CURSOR_LEFT SEP CURSOR_DOWN SEP CURSOR_UP }, + { HID_KEY_PAUSE, HID_KEY_PAUSE, 0x1a, 0, }, + { HID_KEY_PAGE_DOWN, HID_KEY_PAGE_DOWN, 0, FLAG_STRING, CURSOR_PGDN }, + { HID_KEY_PAGE_UP, HID_KEY_PAGE_UP, 0, FLAG_STRING, CURSOR_PGUP }, + { HID_KEY_HOME, HID_KEY_HOME, 0, FLAG_STRING, CURSOR_HOME }, + { HID_KEY_END, HID_KEY_END, 0, FLAG_STRING, CURSOR_END }, + { HID_KEY_INSERT, HID_KEY_INSERT, 0, FLAG_STRING, CURSOR_INS }, + { HID_KEY_DELETE, HID_KEY_DELETE, 0, FLAG_STRING, CURSOR_DEL }, + + { HID_KEY_F1, HID_KEY_F1, 0, FLAG_STRING, F1 }, When USB keycodes are consecutive, which F1..F12 and RIGHT...UP are, it saves a small amount (maybe 8 bytes per entry) of flash storage to use the form which accommodates multiple strings within one entry, { HID_KEY_ARROW_RIGHT, HID_KEY_ARROW_UP, 0, FLAG_STRING, CURSOR_RIGHT SEP CURSOR_LEFT SEP CURSOR_DOWN SEP CURSOR_UP }, This is done somewhat inconsistently in the original (for instance, INSERT..ARROW_UP are consecutive but the table does not use this fact) but it would be nice if it were done here for these 12 consecutive items, it might save about 100 bytes of flash storage. ------------------------------ In supervisor/shared/usb/host_keyboard.c <#8807 (comment)> : > @@ -78,6 +78,26 @@ struct keycode_mapper { #define CURSOR_INS "\e[2~" #define CURSOR_DEL "\e[3~" +// https://learn.microsoft.com/ru-ru/windows/console/console-virtual-terminal-sequences +// https://aperiodic.net/phil/archives/Geekery/term-function-keys/ +#define <https://aperiodic.net/phil/archives/Geekery/term-function-keys/+#define> F1 "\eOP" +#define F2 "\eOQ" +#define F3 "\eOR" +#define F4 "\eOS" +#define F5 "\e[15~" +#define F6 "\e[17~" +#define F7 "\e[18~" +#define F8 "\e[19~" +#define F9 "\e[20~" +#define F10 "\e[21~" +#define F11 "\e[23~" +#define F12 "\e[24~" +#define PRINT_SCREEN "\e[i" [Just a note for myself: yes there really is a code defined for the printscreen key! xterm and rxvt-unicode both use this value] ------------------------------ In supervisor/shared/usb/host_keyboard.c <#8807 (comment)> : > +#define F1 "\eOP" +#define F2 "\eOQ" +#define F3 "\eOR" +#define F4 "\eOS" +#define F5 "\e[15~" Oddly, my documentation ("man 7 urxvt") has "\e[11~" through "\e14~" for F1..F4. "\eOP" .. "\eOS" are for KP_F1..KP_F4, which do not exist on traditional PC keyboards. *however* both xterm and xfce4-terminal are behaving like you've implemented here, so I'm sure it's fine. — Reply to this email directly, view it on GitHub <#8807 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACPGUT6K6UVQFLCI3MFW46LYPFIL5AVCNFSM6AAAAABB5XV67CVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTQMZQGEZDAMRTGQ> . You are receiving this because you authored the thread.Message ID: ***@***.***>

iyalosovetsky · 2024-01-19T14:11:53Z

I've tested on my pico by code bellow. Look like good.

import microcontroller
#microcontroller.cpu.frequency = 120000000
print(microcontroller.cpu.frequency)
import time
import sys
import board

import usb_host

import supervisor
pp=usb_host.Port(board.GP26, board.GP27)
proceedCh=''    
while True:
  try:  
    proceedCh=''    
    while supervisor.runtime.serial_bytes_available:
        proceedCh += sys.stdin.read(1)
    if proceedCh!='':
      print('proceedCh:',proceedCh,[ord(res) for res in proceedCh])
    time.sleep(0.1)
  except KeyboardInterrupt:
        print('main: will cancel proceedCh:',proceedCh,[ord(res) for res in proceedCh])
        break

print("end.")
time.sleep(5)

leave up to jepler

this means that the test for FLAG_CTRL together with FLAG_STRING is not needed. Instead, the FLAG_STRING | FLAG_CTRL row, when encountered earlier, will automatically take precedence.

This bug was always present in the key map.

jepler

Thanks! I fixed a few small items, see the commit messages. Now good to merge, assuming the CI comes up green.

iyalosovetsky · 2024-01-28T18:30:33Z

Thanks! I have compliled , flashed and double checked on my Raspberry pico. All work fine!!

iyalosovetsky added 4 commits January 17, 2024 03:30

Update host_keyboard.c

3cc04f8

Added the ability to work with the function keys f1-f12, ctrl-left, ctrl-right, ctrl-up, ctrl-down, page down, page up, insert, delete, pause. in the format of command VT 100

Update host_keyboard.c

6a7ebaf

Update host_keyboard.c

ae7e917

Update host_keyboard.c remove trail whitespace

18dad11

tannewt previously requested changes Jan 17, 2024

View reviewed changes

jepler requested changes Jan 18, 2024

View reviewed changes

rc01

045a48a

Ihor Yalosovetskyi added 5 commits January 20, 2024 04:42

rc02 ctrl with f1-f12 ins del pgdown pgup home end

769e753

rc02 ctrl with f1-f12 ins del pgdown pgup home end

6067357

rc02 ctrl with f1-f12 ins del pgdown pgup home end

a637523

rc03 ctrl with f1-f12 ins del pgdown pgup home end bug fix

5ff4d34

rc03 ctrl with f1-f12 ins del pgdown pgup home end bug fix

5f5e4a6

tannewt requested a review from jepler January 23, 2024 00:07

jepler added 3 commits January 28, 2024 11:44

Remove commented-out lines

1983dda

Reorder FLAG_STRING | FLAG_CTRL lines

fa2e365

this means that the test for FLAG_CTRL together with FLAG_STRING is not needed. Instead, the FLAG_STRING | FLAG_CTRL row, when encountered earlier, will automatically take precedence.

Fix keypad page up

a28e929

This bug was always present in the key map.

jepler approved these changes Jan 28, 2024

View reviewed changes

jepler merged commit ebe8390 into adafruit:main Jan 28, 2024

usb_host keyboard, keys f1-f12, ctrl-left, ctrl-right, ctrl-up, ctrl-down, page down, page up, insert, delete, pause in VT100 CSI #8807

usb_host keyboard, keys f1-f12, ctrl-left, ctrl-right, ctrl-up, ctrl-down, page down, page up, insert, delete, pause in VT100 CSI #8807

Uh oh!

Conversation

iyalosovetsky commented Jan 17, 2024

Uh oh!

tannewt commented Jan 17, 2024

Uh oh!

tannewt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

iyalosovetsky commented Jan 18, 2024 via email

Uh oh!

jepler commented Jan 18, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iyalosovetsky Jan 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iyalosovetsky commented Jan 18, 2024 via email

Uh oh!

iyalosovetsky commented Jan 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jepler left a comment

Choose a reason for hiding this comment

Uh oh!

iyalosovetsky commented Jan 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

iyalosovetsky Jan 19, 2024 •

edited

Loading

iyalosovetsky commented Jan 19, 2024 •

edited

Loading

iyalosovetsky commented Jan 28, 2024 •

edited

Loading