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/io_keyboard.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