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

Add support for the DEC layout Televideo Linear Space Invader keyboards to ibmpc_usb. #711

Merged

Conversation

purdeaandrei
Copy link
Contributor

See this thread for which keyboards I am referring to:
https://deskthority.net/viewtopic.php?f=2&t=25763

(Many of these keyboards are completely unbranded on the outside, and only the
PCB reads "TELEVIDEO ANSI Scan")

@tmk Are the .c files deprecated now, and I can safely ignore? Or do I need to port my changes to the .c files too?

…ds to ibmpc_usb.

See this thread for which keyboards I am referring to:
https://deskthority.net/viewtopic.php?f=2&t=25763

(Many of these keyboards are completely unbranded on the outside, and only the
PCB reads "TELEVIDEO ANSI Scan")
@tmk
Copy link
Owner

tmk commented Nov 4, 2021

Interesting scan code and keyboard id.
As for Keyboard ID AB91 it can conflict with IBM 5576-003, which is not tested at all yet, though.
We will have to look into how to add support those keyboards with case.

Doesn't the keyboard work well with Code Set 2?
The keyboard starts up with CS2 and you have to change Code Set to 3, right?

Can you make scan code table of CS3(and CS2 if possible) for the keyboard for future reference?
https://github.com/tmk/tmk_keyboard/wiki/IBM-PC-Keyboard-Converter#terminal---scan-code-set-3

And it would be helpful if you can share good photo of the keyboard enough to recognize key legends.

Right. ibmpc_usb.c file is not used currently, you don't need to change it.

@purdeaandrei
Copy link
Contributor Author

Code set 0x82 is not valid for this DEC Televideo keyboard, and it does not accept it.
Assuming the original code for the 5576-003 is correct, I would expect auto-detection to work correctly the way I implemented it, that is:

  1. If using an 5576-003 keyboard, it will accept code set 0x82, and the converter will then use code set 2 with the 5576's modifications
  2. If using a DEC Televideo keyboard, it will not accept code set 0x82, it will print "NG" (which I presume means Not Good), and then my code runs, and tries to select code set 3, with the DEC Televideo adjustments.

I looped through all the possible code sets, and this keyboard supports the following code sets:

  • Code set 1 - The keyboard starts up with PS2 protocol + code set 1, not using XT signaling. I did not use this code set for my implementation, because fewer keys are mapped by default (when compared to cs3), so I would have had to implement a longer translate_...() function.
  • Code set 2 - This works only partially, but some distinct keys send the exact same code, with no way to differentiate them, so for that reason we should not use this code set with this keyboard.
  • Code set 3 - All keys send distinct keycodes, and the largest number of keys are already mapped by default, so this makes the translate_...() function the shortest. That's why I chose Code set 3, instead of code set 1.

Please see Code Set 3 of this keyboard documented below: On the top half I've written the key meanings mapped by me, and on the bottom half the set-3 actual raw codes sent by the keyboard.

  ,-------------------.  ,-------------------.  ,---------------.   ,-----------. ,---------------.
  |F1 |F2 |F3 |F4 |F5 |  |F6 |F7 |F8 |F9 |F10|  |F11|F12|F13|F14|   |PrS|ScrLock| |Pau|VDn|VUp|Mut|
  `-------------------'  `-------------------'  `---------------'   `-----------' `---------------'
,-----------------------------------------------------------------. ,-----------. ,---------------.
|Esc|  `|  1|  2|  3|  4|  5|  6|  7|  8|  9|  0|  -|  =|Bs1  |Bs2| |Ins|Hom|PgU| |NmL|  /|  *|  -|
|-----------------------------------------------------------------| |-----------| |---------------|
    |Tab  |  Q|  W|  E|  R|  T|  Y|  U|  I|  O|  P|  [|  ]| Entr|   |Del|End|PgD| |  7|  8|  9|  +|
 |----------------------------------------------------------    |   `-----------' |---------------|
 |Ctl|CapsL|  A|  S|  D|  F|  G|  H|  J|  K|  L|  ;|  '| \ |    |       | Up|     |  4|  5|  6|KP,|
 |--------------------------------------------------------------|_  ,-----------. |---------------|
 | Shft |  <|  Z|  X|  C|  V|  B|  N|  M|  ,|  .|  /|  Shift |RCtl| |Lef|Dow|Rig| |  1|  2|  3|Ent|
 `----------------------------------------------------------------' `-----------' |-----------|   |
      |LWin|Lalt|      Space                |Ralt|Rwin|                           |      0|  .|   |
      `-----------------------------------------------'                           `---------------'


  ,-------------------.  ,-------------------.  ,---------------.   ,-----------. ,---------------.
  | 07| 0F| 17| 1F| 27|  | 2F| 37| 3F| 47| 4F|  | 56| 5E| 85| 86|   | 87| 88    | | 89| 8A| 8B| 8C|
  `-------------------'  `-------------------'  `---------------'   `-----------' `---------------'
,-----------------------------------------------------------------. ,-----------. ,---------------.
| 08| 0E| 16| 1E| 26| 25| 2E| 36| 3D| 3E| 46| 45| 4E| 55| 66  | 57| | 6E| 67| 64| | 8D| 8E| 8F| 90|
|-----------------------------------------------------------------| |-----------| |---------------|
    | 0D  | 15| 1D| 24| 2D| 2C| 35| 3C| 43| 44| 4D| 54| 5B|  5A?|   | 65| 6D| 6F| | 6C| 75| 7D| 84|
 |----------------------------------------------------------    |   `-----------' |---------------|
 | 11|  14 | 1C| 1B| 23| 2B| 34| 33| 3B| 42| 4B| 4C| 52| 5C|    |       | 63|     | 6B| 73| 74| 7C|
 |--------------------------------------------------------------|_  ,-----------. |---------------|
 |  12  | 13| 1A| 22| 21| 2A| 32| 31| 3A| 41| 49| 4A|    59  | 77 | | 61| 60| 6A| | 69| 72| 7A| 79|
 `----------------------------------------------------------------' `-----------' |-----------|   |
      | 91 | 19 |      29                   | 39 | 92 |                           |     70| 71|   |
      `-----------------------------------------------'                           `---------------'

Please see here for a Keyboard Layout Editor info.json with (I think) Danish original legends on this keyboard,
copy-paste the contents of this gist:
https://gist.github.com/purdeaandrei/81f4dbb4272551fff627c73ebb6923dd
Into the Raw Data section of this website: http://www.keyboard-layout-editor.com/

I don't think there were any english-layout DEC Televideo keyboards among the AZERTY keyboard groupbuy
from deskthority I referenced in the PR.

But take a look at this, different, but also DEC-layout keyboard:
http://www.wickensonline.co.uk/rc2012sc/wp-content/uploads/2014/07/20140724_215341_resized.jpg
This is a different keyboard, and this PR is not intended to support this keyboard, but at least the legends
on it are in english, and I think the layout is pretty much the same.

Many keys don't really have a modern-day equivalent, so I have opted to mostly ignore what
the legends say, and to make the layout physically close to a modern day layout so at least
by default people can make use of muscle memory. Please let me know if you think this was a
wrong choice.

@tmk
Copy link
Owner

tmk commented Nov 5, 2021

Thank you for great writeup and very clear explanation!
It sounds totally reasonable now, especially about choice of code set and 5576/Televideo detection.
Also your key mapping seems to be almost suitable for Keymap Editor layout as possible.

I found one thing on the mapping.
As for Baspace part 'Backspace and F15' is currently mapped there according to translate_televideo_dec_cs3() code.

,-----------------------------------------------------------------. ,-----------. ,---------------.
|Esc|  `|  1|  2|  3|  4|  5|  6|  7|  8|  9|  0|  -|  =|Bsp  |F15| |Ins|Hom|PgU| |NmL|  /|  *|  -|
|-----------------------------------------------------------------| |-----------| |---------------|

I assume users would use this Keymap Editor to edit keymap for Televideo DEC style.
http://www.tmk-kbd.com/tmk_keyboard/editor/unimap/?ibmpc_usb_32u4
Then, 'JPY and Backspace' would be a bit intuitive, instead of 'Backspace and F15'?
(JPY = JIS ¥)

,-----------------------------------------------------------------. ,-----------. ,---------------.
|Esc|  `|  1|  2|  3|  4|  5|  6|  7|  8|  9|  0|  -|  =|JPY  |Bsp| |Ins|Hom|PgU| |NmL|  /|  *|  -|
|-----------------------------------------------------------------| |-----------| |---------------|

These scan code would be translated in this case.
0x66 -> 0x5D
0x57 -> 0x66

How do you think?

@purdeaandrei
Copy link
Contributor Author

Yeah, I really wasn't sure what to do there, there's two backspace keys, the left-most one is 1.5u wide, and has the backspace symbol on the key, while the right-most one is 1u wide, and has the words "Back Space" on the key.
I couldn't map backspace to both, cause then the user would lose the ability to map different actions.

The reason I chose backspace on the left, was cause the left-most backspace is 1.5 wide, and most people's muscle memory would make them prefer the 1.5u key for actual backspace functionality.

But I guess you're right, it's more important to make remapping more intuitive in this case.
I will make the edits later tonight.

Is there a repository where I could contribute to the editor webpage, to add a DEC layout to the drop-down? (If you want that)

@purdeaandrei
Copy link
Contributor Author

Adjusted, as discussed

@tmk
Copy link
Owner

tmk commented Nov 6, 2021

Thank you for the fix. I'll merge this PR.

You can acess codes of Keymap Editor at gh-pages branch. Current Editor can't manage with many editor layouts and firmwares well enough. I will have to find be better way to organize editor layouts.
https://github.com/tmk/tmk_keyboard/tree/gh-pages

I hope that users can edit keymap using default editor layout somehow with referring to this table to know how Televideo DEC keyboard is mapped on Editor.
https://github.com/tmk/tmk_keyboard/wiki/IBM-PC-Keyboard-Converter#televideo-990995-dec-style

I usually do without adding new specific editor layout preferably when adding support for new keyboard.
I think you don't have to add layout for Televideo necessarily, but I'll be happy to merge if you can contribute.

@tmk tmk merged commit 90d4f42 into tmk:master Nov 6, 2021
@purdeaandrei
Copy link
Contributor Author

Thanks for the merge.

Could you remove the question mark from Enter's keycode? I left that there by accident (| 5A?| -> | 5A |)

Can I propose another alternative for the backspace area?
How about making the key next to the bottom row of enter NUHS (like in iso layouts), and making the two backspaces be backspace, and backslash? What do you think?:

,-----------------------------------------------------------------. ,-----------. ,---------------.
|Esc|  `|  1|  2|  3|  4|  5|  6|  7|  8|  9|  0|  -|  =|Bacsp| \ | |Ins|Hom|PgU| |NmL|  /|  *|  -|
|-----------------------------------------------------------------| |-----------| |---------------|
    |Tab  |  Q|  W|  E|  R|  T|  Y|  U|  I|  O|  P|  [|  ]| Entr|   |Del|End|PgD| |  7|  8|  9|  +|
 |----------------------------------------------------------    |   `-----------' |---------------|
 |Ctl|CapsL|  A|  S|  D|  F|  G|  H|  J|  K|  L|  ;|  '|NUH|    |       | Up|     |  4|  5|  6|KP,|

@tmk
Copy link
Owner

tmk commented Nov 6, 2021

That seems to be acceptable and reasonable to me.

Is this change what you mean?

diff --git a/converter/ibmpc_usb/ibmpc_usb.cpp b/converter/ibmpc_usb/ibmpc_usb.cpp
index f9fc9202..ab875aa1 100644
--- a/converter/ibmpc_usb/ibmpc_usb.cpp
+++ b/converter/ibmpc_usb/ibmpc_usb.cpp
@@ -1152,8 +1152,8 @@ uint8_t IBMPCConverter::translate_televideo_dec_cs3(uint8_t code) {
         case 0x91: return 0x01; // LGUI
         case 0x92: return 0x09; // RGUI
         case 0x77: return 0x58; // RCTRL
-        case 0x57: return 0x66; // Backspace
-        case 0x66: return 0x5D; // JPY
+        case 0x57: return 0x5C; // Backslash
+        case 0x5C: return 0x53; // Non-US Hash
         case 0x7c: return 0x68; // Kp Comma
     }
     return code;

@purdeaandrei
Copy link
Contributor Author

Yeah, that's it, I just tested it and works as expected

@tmk
Copy link
Owner

tmk commented Nov 6, 2021

OK. I'll commit the change then.

@purdeaandrei
Copy link
Contributor Author

Thanks!

@tmk
Copy link
Owner

tmk commented Nov 6, 2021

Updated in repo.

Keymap Editor should work with Televideo DEC keyboard now. Let me know if you have any problem.
http://www.tmk-kbd.com/tmk_keyboard/editor/unimap/?ibmpc_usb_32u4

@purdeaandrei
Copy link
Contributor Author

Tested with keymap editor too, and works fine.
Thanks!

@purdeaandrei purdeaandrei deleted the f_add_support_for_televideo_dec_board_using_set3 branch November 6, 2021 19:17
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 this pull request may close these issues.

2 participants