DCPUTeam / DCPUToolchain

[ARCHIVED] The code repository for the DCPU-16 Toolchain.
http://dcputoolcha.in/
MIT License
96 stars 14 forks source link

keyboard hwi problem #215

Closed orlof closed 11 years ago

orlof commented 11 years ago

...here is my daily bug report

Version: http://bb.dcputoolcha.in/builds/windows/dists/dcputoolchain-windows--ce67ac64cfa294bb483179603419ebee9a296714.exe

Here is the code ( not a standalone example, but just part of admiral's code). Idea is to wait that button is released.

:win_getchar_release
    set b, c
:win_getchar_release_loop
    set a, 2
    hwi [keyboard]
    ifn c, FALSE
        set pc, win_getchar_release_loop

    set pc, pop

and here it gets stuck in above loop.

0x1E0A: SET B C
--------------------------------
A:   0x0110   [A]:   0xCD00
B:   0x0080   [B]:   0x7C72
C:   0x0080   [C]:   0x7C72
X:   0xCD00   [X]:   0xFF80
Y:   0x01AD   [Y]:   0x0000
Z:   0xFFBF   [Z]:   0xFFCE
I:   0x34A4   [I]:   0x0000
J:   0xCE80   [J]:   0x0000
PC:  0x1E0A   SP:    0xFFBC
EX:  0x0000   IA:    0x0000
IRQ ENABLED:      false
IRQ COUNT:   0x0000
CYCLES TO SLEEP: 0x0000

0x1E0B: SET A NXT_LIT (0x0002) 
--------------------------------
A:   0x0002   [A]:   0x7F01
B:   0x0080   [B]:   0x7C72
C:   0x0080   [C]:   0x7C72
X:   0xCD00   [X]:   0xFF80
Y:   0x01AD   [Y]:   0x0000
Z:   0xFFBF   [Z]:   0xFFCE
I:   0x34A4   [I]:   0x0000
J:   0xCE80   [J]:   0x0000
PC:  0x1E0C   SP:    0xFFBC
EX:  0x0000   IA:    0x0000
IRQ ENABLED:      false
IRQ COUNT:   0x0000
CYCLES TO SLEEP: 0x0000

0x1E0D: HWI NXT (0x0003) 
--------------------------------
A:   0x0002   [A]:   0x7F01
B:   0x0080   [B]:   0x7C72
C:   0x00CD   [C]:   0x7C20
X:   0xCD00   [X]:   0xFF80
Y:   0x01AD   [Y]:   0x0000
Z:   0xFFBF   [Z]:   0xFFCE
I:   0x34A4   [I]:   0x0000
J:   0xCE80   [J]:   0x0000
PC:  0x1E0E   SP:    0xFFBC
EX:  0x0000   IA:    0x0000
IRQ ENABLED:      false
IRQ COUNT:   0x0000
CYCLES TO SLEEP: 0x0003

0x1E0F: IFN C NXT_LIT (0x0000) 
--------------------------------
A:   0x0002   [A]:   0x7F01
B:   0x0080   [B]:   0x7C72
C:   0x00CD   [C]:   0x7C20
X:   0xCD00   [X]:   0xFF80
Y:   0x01AD   [Y]:   0x0000
Z:   0xFFBF   [Z]:   0xFFCE
I:   0x34A4   [I]:   0x0000
J:   0xCE80   [J]:   0x0000
PC:  0x1E10   SP:    0xFFBC
EX:  0x0000   IA:    0x0000
IRQ ENABLED:      false
IRQ COUNT:   0x0000
CYCLES TO SLEEP: 0x0000

0x1E11: SET PC NXT_LIT (0x1E0A) 
--------------------------------
A:   0x0002   [A]:   0x7F01
B:   0x0080   [B]:   0x7C72
C:   0x00CD   [C]:   0x7C20
X:   0xCD00   [X]:   0xFF80
Y:   0x01AD   [Y]:   0x0000
Z:   0xFFBF   [Z]:   0xFFCE
I:   0x34A4   [I]:   0x0000
J:   0xCE80   [J]:   0x0000
PC:  0x1E0A   SP:    0xFFBC
EX:  0x0000   IA:    0x0000
IRQ ENABLED:      false
IRQ COUNT:   0x0000
CYCLES TO SLEEP: 0x0000

0x1E0B: SET A NXT_LIT (0x0002) 
--------------------------------
A:   0x0002   [A]:   0x7F01
B:   0x0080   [B]:   0x7C72
C:   0x00CD   [C]:   0x7C20
X:   0xCD00   [X]:   0xFF80
Y:   0x01AD   [Y]:   0x0000
Z:   0xFFBF   [Z]:   0xFFCE
I:   0x34A4   [I]:   0x0000
J:   0xCE80   [J]:   0x0000
PC:  0x1E0C   SP:    0xFFBC
EX:  0x0000   IA:    0x0000
IRQ ENABLED:      false
IRQ COUNT:   0x0000
CYCLES TO SLEEP: 0x0000

Spec says:

 2 | Set C register to 1 if the key specified by the B register is pressed, or
   | 0 if it's not pressed

C should be 0 or 1 after HWI - not 0x00CD as above.

hach-que commented 11 years ago

Register C currently holds a character value inside it (I believe the current key). I can't find the DCPU-16 keyboard spec from my tablet, so @jdiez17 will need to confirm which is correct here.

orlof commented 11 years ago

Perhaps even bigger problem here (that I forgot to mention in the original report) is that I was NOT pressing any buttons, but the hwi still set c!=0.

coffeyk commented 11 years ago

This sounds like a bug that was actually fixed in ce67ac64cfa294bb483179603419ebee9a296714.

It was introduced in d4d009fd150ad1ba227e4e0ea87b1366cf2498e2, which was the previous build. The bug caused the keyboard not to set C=0 when no keypresses remained buffered in the A=1 HWI.

I wrote a test program based on your example. When a key is pressed down, it prints the key's hex value to the screen. When released, the string "UP!" is printed. Then it goes to wait for another key press. http://0x10co.de/bnn50

orlof commented 11 years ago

I tested with your test program and it seems that SHIFTed symbols dont provide UP event in windows. Thus admiral freezes after edit() because ) is SHIFTed.

coffeyk commented 11 years ago

My proposed solution is that only the physical keys on the keyboard can provide UP or DOWN events. Characters that can only be accessed with the Shift modifier will only provide a TYPED event to be added to the keyboard buffer. They will always report their status as UP when requested in mode 2.

Here is the definition of keyboard mode 3:

Turns on interrupts with the message specified by the B register, if B is zero it turns off the interrupts. When interrupts are enabled, the keyboard will trigger an interrupt when one or more keys have been pressed, released, or typed.

The spec makes the point to differentiate pressed, released, and typed as different events. It doesn't make sense for Shifted characters to cause UP/DOWN events because they aren't actual keys. They are a combination of two keys, which provide their own UP and DOWN events.

hach-que commented 11 years ago

I think the raw access of keys is important. For example, when Shift-A is hit, you don't want to be forced to detect this by comparing the character 'A'; likewise for Ctrl and Alt.

orlof commented 11 years ago

DTEMU keyboard has multiple issues. Here is my summary from admiral point of view:

Following should work:

:start
SET A, 1
:wait_for_any_key_down
HWI kbd  ; return key in C
IFE C, 0
  SET PC, wait_for_any_key_down
...process key down
SET A, 2
SET B, C
:wait_for_specific_key_up
  HWI kbd  ; should return 0 if key is released or 1 if key is still down
  IFE C, 1
    SET PC, wait_for_specific_key_up
SET PC, start

Currently above code fails because HWI (with A=2) does not return 0 or 1 as in the specs and because SHIFTed symbols (e.g. '(') break the HWI (with A=2) so that is will always return 0x00CD.

Psycocoffey's proposal is not optimal for me because key-up for '(' would always return TRUE and key-repeat would fill the kbd-buffer too fast when compared to DCPU processing capacity.

Hach-que's proposal is not optimal either, because Admiral doesn't know the users keyboard layout. Admiral shouldn't know what key combination is required from the users keyboard to detect key up or down.

orlof commented 11 years ago

...my interpretation of keyboard specification is that "A=1 HWI kbd" only generates printable (ASCII 0x20-0x7F) characters. These characters should all work as they were physical keys in the keyboard. Thus calling "A=2 HWI" with any of printable character should return 1 only if the physical key combination required to produce that character is down. Still HWI with A=2 can also be called with non-printable control keys as SHIFT, CONTROL or ARROWS...

I think DCPU keyboard is mend to work with characters instead of scancodes. The keyboard layout sw (scancode -> character) is supposed to be provided by the environment.

patflick commented 11 years ago

thanks to @psycocoffey 's pull requests and @orlof 's feedback this issue is now all fixed and closed. Thanks a lot guys!