Function KEY() is broken (or at least does not work as expected)

[kochs-online]

Boy, did I have a hard time figuring this one out! 😅 BASICally (pun intended) had to learn Assembler first…

So, I was trying to use KEY() for target pet8032 and because the list of keyboard scancodes does not include the PET/CBM [chiclet, graphics and business] keyboard I tried to find out which scancodes I would have to use.

After learning about all the quirks of the Commodore keyboard matrix and a lot of trial & error I was pretty sure 1007 ($03EF) would be the scancode for the RETURN key on a PET/CBM business keyboard: It's wired in row $03 of the matrix and will pull down bit 4, resulting in a bit mask of 11110111 ($EF) when pressed.

But to my surprise KEY(1007) returns FALSE while pressing RETURN. What's happening?!

After quite a few more hours of experimenting and scratching my head I ended up writing a small sample programm:

KEY(1007)

And then disassembled it (using the WFDis Interactive 6502 Disassembler):

[…]   ; launcher and some initialization
L0422               lda #$ef
                    ldy #$03
                    sty $e810   ; store $03 to keyboard row select port (DDR A)
                    and $e812   ; accumulator AND bit mask of keyboard column bits (DDR B)
                    bne L0432
                    lda #$ff   ; return TRUE
                    bne L0439
L0432               lda #$00   ; return FALSE
                    bne L0439
                    jmp L0439
L0439               jmp $b3ff   ; exit to BASIC

This will always return FALSE if anything else than 0 (well almost; see below) is written to the accumulator and used for the AND opcode in $0429.

Example:

    11110111   [accumulator]
AND 11110111   [keyboard column bits at $E812]
    ========
    11110111   [result in accumulator]

This will NOT set the zero flag of the status register and BNE at $042C will trigger and branch to $0432.

Using the inverse binary value of the key's bit mask (here: 00001000) does work, but this is rather counter-intuitive, I guess.

Replacing opcode AND with CMP should work as intended.

[neilsf]

Nice catch. This shows how poorly XC=BASIC was tested for the PET. The scan routine was designed for the C64 but does not work on the PET. The correct routine for the PET would be (in _lib/iokeyboard.asm)

IF TARGET & pet
 MAC F_key_word ; @push @pull
    IF !FPULL
      pla
      sta IO_KEYW
      pla
    ELSE
      sty IO_KEYW
    ENDIF
    bit IO_KEYR
    bne .f
    ptrue
    bne .q
.f
    pfalse
.q  
  ENDM
ENDIF

... and the logic for scan codes must be reversed. The scan code for the RETURN key would be $0310 or %0000 0011 0001 0000, that is row 3 and column 4:

PRINT "press return"
DO
LOOP UNTIL KEY($0310)

I'll fix this with the next release and I'll try to find some time to calculate keyboard scan codes for the PET graphics/business keyboard and add them to the docs. My time is very limited unfortunately, if anyone in the community has the time and expertise to contribute a fix or any improvements (including documentation), it would be incredibly helpful.

[neilsf]

...actually the coolest solution would be to include predefined constants for every target, so that coders wouldn't have to look up scan codes in the table at all. For example:

PRINT "press return"
DO
LOOP UNTIL KEY(KEY_RETURN) ' predefined as $0310 on the PET

[kochs-online]

Thanks for the quick reply! Your fix using BIT works for me.

While changing the code in [my local copy of] _keyboard.asm I noticed a superfluous ENDIF in line 81 that I also removed without changes to the resulting machine code.

How would you go about defining the proposed constants for every target? If you could provide a template I can try to add constants for all the PET keys.

[neilsf]

While changing the code in [my local copy of] _keyboard.asm I noticed a superfluous ENDIF in line 81 that I also removed without changes to the resulting machine code.

Interestingly, Dasm did not complain about the extra ENDIF :-)

How would you go about defining the proposed constants for every target?

The easiest would be to add files like keycodes_\<target>.bas to the lib/ folder. For example:

' keycodes_c64.bas
SHARED CONST KEY_A = 64772
SHARED CONST KEY_B = 63248
SHARED CONST KEY_CTRL = 32516

This way userland programs could simply do INCLUDE keycodes_<target>.bas at the top and access the predefined constants. Note: modules in the lib/ folder get automatically resolved, without specifying the file path.

The code for a PET key would be row << 8 + mask - the entire list could be easily generated with Perl, Python, etc.

[kochs-online]

Interestingly, Dasm did not complain about the extra ENDIF :-)

I was wondering about that as well and shied away from removing it at first because I expected DASM to complain if it was superfluous.

The easiest would be to add files like keycodes.bas_ to the lib/ folder. For example:

[…]

This way userland programs could simply do INCLUDE keycodes_<target>.bas at the top and access the predefined constants.

I see. I thought it might be possible to evaluate a target by code somehow. In fact I wished for something like that before to write code more platform-independently.

Not to divert from the issue at hand, but would it be feasible to extend the INCLUDE statement with an optional parameter for target(s)? For example:

INCLUDE "keycodes_c64.bas", "C64"
INCLUDE "keycodes_pet.bas", "PET8032"

Or maybe even using a bitmask:

INCLUDE "keycodes_pet.bas", PET4032 OR PET8032

Note: modules in the lib/ folder get automatically resolved, without specifying the file path.

Good to know!

[neilsf]

I thought it might be possible to evaluate a target by code somehow.

XC=BASIC does not provide any kind of preprocessing (yet?), but you can use a generic preprocessor like gpp as part of your toolchain to resolve things like this:

#ifeq target c64
INCLUDE "keycodes_c64.bas"
#endif

[kochs-online]

While creating the lists of pre-defined constants for all the targets, I noticed another issue:

Writing to Data Direction Register A at $E810 currently also sets the high nibble (bits 4-7) at this address. But only the low nibble (bits 0-3) is used for keyboard row selection. The high nibble is used for tape I/O, IEEE488 EOI and diagnostic sense (jump to TIM) (see 6502.org: Commodore PET programming model) and I think it should not be changed to avoid side effects.

What do you think of this approach?

lda $e810       ; load DDR A to accumulator
and #%11110000   ; set low nibble to 0, but keep high nibble
sta $e810
pla             ; pull KEY() parameter from stack
ora $e810       ; set low nibble according to KEY() parameter, but keep high nibble
sta $e810

This is somewhat similar to the ROM routine (see <NowGoBang!> PET Keys — Series 2001 Edition: How To), but assembler is still really new to me, so maybe there's a faster and/or more elegant way to do this?!

Of course this would require the scancode's first byte to have 1111 as high nibble, so e.g. $0310 for RETURN becomes $F310.

Edit: Fixed missing # in assembler and statement.

[kochs-online]

Attached are the pre-defined constants for all the targets: keycodes.zip [edit: see comment below]

Here's a list of all files and a short description:	File	Description
keycodes_pet_common.bas	Scancodes that work on all PET/CBM models
keycodes_pet_graphics.bas	Scancodes for the PET chiclet & graphics keyboard
keycodes_pet_business.bas	Scancodes that work on both UK & US versions of the PET business keyboard
keycodes_pet_business_uk.bas	Scancodes for the PET business keyboard (UK layout)
keycodes_pet_business_us.bas	Scancodes for the PET business keyboard (US layout)
keycodes_vic20_common.bas	Scancodes that work on all keyboards of VIC 1001/VC 20/VIC 20, C64/C128 and 264 series
keycodes_vic20.bas	Scancodes for the VIC 1001/VC 20/VIC 20
keycodes_c64_common.bas	Scancodes that work on C64/C128 and 264 series
keycodes_c64.bas	Scancodes for the C64/C128
keycodes_264.bas	Scancodes for the 264 series

The scancodes for all PET/CBM keyboards need to have the high nibble of the first byte set to 1111 0000 [edit: see comment below] to avoid side effects with tape I/O, IEEE488 EOI and diagnostic sense. So this requires adding the AND/OR code shown in my comment above (or something similar) for them to work.

[neilsf]

Awesome job, thank you. I'll add this to version 3.2.0.

[kochs-online]

lda $e810       ; load DDR A to accumulator
and #%11110000   ; set low nibble to 0, but keep high nibble
sta $e810
pla             ; pull KEY() parameter from stack
ora $e810       ; set low nibble according to KEY() parameter, but keep high nibble
sta $e810

I tested this approach and it works apart from a minor bug (missing #) that I fixed in the edited comment above as well.

But I also noticed that I had a brain fart concerning the OR logic: Of course the high nibble of the scancode needs to be set to 0000 so it will not influence the high nibble already set in $E810 when OR'ed:

   00000111   [accumulator]
OR 11010000   [$E810]
   ========
   11010111   [result in accumulator]

I fixed this in the attached constant files: keycodes.zip

For documentation, this is the INCLUDE hierarchy of the constant files in the zip archive (same as described in the comment above):

── keycodes_pet_common.bas
   ├─ keycodes_pet_graphics.bas
   └─ keycodes_pet_business.bas
      ├─ keycodes_pet_business_uk.bas
      └─ keycodes_pet_business_us.bas
── keycodes_vic20_common.bas
   ├─ keycodes_vic20.bas
   └─ keycodes_c64_common.bas
      ├─ keycodes_c64.bas
      └─ keycodes_264.bas

[kochs-online]

And here is a XC=BASIC function with some assembler that can be used to find scancodes on the PET/CBM (e.g. for internationalized keyboards) or build a menu selectable by key:

FUNCTION getkey AS WORD () STATIC
  DIM e810 AS BYTE
  DIM e812 AS BYTE

  DO
    IF e810 >= 9 THEN
      e810 = 0
    ELSE
      e810 = e810 + 1
    END IF

    ASM

      lda $E810     ; load DDR A
      and #%11110000    ; set low nibble to 0, but keep high nibble
      sta $E810
      lda {e810}    ; load row select variable
      ora $E810     ; set low nibble according to row select variable, but keep high nibble
      sta $E810
      lda $E812     ; load keyboard column bits
      sta {e812}

    END ASM
  LOOP UNTIL e812 <> 255   ' any key pressed?

  RETURN e810 * 256 + e812
END FUNCTION