add tk10 keyboard device for apple ii clones #13004

goldnchild · 2024-11-21T09:59:56Z

I found a schematic of the TK10 keyboard and made a device for it.

The rom can be found as part of the am64 apple II clone in apple2.cpp.

rb6502 · 2024-11-21T13:30:50Z

If it's part of the AM64 machine, then you should just hook it up to the AM64. Also, unless the keyboard was also used on non-Apple machines you can just put it in src/mame/apple.

cuavas

This isn’t much use if it isn’t actually attached to anything. The device won’t be linked since there’s nothing referenced in the translation unit, so it won’t appear in -listxml or detect things with -romident, etc.

Also it should be in src/mame/apple.

cuavas · 2024-11-21T15:23:03Z

src/devices/machine/tk10_keyboard.cpp

+	m_tk10cpu->p1_in_cb().set([this] () -> u8
+	{
+		for (int i = 0; i < 8; i++)
+		{
+			if (!BIT(m_tk10cpu->p2_r(), i)) return m_keyboard[i]->read();
+			// read key matrix:
+			// a low value in p2 bit will drive bit of p1 low when switch contact made
+		}
+		return 0xff;
+	});


You should lift the p2_r out of the loop.

cuavas · 2024-11-21T15:25:50Z

src/devices/machine/tk10_keyboard.cpp

+	PORT_START("P21")
+	PORT_BIT(0x001, IP_ACTIVE_LOW, IPT_KEYBOARD) PORT_CODE(KEYCODE_ENTER_PAD)   //  _ SHIFT=^
+	PORT_BIT(0x002, IP_ACTIVE_LOW, IPT_KEYBOARD) PORT_CODE(KEYCODE_PGUP)   // ]
+	PORT_BIT(0x004, IP_ACTIVE_LOW, IPT_KEYBOARD) PORT_CODE(KEYCODE_CLOSEBRACE)  // [
+	PORT_BIT(0x008, IP_ACTIVE_LOW, IPT_KEYBOARD) PORT_CODE(KEYCODE_PGDN)        // @ SHIFT=^
+	PORT_BIT(0x010, IP_ACTIVE_LOW, IPT_KEYBOARD) PORT_CODE(KEYCODE_SLASH_PAD)   // /
+	PORT_BIT(0x020, IP_ACTIVE_LOW, IPT_KEYBOARD) PORT_CODE(KEYCODE_MINUS_PAD)
+	PORT_BIT(0x040, IP_ACTIVE_LOW, IPT_KEYBOARD) PORT_CODE(KEYCODE_PLUS_PAD)
+	PORT_BIT(0x080, IP_ACTIVE_LOW, IPT_KEYBOARD) PORT_CODE(KEYCODE_ASTERISK)
+
+	PORT_START("P22")
+	PORT_BIT(0x001, IP_ACTIVE_LOW, IPT_KEYBOARD) PORT_CODE(KEYCODE_BACKSLASH) // BACKSLASH


Do these keys really have blank key caps? It’s bad to have inputs that lack either PORT_NAME or PORT_CHAR.

ok, tried to label them all

cuavas · 2024-11-21T15:26:58Z

src/devices/machine/tk10_keyboard.h

+#ifndef MAME_TK10_KEYBOARD_H
+#define MAME_TK10_KEYBOARD_H


This doesn’t match the path.

ok, moved to src/mame/apple

cuavas · 2024-11-21T15:27:33Z

src/devices/machine/tk10_keyboard.h

+	INPUT_CHANGED_MEMBER( tk10_func );
+	INPUT_CHANGED_MEMBER( tk10_reset );
+
+	auto reset_write_cb() { return m_reset_write.bind(); }
+	auto data_write_cb() { return m_data_write.bind(); }


Conventionally, configuration members come before other stuff.

cuavas · 2024-11-21T15:28:50Z

src/devices/machine/tk10_keyboard.h

+	// device-level overrides
+	virtual void device_start() override;
+	virtual void device_add_mconfig(machine_config &config) override;
+	virtual ioport_constructor device_input_ports() const override;
+	virtual const tiny_rom_entry *device_rom_region() const override;


Please don’t copy/paste this meaningless comment. Also, please add ATTR_COLD for typical cold lifecycle member functions as @holub has done for most existing devices.

goldnchild · 2024-11-22T16:23:06Z

If it's part of the AM64 machine, then you should just hook it up to the AM64. Also, unless the keyboard was also used on non-Apple machines you can just put it in src/mame/apple.

I wanted to check with you on changes to apple2.cpp.

adding a member variable to hold the value of tk10 reset line.

void apple2_state::tk10_data_write(u8 data)
{
if (m_maincpu->total_cycles() < 25000)
{
return;
}

	if (data & 0x80)
	{
		m_transchar = data & 0x7f;
		m_strobe = 0x80;
	}

}

void apple2_state::tk10_reset_write(u8 data)
{
m_tk10_reset_data = data;
}

TIMER_DEVICE_CALLBACK_MEMBER(apple2_state::apple2_interrupt_tk10)
{
basically the same as the regular function, only this time using the tk10 reset line
}

void apple2_state::am64(machine_config &config)
{
apple2p(config);
m_scantimer->configure_scanline(FUNC(apple2_state::apple2_interrupt_tk10), "screen", 0, 1);

tk10_keyboard_device &tk10(TK10_KEYBOARD(config, "tk10", 0));
tk10.data_write_cb().set(FUNC(apple2_state::tk10_data_write));
tk10.reset_write_cb().set(FUNC(apple2_state::tk10_reset_write));

}

I didn't want to touch any of the existing code, like removing the normal keyboard.
// config.device_remove(A2_KBDC_TAG);

When you switch the keyboard selection under the Input Menu, the normal keyboard can be deactivated and the tk10 can be activated.

cuavas

You really should move the systems with the different keyboard to a derived class so any additional members don’t pollute the machines that don’t have it. You can also conveniently override machine_start to save additional members.

If these machines don’t have the “standard” keyboard in addition to the TK-10 keyboard, they shouldn’t be emulated as having both (if they could be used with one or the other, it needs to be slotted, but that’s another story).

You could make the driver state class get the keyboard reset input via a virtual member function to avoid copy/pasting the the scan line callback.

cuavas · 2024-12-01T16:31:48Z

src/mame/apple/apple2.cpp

+		if (m_maincpu->total_cycles() < 25000)
+		{
+			return;
+		}


This is definitely the wrong way to do this. What’s actually going on here? Is there a monostable that holds the microcontroller reset for a while after power-on? If there is, is it also affected by reset? Or is there some weird glue logic that just ignores data from the keyboard after power-on/reset? Either way you should probably be using a timer.

This has nothing to do with hardware and everything to do with MAME. The keypress to start the emulation past the info screen leaks into the emulated system, and that was causing issues with disks where you boot them and then press a key to skip a title screen or select a menu item or whatever.

That “leak” is very important for systems that require you to hold a button very early in the boot process to get into the desired mode :/

I don't disagree that it can be a handy shortcut, but I also spent over a week trying to reconcile reports that MAME skipped the initial screens of a bunch of random software for no apparent reason.

I got fooled by it as well – I was hitting an arrow key to dismiss the message, which toggles a switch by default on an Intel dev system in MAME. I was wondering why the switch wasn’t in its default state when I started with no .cfg file.

My strategy was to match the routines as closely as possible to the existing code for processing the keyboard.

cuavas · 2024-12-01T16:35:44Z

src/mame/apple/tk10_keyboard.h

+#ifndef MAME_TK10_KEYBOARD_H
+#define MAME_TK10_KEYBOARD_H


This will fail CI now that #include guards are checked.

ok, so I just have to rename it so it matches the filename

MAME_TK10_KEYBOARD_H
to
MAME_APPLE_TK10_KEYBOARD_H

so that the include guard has the proper name like:

MAME_APPLE_APPLE2COMMON_H

cuavas · 2024-12-01T16:38:31Z

src/mame/apple/tk10_keyboard.cpp

+		for (int i = 0; i < 8; i++)
+		{
+			if (!BIT(p2_r, i)) return m_keyboard[i]->read();
+			// read key matrix:
+			// a low value in p2 bit will drive bit of p1 low when switch contact made
+		}


You need to AND together the active rows in a switch matrix. It isn’t a multiplexer.

I'm not quite sure that AND would work here in this situation.

It treats P2 as a selector to select which column of the matrix is being scanned.

I got the schematic from:

http://john.ccac.rwth-aachen.de:8000/patrick/schematics.htm
http://john.ccac.rwth-aachen.de:8000/patrick/data/AppleClone_Keyboard.pdf

I did it that way because the code scans the p2 bits one by one, and then reads all of the p1 bits as a byte for the selected column.

I setup the PORT_BITS to match which bits of P1 will get driven low when P2 gets scanned.

0:05A mov r6,#$3F BE 3
0:05C mov r2,#$7F BA 7
0:05E mov r4,#$08 BC 0
0:060 mov a,r2 FA
0:061 rl a E7
0:062 mov r2,a AA
0:063 outl p2,a 3A
0:064 in a,p1 09
... loops back to 060

It writes a single bit low to p2, then reads p1. The low bit in p2 rotates through (the RL A), doing the scanning .

cuavas · 2024-12-01T16:40:59Z

src/mame/apple/apple2.cpp

+		if (data & 0x80)
+		{
+			m_transchar = data & 0x7f;
+			m_strobe = 0x80;
+//      printf("new char = %04x (%02x)\n", m_lastchar&0x3f, m_transchar);
+		}


If the top bit is the strobe, you need to actually look for edges.

It seems to work fine without edge detection, I don't think the 8039 will write the OUTL BUS unless it actually changes, so its won't write the OUTL BUS twice with the strobe high.

0:147 mov r6,a AE
0:148 jmp $109 24 0
0:14A mov a,r5 FD
0:14B orl a,#$80 43 8
0:14D outl bus,a 02 write it with strobe high
0:14E mov r5,#$10 BD 1
0:150 djnz r5,$150 ED 5 delay a bit
0:152 anl a,#$7F 53 7 write it with strobe low
0:154 outl bus,a 02

tk10: fix the include guard to have proper filename

goldnchild · 2024-12-02T05:37:55Z

You really should move the systems with the different keyboard to a derived class so any additional members don’t pollute the machines that don’t have it. You can also conveniently override machine_start to save additional members.

ok, created a new class and moved all the members to it, however there were a couple of member variables that were private and couldn't be accessed until I marked them as protected.

I also moved all of the routines for the tk10 and the class definition to be together so they aren't scattered around the file.

If these machines don’t have the “standard” keyboard in addition to the TK-10 keyboard, they shouldn’t be emulated as having both (if they could be used with one or the other, it needs to be slotted, but that’s another story).

I didn't want to disturb the existing code so I removed the ay3600 with config.device_remove()

The IO ports still show up in the configuration menus but the ay3600 is effectively disabled.

You could make the driver state class get the keyboard reset input via a virtual member function to avoid copy/pasting the the scan line callback.

I wanted to match the existing code as much as possible so is it really bad to reassign the scan line callback to use the tk10?

add tk10 keyboard device for apple ii clones

02a2c97

cuavas reviewed Nov 21, 2024

View reviewed changes

goldnchild added 4 commits November 23, 2024 06:24

apple2: add tk10 keyboard for am64 clone

84eee92

apple2: moved tk10 to mame/apple

32fc7ce

tk10: removed from scripts/src/machine.lua

6f60cbe

fix removal of tk10 from machine.lua

ad08698

cuavas reviewed Dec 1, 2024

View reviewed changes

apple2: add apple2_state_with_tk10 subclass

83fda6c

tk10: fix the include guard to have proper filename

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add tk10 keyboard device for apple ii clones #13004

add tk10 keyboard device for apple ii clones #13004

goldnchild commented Nov 21, 2024

rb6502 commented Nov 21, 2024

cuavas left a comment

cuavas Nov 21, 2024

goldnchild Nov 30, 2024

cuavas Nov 21, 2024

goldnchild Nov 30, 2024

cuavas Nov 21, 2024

goldnchild Nov 30, 2024

cuavas Nov 21, 2024

goldnchild Nov 30, 2024

cuavas Nov 21, 2024

goldnchild Nov 30, 2024

goldnchild commented Nov 22, 2024

cuavas left a comment

cuavas Dec 1, 2024

rb6502 Dec 1, 2024

cuavas Dec 1, 2024

rb6502 Dec 1, 2024

cuavas Dec 1, 2024

goldnchild Dec 2, 2024

cuavas Dec 1, 2024

goldnchild Dec 2, 2024

cuavas Dec 1, 2024

goldnchild Dec 2, 2024

cuavas Dec 1, 2024

goldnchild Dec 2, 2024 •

edited

Loading

goldnchild commented Dec 2, 2024

		#ifndef MAME_TK10_KEYBOARD_H
		#define MAME_TK10_KEYBOARD_H

		#ifndef MAME_TK10_KEYBOARD_H
		#define MAME_TK10_KEYBOARD_H

add tk10 keyboard device for apple ii clones #13004

Are you sure you want to change the base?

add tk10 keyboard device for apple ii clones #13004

Conversation

goldnchild commented Nov 21, 2024

rb6502 commented Nov 21, 2024

cuavas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

goldnchild commented Nov 22, 2024

cuavas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

goldnchild Dec 2, 2024 • edited Loading

Choose a reason for hiding this comment

goldnchild commented Dec 2, 2024

goldnchild Dec 2, 2024 •

edited

Loading