keyboard improvements: [Vas Crabb:

* Fix repeat delay/rate in x68000
* Make HLE Sun keyboard use common matrix keyboard routines
* Add linefeed key to generic keyboard (maps to ins by default)

src/devices/bus/sunkbd/hlekbd.cpp

@@ -2,7 +2,7 @@
 // copyright-holders:Vas Crabb
 #include "hlekbd.h"
 
-#include <numeric>
+#include "machine/keyboard.ipp"
 
 
 /*
@@ -761,24 +761,18 @@ hle_device_base::hle_device_base(
 	: device_t(mconfig, type, name, tag, owner, clock, shortname, source)
 	, device_serial_interface(mconfig, *this)
 	, device_sun_keyboard_port_interface(mconfig, *this)
+	, device_matrix_keyboard_interface(mconfig, *this, "ROW0", "ROW1", "ROW2", "ROW3", "ROW4", "ROW5", "ROW6", "ROW7")
 	, m_dips(*this, "DIP")
-	, m_key_inputs{
-			{ *this, "ROW0" }, { *this, "ROW1" },
-			{ *this, "ROW2" }, { *this, "ROW3" },
-			{ *this, "ROW4" }, { *this, "ROW5" },
-			{ *this, "ROW6" }, { *this, "ROW7" } }
-	, m_scan_timer(nullptr)
 	, m_click_timer(nullptr)
 	, m_beeper(*this, "beeper")
-	, m_current_keys{ 0, 0, 0, 0, 0, 0, 0, 0 }
-	, m_next_row(0)
 	, m_fifo{ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 }
-	, m_head(0)
+	, m_head(0U)
-	, m_tail(0)
+	, m_tail(0U)
-	, m_empty(0)
+	, m_empty(0U)
+	, m_make_count(0U)
 	, m_rx_state(RX_IDLE)
-	, m_keyclick(0)
+	, m_keyclick(0U)
-	, m_beeper_state(0)
+	, m_beeper_state(0U)
 {
 }
 
@@ -825,15 +819,13 @@ void hle_device_base::device_start()
 {
 	device_serial_interface::register_save_state(machine().save(), this);
 
-	m_scan_timer = timer_alloc(SCAN_TIMER_ID);
 	m_click_timer = timer_alloc(CLICK_TIMER_ID);
 
-	save_item(NAME(m_current_keys));
-	save_item(NAME(m_next_row));
 	save_item(NAME(m_fifo));
 	save_item(NAME(m_head));
 	save_item(NAME(m_tail));
 	save_item(NAME(m_empty));
+	save_item(NAME(m_make_count));
 	save_item(NAME(m_rx_state));
 	save_item(NAME(m_keyclick));
 	save_item(NAME(m_beeper_state));
@@ -849,11 +841,10 @@ void hle_device_base::device_start()
 void hle_device_base::device_reset()
 {
 	// initialise state
-	std::fill(std::begin(m_current_keys), std::end(m_current_keys), UINT16(0x0000U));
-	m_next_row = 0U;
 	std::fill(std::begin(m_fifo), std::end(m_fifo), UINT8(0x00U));
 	m_head = m_tail = 0U;
 	m_empty = 1U;
+	m_make_count = 0U;
 	m_rx_state = RX_IDLE;
 	m_keyclick = 0U;
 	m_beeper_state = 0x00U;
@@ -874,21 +865,18 @@ void hle_device_base::device_reset()
 	machine().output().set_led_value(LED_CAPS, 0);
 	machine().output().set_led_value(LED_KANA, 0);
 
+	// no beep
+	m_click_timer->reset();
+
+	// kick the base
+	reset_key_state();
+	start_processing(attotime::from_hz(1'200));
+
 	// send reset response
 	send_byte(0xffU);
 	send_byte(ident_byte());
-	UINT16 const acc(
+	if (are_all_keys_up())
-			std::accumulate(
-				std::begin(m_key_inputs),
-				std::end(m_key_inputs),
-				UINT16(0),
-				[] (UINT16 a, auto const &b) { return a | b->read(); }));
-	if (!acc)
 		send_byte(0x7fU);
-
-	// kick the scan timer
-	m_scan_timer->adjust(attotime::from_hz(1'200), 0, attotime::from_hz(1'200));
-	m_click_timer->reset();
 }
 
 
@@ -901,16 +889,13 @@ void hle_device_base::device_timer(emu_timer &timer, device_timer_id id, int par
 {
 	switch (id)
 	{
-	case SCAN_TIMER_ID:
-		scan_row();
-		break;
-
 	case CLICK_TIMER_ID:
 		m_beeper_state &= ~UINT8(BEEPER_CLICK);
 		m_beeper->set_state(m_beeper_state ? 1 : 0);
 		break;
 
 	default:
+		device_matrix_keyboard_interface::device_timer(timer, id, param, ptr);
 		device_serial_interface::device_timer(timer, id, param, ptr);
 	}
 }
@@ -942,6 +927,8 @@ void hle_device_base::tra_complete()
 	if (!m_empty)
 	{
 		transmit_register_setup(m_fifo[m_head]);
+		if (m_head == m_tail)
+			start_processing(attotime::from_hz(1'200));
 		m_head = (m_head + 1) & 0x0fU;
 		m_empty = (m_head == m_tail) ? 1 : 0;
 	}
@@ -990,12 +977,12 @@ void hle_device_base::rcv_complete()
 			break;
 
 		case COMMAND_CLICK_ON:
-			m_keyclick = 1;
+			m_keyclick = 1U;
 			m_rx_state = RX_IDLE;
 			break;
 
 		case COMMAND_CLICK_OFF:
-			m_keyclick = 0;
+			m_keyclick = 0U;
 			m_click_timer->reset();
 			m_beeper_state &= ~UINT8(BEEPER_CLICK);
 			m_beeper->set_state(m_beeper_state ? 1 : 0);
@@ -1021,59 +1008,48 @@ void hle_device_base::rcv_complete()
 
 
 /*--------------------------------------------------
-    hle_device_base::scan_row
+    hle_device_base::key_make
-    scan the next row in the keyboard matrix and
+    handle a key being pressed
-    send update to host
 --------------------------------------------------*/
 
-void hle_device_base::scan_row()
+void hle_device_base::key_make(UINT8 row, UINT8 column)
 {
-	// ensure we aren't in a stupid state
+	// we should have stopped processing if we filled the FIFO
-	assert(m_next_row < ARRAY_LENGTH(m_key_inputs));
+	assert(m_empty || (m_head != m_tail));
-	assert(m_next_row < ARRAY_LENGTH(m_current_keys));
 
-	// get last read state and new state of row
+	// send the make code, click if desired
-	UINT16 const row(m_key_inputs[m_next_row]->read());
+	send_byte((row << 4) | column);
-	UINT16 &current(m_current_keys[m_next_row]);
+	if (m_keyclick)
-	bool keydown(false), keyup(false);
-
-	// while the buffer isn't full, look for changed keys in this row
-	for (UINT8 bit = 0; (bit < 16) && (m_empty || (m_head != m_tail)); ++bit)
-	{
-		UINT16 const mask(UINT16(1) << bit);
-		if ((row ^ current) & mask)
-		{
-			UINT8 const make_break((row & mask) ? 0x00U : 0x80U);
-			keydown = keydown || !bool(make_break);
-			keyup = keyup || bool(make_break);
-			current = (current & ~mask) | (row & mask);
-			send_byte(make_break | (m_next_row << 4) | bit);
-		}
-	}
-
-	// if a key was pressed and keyclick is enabled, turn on the beeper
-	if (keydown && m_keyclick)
 	{
 		m_beeper_state |= UINT8(BEEPER_CLICK);
 		m_beeper->set_state(m_beeper_state ? 1 : 0);
 		m_click_timer->reset(attotime::from_msec(5));
 	}
 
-	// if a key was released and no keys are down, send all keys up code
+	// count keys
-	if (keyup)
+	++m_make_count;
-	{
+	assert(m_make_count);
-		UINT16 const acc(
+}
-				std::accumulate(
-					std::begin(m_current_keys),
-					std::end(m_current_keys),
-					UINT16(0),
-					[] (UINT16 a, UINT16 b) { return a | b; }));
-		if (!acc)
-			send_byte(0x7fU);
-	}
 
-	// advance to the next row
+
-	m_next_row = (m_next_row + 1) & 0x07U;
+/*--------------------------------------------------
+    hle_device_base::key_break
+    handle a key being released
+--------------------------------------------------*/
+
+void hle_device_base::key_break(UINT8 row, UINT8 column)
+{
+	// we should have stopped processing if we filled the FIFO
+	assert(m_empty || (m_head != m_tail));
+	assert(m_make_count);
+
+	// send the break code, and the idle code if no other keysa are down
+	send_byte(0x80U | (row << 4) | column);
+	if (!--m_make_count)
+		send_byte(0x7fU);
+
+	// check our counting
+	assert(are_all_keys_up() == !bool(m_make_count));
 }
 
 
@@ -1097,6 +1073,8 @@ void hle_device_base::send_byte(UINT8 code)
 		m_fifo[m_tail] = code;
 		m_tail = (m_tail + 1) & 0x0fU;
 		m_empty = 0;
+		if (m_head == m_tail)
+			stop_processing();
 	}
 	else
 	{

src/devices/bus/sunkbd/hlekbd.h

@@ -6,6 +6,7 @@
 #pragma once
 
 #include "sunkbd.h"
+#include "machine/keyboard.h"
 #include "sound/beep.h"
 
 
@@ -18,7 +19,11 @@ extern device_type const SUN_TYPE5_JP_HLE_KEYBOARD;
 
 
 namespace bus { namespace sunkbd {
-class hle_device_base : public device_t, public device_serial_interface, public device_sun_keyboard_port_interface
+class hle_device_base
+	: public device_t
+	, public device_serial_interface
+	, public device_sun_keyboard_port_interface
+	, protected device_matrix_keyboard_interface<8U>
 {
 public:
 	virtual machine_config_constructor device_mconfig_additions() const override;
@@ -47,14 +52,17 @@ protected:
 	virtual void tra_complete() override;
 	virtual void rcv_complete() override;
 
+	// device_matrix_keyboard_interface overrides
+	virtual void key_make(UINT8 row, UINT8 column) override;
+	virtual void key_break(UINT8 row, UINT8 column) override;
+
 	required_ioport m_dips;
-	required_ioport m_key_inputs[8];
 
 private:
 	// device_serial_interface uses 10'000 range
+	// device_matrix_keyboard_interface uses 20'000 range
 	enum {
-		SCAN_TIMER_ID = 20'000,
+		CLICK_TIMER_ID = 30'000
-		CLICK_TIMER_ID
 	};
 
 	// TODO: ensure these don't clash with diagnostic LEDs on host computer
@@ -91,17 +99,14 @@ private:
 	void scan_row();
 	void send_byte(UINT8 code);
 
-	emu_timer                       *m_scan_timer;
 	emu_timer                       *m_click_timer;
 	required_device<beep_device>    m_beeper;
 
-	UINT16  m_current_keys[8];
-	UINT8   m_next_row;
-
 	UINT8   m_fifo[16];
 	UINT8   m_head, m_tail;
 	UINT8   m_empty;
 
+	UINT8   m_make_count;
 	UINT8   m_rx_state;
 
 	UINT8   m_keyclick;

src/devices/machine/keyboard.cpp

@@ -34,13 +34,13 @@ UINT8 const TRANSLATION_TABLE[][2][4][16] = {
 		{	// ANSI
 			{ '`',   '1',   '2',   '3',   '4',   '5',   '6',   '7',   '8',   '9',   '0',   '-',   '=',   0x08U, 0x7fU, 0x1bU },
 			{ 0x09U, 'q',   'w',   'e',   'r',   't',   'y',   'u',   'i',   'o',   'p',   '[',   ']',   '\\',  0xffU, 0xffU },
-			{ 0xffU, 'a',   's',   'd',   'f',   'g',   'h',   'j',   'k',   'l',   ';',   '\'',  0x0dU, 0xffU, 0xffU, 0xffU },
+			{ 0xffU, 'a',   's',   'd',   'f',   'g',   'h',   'j',   'k',   'l',   ';',   '\'',  0x0dU, 0xffU, 0xffU, 0x0aU },
 			{ 0xffU, '\\',  'z',   'x',   'c',   'v',   'b',   'n',   'm',   ',',   '.',   '/',   0xffU, 0xffU, 0xffU, ' '   }
 		},
 		{	// JIS
 			{ '\\',  '1',   '2',   '3',   '4',   '5',   '6',   '7',   '8',   '9',   '0',   '-',   '^',   0x08U, 0x7fU, 0x1bU },
 			{ 0x09U, 'q',   'w',   'e',   'r',   't',   'y',   'u',   'i',   'o',   'p',   '@',   '[',   ']',   0xffU, 0xffU },
-			{ 0xffU, 'a',   's',   'd',   'f',   'g',   'h',   'j',   'k',   'l',   ';',   ':',   0x0dU, 0xffU, 0xffU, 0xffU },
+			{ 0xffU, 'a',   's',   'd',   'f',   'g',   'h',   'j',   'k',   'l',   ';',   ':',   0x0dU, 0xffU, 0xffU, 0x0aU },
 			{ 0xffU, '\\',  'z',   'x',   'c',   'v',   'b',   'n',   'm',   ',',   '.',   '/',   0xffU, 0xffU, 0xffU, ' '   }
 		}
 	},
@@ -48,13 +48,13 @@ UINT8 const TRANSLATION_TABLE[][2][4][16] = {
 		{	// ANSI shift
 			{ '~',   '!',   '@',   '#',   '$',   '%',   '^',   '&',   '*',   '(',   ')',   '_',   '+',   0x08U, 0x7fU, 0x1bU },
 			{ 0x09U, 'Q',   'W',   'E',   'R',   'T',   'Y',   'U',   'I',   'O',   'P',   '{',   '}',   '|',   0xffU, 0xffU },
-			{ 0xffU, 'A',   'S',   'D',   'F',   'G',   'H',   'J',   'K',   'L',   ':',   '"',   0x0dU, 0xffU, 0xffU, 0xffU },
+			{ 0xffU, 'A',   'S',   'D',   'F',   'G',   'H',   'J',   'K',   'L',   ':',   '"',   0x0dU, 0xffU, 0xffU, 0x0aU },
 			{ 0xffU, '_',   'Z',   'X',   'C',   'V',   'B',   'N',   'M',   '<',   '>',   '?',   0xffU, 0xffU, 0xffU, ' '   }
 		},
 		{	// JIS shift
 			{ '|',   '!',   '"',   '#',   '$',   '%',   '&',   '\'',  '(',   ')',   0xffU, '=',   '~',   0x08U, 0x7fU, 0x1bU },
 			{ 0x09U, 'Q',   'W',   'E',   'R',   'T',   'Y',   'U',   'I',   'O',   'P',   '`',   '{',   '}',   0xffU, 0xffU },
-			{ 0xffU, 'A',   'S',   'D',   'F',   'G',   'H',   'J',   'K',   'L',   '+',   '*',   0x0dU, 0xffU, 0xffU, 0xffU },
+			{ 0xffU, 'A',   'S',   'D',   'F',   'G',   'H',   'J',   'K',   'L',   '+',   '*',   0x0dU, 0xffU, 0xffU, 0x0aU },
 			{ 0xffU, '_',   'Z',   'X',   'C',   'V',   'B',   'N',   'M',   '<',   '>',   '?',   0xffU, 0xffU, 0xffU, ' '   }
 		}
 	},
@@ -62,13 +62,13 @@ UINT8 const TRANSLATION_TABLE[][2][4][16] = {
 		{	// ANSI ctrl
 			{ 0x00U, '1',   '2',   '3',   '4',   '5',   '6',   '7',   '8',   '9',   '0',   0x1fU, 0x1eU, 0x08U, 0x7fU, 0x1bU },
 			{ 0x09U, 0x11U, 0x17U, 0x05U, 0x12U, 0x14U, 0x19U, 0x15U, 0x09U, 0x0fU, 0x10U, 0x1bU, 0x1dU, 0x1cU, 0xffU, 0xffU },
-			{ 0xffU, 0x01U, 0x13U, 0x04U, 0x06U, 0x07U, 0x08U, 0x0aU, 0x0bU, 0x0cU, ';',   '\'',  0x0dU, 0xffU, 0xffU, 0xffU },
+			{ 0xffU, 0x01U, 0x13U, 0x04U, 0x06U, 0x07U, 0x08U, 0x0aU, 0x0bU, 0x0cU, ';',   '\'',  0x0dU, 0xffU, 0xffU, 0x0aU },
 			{ 0xffU, 0x1cU, 0x1aU, 0x18U, 0x03U, 0x16U, 0x02U, 0x0eU, 0x0dU, ',',   '.',   0x1fU, 0xffU, 0xffU, 0xffU, 0x00U }
 		},
 		{	// JIS ctrl
 			{ 0x1cU, '1',   '2',   '3',   '4',   '5',   '6',   '7',   '8',   '9',   '0',   0x1fU, 0x1eU, 0x08U, 0x7fU, 0x1bU },
 			{ 0x09U, 0x11U, 0x17U, 0x05U, 0x12U, 0x14U, 0x19U, 0x15U, 0x09U, 0x0fU, 0x10U, 0x00U, 0x1bU, 0x1dU, 0xffU, 0xffU },
-			{ 0xffU, 0x01U, 0x13U, 0x04U, 0x06U, 0x07U, 0x08U, 0x0aU, 0x0bU, 0x0cU, ';',   ':',   0x0dU, 0xffU, 0xffU, 0xffU },
+			{ 0xffU, 0x01U, 0x13U, 0x04U, 0x06U, 0x07U, 0x08U, 0x0aU, 0x0bU, 0x0cU, ';',   ':',   0x0dU, 0xffU, 0xffU, 0x0aU },
 			{ 0xffU, 0x1cU, 0x1aU, 0x18U, 0x03U, 0x16U, 0x02U, 0x0eU, 0x0dU, ',',   '.',   0x1fU, 0xffU, 0xffU, 0xffU, 0x00U }
 		}
 	}
@@ -203,10 +203,10 @@ INPUT_PORTS_START( generic_keyboard )
 	PORT_BIT( 0x0400U, IP_ACTIVE_HIGH, IPT_KEYBOARD ) PORT_CODE(KEYCODE_COLON)      PORT_CONDITION("GENKBD_CFG", 0x01, EQUALS, 0x01)                         PORT_CHAR(';')   PORT_CHAR('+')
 	PORT_BIT( 0x0800U, IP_ACTIVE_HIGH, IPT_KEYBOARD ) PORT_CODE(KEYCODE_QUOTE)      PORT_CONDITION("GENKBD_CFG", 0x01, EQUALS, 0x00)                         PORT_CHAR('\'')  PORT_CHAR('"')
 	PORT_BIT( 0x0800U, IP_ACTIVE_HIGH, IPT_KEYBOARD ) PORT_CODE(KEYCODE_QUOTE)      PORT_CONDITION("GENKBD_CFG", 0x01, EQUALS, 0x01)                         PORT_CHAR(':')   PORT_CHAR('*')  
-	PORT_BIT( 0x1000U, IP_ACTIVE_HIGH, IPT_KEYBOARD ) PORT_CODE(KEYCODE_ENTER)                                                                               PORT_CHAR(0x0dU)
+	PORT_BIT( 0x1000U, IP_ACTIVE_HIGH, IPT_KEYBOARD ) PORT_CODE(KEYCODE_ENTER)                                                        PORT_NAME("Return")    PORT_CHAR(0x0dU)
 	PORT_BIT( 0x2000U, IP_ACTIVE_HIGH, IPT_UNUSED   )
 	PORT_BIT( 0x4000U, IP_ACTIVE_HIGH, IPT_UNUSED   )
-	PORT_BIT( 0x8000U, IP_ACTIVE_HIGH, IPT_UNUSED   )
+	PORT_BIT( 0x8000U, IP_ACTIVE_HIGH, IPT_KEYBOARD ) PORT_CODE(KEYCODE_INS)                                                          PORT_NAME("Linefeed")  PORT_CHAR(UCHAR_MAMEKEY(INS))
 
 	PORT_START("GENKBD_ROW3")
 	PORT_BIT( 0x0001U, IP_ACTIVE_HIGH, IPT_UNUSED   )

src/devices/machine/keyboard.h

@@ -61,6 +61,8 @@ protected:
 	virtual void key_break(UINT8 row, UINT8 column);
 	virtual void will_scan_row(UINT8 row);
 
+	bool are_all_keys_up();
+
 private:
 	// device_serial_interface uses 10'000 range
 	enum {

src/devices/machine/keyboard.ipp

@@ -7,6 +7,9 @@
 
 #include "keyboard.h"
 
+#include <numeric>
+
+
 template <UINT8 ROW_COUNT>
 template <typename... T>
 device_matrix_keyboard_interface<ROW_COUNT>::device_matrix_keyboard_interface(machine_config const &mconfig, device_t &device, T &&... tags)
@@ -157,4 +160,15 @@ void device_matrix_keyboard_interface<ROW_COUNT>::will_scan_row(UINT8 row)
 {
 }
 
+
+template <UINT8 ROW_COUNT>
+bool device_matrix_keyboard_interface<ROW_COUNT>::are_all_keys_up()
+{
+	return 0U == std::accumulate(
+			std::begin(m_key_rows),
+			std::end(m_key_rows),
+			ioport_value(0),
+			[] (ioport_value a, auto const &b) { return a | b->read(); });
+}
+
 #endif // MAME_DEVICES_MACHINE_KEYBOARD_IPP

src/mame/machine/x68k_kbd.cpp

@@ -74,14 +74,14 @@ void x68k_keyboard_device::received_byte(UINT8 data)
 
 	if ((data & 0xf0) == 0x60)  // Key delay time
 	{
-		m_delay = data & 0x0f;
+		m_delay = ((data & 0x0f) * 100) + 200;
-		logerror("KB: Keypress delay time is now %ims\n", ((data & 0x0f) * 100) + 200);
+		logerror("KB: Keypress delay time is now %ims\n", m_delay);
 	}
 
 	if ((data & 0xf0) == 0x70)  // Key repeat rate
 	{
-		m_repeat = data & 0x0f;
+		m_repeat = (((data & 0x0f)^2) * 5) + 30;
-		logerror("KB: Keypress repeat rate is now %ims\n", (((data & 0x0f)^2) * 5) + 30);
+		logerror("KB: Keypress repeat rate is now %ims\n", m_repeat);
 	}
 }