From bf52ef7598a0b7243c776c59b16eade9edcf10a9 Mon Sep 17 00:00:00 2001 From: GreaseMonkey Date: Sun, 7 Jan 2024 11:21:38 +1300 Subject: [PATCH 01/20] Add "86Box unit tester" config option + Qt UI checkbox This is in preparation for making the device actually exist. --- src/86box.c | 1 + src/config.c | 10 ++++++++-- src/include/86box/86box.h | 1 + src/qt/qt_settingsotherperipherals.cpp | 8 +++++--- src/qt/qt_settingsotherperipherals.ui | 7 +++++++ 5 files changed, 22 insertions(+), 5 deletions(-) diff --git a/src/86box.c b/src/86box.c index 2e9434b82..31c0c0e09 100644 --- a/src/86box.c +++ b/src/86box.c @@ -173,6 +173,7 @@ bool serial_passthrough_enabled[SERIAL_MAX] = { 0, 0, 0, 0 }; /* (C) activat pass-through for serial ports */ int bugger_enabled = 0; /* (C) enable ISAbugger */ int postcard_enabled = 0; /* (C) enable POST card */ +int unittester_enabled = 0; /* (C) enable unit tester device */ int isamem_type[ISAMEM_MAX] = { 0, 0, 0, 0 }; /* (C) enable ISA mem cards */ int isartc_type = 0; /* (C) enable ISA RTC card */ int gfxcard[2] = { 0, 0 }; /* (C) graphics/video card */ diff --git a/src/config.c b/src/config.c index 8c4ad0de4..c973abf23 100644 --- a/src/config.c +++ b/src/config.c @@ -1514,8 +1514,9 @@ load_other_peripherals(void) char *p; char temp[512]; - bugger_enabled = !!ini_section_get_int(cat, "bugger_enabled", 0); - postcard_enabled = !!ini_section_get_int(cat, "postcard_enabled", 0); + bugger_enabled = !!ini_section_get_int(cat, "bugger_enabled", 0); + postcard_enabled = !!ini_section_get_int(cat, "postcard_enabled", 0); + unittester_enabled = !!ini_section_get_int(cat, "unittester_enabled", 0); for (uint8_t c = 0; c < ISAMEM_MAX; c++) { sprintf(temp, "isamem%d_type", c); @@ -2348,6 +2349,11 @@ save_other_peripherals(void) else ini_section_set_int(cat, "postcard_enabled", postcard_enabled); + if (unittester_enabled == 0) + ini_section_delete_var(cat, "unittester_enabled"); + else + ini_section_set_int(cat, "unittester_enabled", unittester_enabled); + for (uint8_t c = 0; c < ISAMEM_MAX; c++) { sprintf(temp, "isamem%d_type", c); if (isamem_type[c] == 0) diff --git a/src/include/86box/86box.h b/src/include/86box/86box.h index c16cd5efb..20f3fffcc 100644 --- a/src/include/86box/86box.h +++ b/src/include/86box/86box.h @@ -125,6 +125,7 @@ extern int gfxcard[2]; /* (C) graphics/video card */ extern char video_shader[512]; /* (C) video */ extern int bugger_enabled; /* (C) enable ISAbugger */ extern int postcard_enabled; /* (C) enable POST card */ +extern int unittester_enabled; /* (C) enable unit tester device */ extern int isamem_type[]; /* (C) enable ISA mem cards */ extern int isartc_type; /* (C) enable ISA RTC card */ extern int sound_is_float; /* (C) sound uses FP values */ diff --git a/src/qt/qt_settingsotherperipherals.cpp b/src/qt/qt_settingsotherperipherals.cpp index d168138a5..ad7a00bb4 100644 --- a/src/qt/qt_settingsotherperipherals.cpp +++ b/src/qt/qt_settingsotherperipherals.cpp @@ -44,6 +44,7 @@ SettingsOtherPeripherals::onCurrentMachineChanged(int machineId) bool machineHasIsa = (machine_has_bus(machineId, MACHINE_BUS_ISA) > 0); ui->checkBoxISABugger->setChecked((machineHasIsa && (bugger_enabled > 0)) ? true : false); ui->checkBoxPOSTCard->setChecked(postcard_enabled > 0 ? true : false); + ui->checkBoxUnitTester->setChecked(unittester_enabled > 0 ? true : false); ui->checkBoxISABugger->setEnabled(machineHasIsa); ui->comboBoxRTC->setEnabled(machineHasIsa); ui->pushButtonConfigureRTC->setEnabled(machineHasIsa); @@ -112,9 +113,10 @@ void SettingsOtherPeripherals::save() { /* Other peripherals category */ - bugger_enabled = ui->checkBoxISABugger->isChecked() ? 1 : 0; - postcard_enabled = ui->checkBoxPOSTCard->isChecked() ? 1 : 0; - isartc_type = ui->comboBoxRTC->currentData().toInt(); + bugger_enabled = ui->checkBoxISABugger->isChecked() ? 1 : 0; + postcard_enabled = ui->checkBoxPOSTCard->isChecked() ? 1 : 0; + unittester_enabled = ui->checkBoxUnitTester->isChecked() ? 1 : 0; + isartc_type = ui->comboBoxRTC->currentData().toInt(); /* ISA memory boards. */ for (int i = 0; i < ISAMEM_MAX; i++) { diff --git a/src/qt/qt_settingsotherperipherals.ui b/src/qt/qt_settingsotherperipherals.ui index 01f5545f8..481d80ebe 100644 --- a/src/qt/qt_settingsotherperipherals.ui +++ b/src/qt/qt_settingsotherperipherals.ui @@ -190,6 +190,13 @@ + + + + 86Box unit tester + + + From 72b465e18126c02e9ddb04ad54696428d643f70e Mon Sep 17 00:00:00 2001 From: GreaseMonkey Date: Sun, 7 Jan 2024 11:50:22 +1300 Subject: [PATCH 02/20] Add dummy 86Box Unit Tester device --- src/86box.c | 3 ++ src/device/CMakeLists.txt | 2 +- src/device/unittester.c | 79 ++++++++++++++++++++++++++++++++++ src/include/86box/unittester.h | 35 +++++++++++++++ 4 files changed, 118 insertions(+), 1 deletion(-) create mode 100644 src/device/unittester.c create mode 100644 src/include/86box/unittester.h diff --git a/src/86box.c b/src/86box.c index 31c0c0e09..220d38dfa 100644 --- a/src/86box.c +++ b/src/86box.c @@ -65,6 +65,7 @@ #include <86box/machine.h> #include <86box/bugger.h> #include <86box/postcard.h> +#include <86box/unittester.h> #include <86box/isamem.h> #include <86box/isartc.h> #include <86box/lpt.h> @@ -1222,6 +1223,8 @@ pc_reset_hard_init(void) device_add(&bugger_device); if (postcard_enabled) device_add(&postcard_device); + if (unittester_enabled) + device_add(&unittester_device); if (IS_ARCH(machine, MACHINE_BUS_PCI)) { pci_register_cards(); diff --git a/src/device/CMakeLists.txt b/src/device/CMakeLists.txt index 9e2c2b53d..24a9d7ac4 100644 --- a/src/device/CMakeLists.txt +++ b/src/device/CMakeLists.txt @@ -17,7 +17,7 @@ add_library(dev OBJECT bugger.c cassette.c cartridge.c hasp.c hwm.c hwm_lm75.c hwm_lm78.c hwm_gl518sm.c hwm_vt82c686.c ibm_5161.c isamem.c isartc.c ../lpt.c pci_bridge.c - postcard.c serial.c clock_ics9xxx.c isapnp.c i2c.c i2c_gpio.c + postcard.c serial.c unittester.c clock_ics9xxx.c isapnp.c i2c.c i2c_gpio.c smbus_piix4.c smbus_ali7101.c keyboard.c keyboard_xt.c kbc_at.c kbc_at_dev.c keyboard_at.c diff --git a/src/device/unittester.c b/src/device/unittester.c new file mode 100644 index 000000000..8b4a0ca5b --- /dev/null +++ b/src/device/unittester.c @@ -0,0 +1,79 @@ +/* + * 86Box A hypervisor and IBM PC system emulator that specializes in + * running old operating systems and software designed for IBM + * PC systems and compatibles from 1981 through fairly recent + * system designs based on the PCI bus. + * + * This file is part of the 86Box distribution. + * + * Debug device for assisting in unit testing. + * + * + * + * Authors: GreaseMonkey, + * + * Copyright 2024 GreaseMonkey. + */ +#include +#include +#include +#include +#include +#define HAVE_STDARG_H +#include <86box/86box.h> +#include <86box/plat.h> +#include <86box/unittester.h> + +static uint16_t unittester_trigger_port = 0x0080; +static uint16_t unittester_base_port = 0xFFFF; + +/* FIXME TEMPORARY --GM */ +#define ENABLE_UNITTESTER_LOG 1 + +#ifdef ENABLE_UNITTESTER_LOG +int unittester_do_log = ENABLE_UNITTESTER_LOG; + +static void +unittester_log(const char *fmt, ...) +{ + va_list ap; + + if (unittester_do_log) { + va_start(ap, fmt); + pclog_ex(fmt, ap); + va_end(ap); + } +} +#else +# define unittester_log(fmt, ...) +#endif + +static void * +unittester_init(UNUSED(const device_t *info)) +{ + //io_sethandler(unittester_trigger_port, 1, NULL, NULL, NULL, unittester_write, NULL, NULL, NULL); + unittester_log("[UT] 86Box Unit Tester initialised\n"); + + return &unittester_trigger_port; /* Dummy non-NULL value */ +} + +static void +unittester_close(UNUSED(void *priv)) +{ + //io_removehandler(unittester_trigger_port, 1, NULL, NULL, NULL, unittester_write, NULL, NULL, NULL); + unittester_log("[UT] 86Box Unit Tester closed\n"); +} + +const device_t unittester_device = { + .name = "86Box Unit Tester", + .internal_name = "unittester", + .flags = DEVICE_ISA, + .local = 0, + .init = unittester_init, + .close = unittester_close, + .reset = NULL, + { .available = NULL }, + .speed_changed = NULL, + .force_redraw = NULL, + .config = NULL +}; diff --git a/src/include/86box/unittester.h b/src/include/86box/unittester.h new file mode 100644 index 000000000..1e1255f52 --- /dev/null +++ b/src/include/86box/unittester.h @@ -0,0 +1,35 @@ +/* + * 86Box A hypervisor and IBM PC system emulator that specializes in + * running old operating systems and software designed for IBM + * PC systems and compatibles from 1981 through fairly recent + * system designs based on the PCI bus. + * + * This file is part of the 86Box distribution. + * + * Debug device for assisting in unit testing. + * + * + * + * Authors: GreaseMonkey, + * + * Copyright 2024 GreaseMonkey. + */ + +#ifndef UNITTESTER_H +#define UNITTESTER_H + +#ifdef __cplusplus +extern "C" { +#endif + +/* Global variables. */ +extern const device_t unittester_device; + +/* Functions. */ + +#ifdef __cplusplus +} +#endif + +#endif /*UNITTESTER_H*/ + From 130d4094e17de91978e7e399679b76e20d1f755b Mon Sep 17 00:00:00 2001 From: GreaseMonkey Date: Sun, 7 Jan 2024 13:17:21 +1300 Subject: [PATCH 03/20] unittester: Implement basic activation + IOBASE-setting protocol Next up is the actual I/O ports! --- src/device/unittester.c | 106 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 101 insertions(+), 5 deletions(-) diff --git a/src/device/unittester.c b/src/device/unittester.c index 8b4a0ca5b..5c0932847 100644 --- a/src/device/unittester.c +++ b/src/device/unittester.c @@ -21,11 +21,42 @@ #include #define HAVE_STDARG_H #include <86box/86box.h> +#include <86box/io.h> #include <86box/plat.h> #include <86box/unittester.h> -static uint16_t unittester_trigger_port = 0x0080; -static uint16_t unittester_base_port = 0xFFFF; +enum fsm1_value { + UT_FSM1_WAIT_8, + UT_FSM1_WAIT_6, + UT_FSM1_WAIT_B, + UT_FSM1_WAIT_o, + UT_FSM1_WAIT_x, +}; +enum fsm2_value { + UT_FSM2_IDLE, + UT_FSM2_WAIT_IOBASE_0, + UT_FSM2_WAIT_IOBASE_1, +}; + +struct unittester_state { + /* I/O port settings */ + uint16_t trigger_port; + uint16_t iobase_port; + + /* Trigger port finite state machines */ + /* FSM1: "86Box" string detection */ + enum fsm1_value fsm1; + /* FSM2: IOBASE port selection, once trigger is activated */ + enum fsm2_value fsm2; + uint16_t fsm2_new_iobase; +}; +static struct unittester_state unittester; +static const struct unittester_state unittester_defaults = { + .trigger_port = 0x0080, + .iobase_port = 0xFFFF, + .fsm1 = UT_FSM1_WAIT_8, + .fsm2 = UT_FSM2_IDLE, +}; /* FIXME TEMPORARY --GM */ #define ENABLE_UNITTESTER_LOG 1 @@ -48,19 +79,84 @@ unittester_log(const char *fmt, ...) # define unittester_log(fmt, ...) #endif +static void +unittester_trigger_write(UNUSED(uint16_t port), uint8_t val, UNUSED(void *priv)) +{ + /* Update FSM2 */ + switch (unittester.fsm2) { + /* IDLE: Do nothing - FSM1 will put us in the right state. */ + case UT_FSM2_IDLE: + unittester.fsm2 = UT_FSM2_IDLE; + break; + + /* WAIT IOBASE 0: Set low byte of temporary IOBASE. */ + case UT_FSM2_WAIT_IOBASE_0: + unittester.fsm2_new_iobase = ((uint16_t)val); + unittester.fsm2 = UT_FSM2_WAIT_IOBASE_1; + break; + + /* WAIT IOBASE 0: Set high byte of temporary IOBASE and commit to the real IOBASE. */ + case UT_FSM2_WAIT_IOBASE_1: + unittester.fsm2_new_iobase |= ((uint16_t)val)<<8; + unittester.iobase_port = unittester.fsm2_new_iobase; + unittester.fsm2 = UT_FSM2_IDLE; + break; + } + + /* Update FSM1 */ + switch (val) { + case '8': + unittester.fsm1 = UT_FSM1_WAIT_6; + break; + case '6': + if (unittester.fsm1 == UT_FSM1_WAIT_6) + unittester.fsm1 = UT_FSM1_WAIT_B; + else + unittester.fsm1 = UT_FSM1_WAIT_8; + break; + case 'B': + if (unittester.fsm1 == UT_FSM1_WAIT_B) + unittester.fsm1 = UT_FSM1_WAIT_o; + else + unittester.fsm1 = UT_FSM1_WAIT_8; + break; + case 'o': + if (unittester.fsm1 == UT_FSM1_WAIT_o) + unittester.fsm1 = UT_FSM1_WAIT_x; + else + unittester.fsm1 = UT_FSM1_WAIT_8; + break; + case 'x': + if (unittester.fsm1 == UT_FSM1_WAIT_x) { + unittester.fsm2 = UT_FSM2_WAIT_IOBASE_0; + } + unittester.fsm1 = UT_FSM1_WAIT_8; + break; + + default: + unittester.fsm1 = UT_FSM1_WAIT_8; + break; + } + + unittester_log("[UT] Trigger value %02X -> FSM1 = %02X, FSM2 = %02X, IOBASE = %04X\n", val, unittester.fsm1, unittester.fsm2, unittester.iobase_port); +} + static void * unittester_init(UNUSED(const device_t *info)) { - //io_sethandler(unittester_trigger_port, 1, NULL, NULL, NULL, unittester_write, NULL, NULL, NULL); + unittester = (struct unittester_state)unittester_defaults; + io_sethandler(unittester.trigger_port, 1, NULL, NULL, NULL, unittester_trigger_write, NULL, NULL, NULL); + unittester_log("[UT] 86Box Unit Tester initialised\n"); - return &unittester_trigger_port; /* Dummy non-NULL value */ + return &unittester; /* Dummy non-NULL value */ } static void unittester_close(UNUSED(void *priv)) { - //io_removehandler(unittester_trigger_port, 1, NULL, NULL, NULL, unittester_write, NULL, NULL, NULL); + io_removehandler(unittester.trigger_port, 1, NULL, NULL, NULL, unittester_trigger_write, NULL, NULL, NULL); + unittester_log("[UT] 86Box Unit Tester closed\n"); } From 5279cd5d8d84f40f652a29175ae3b5857095e817 Mon Sep 17 00:00:00 2001 From: GreaseMonkey Date: Sun, 7 Jan 2024 13:31:45 +1300 Subject: [PATCH 04/20] unittester: Add dummy main ports Reads and writes mostly do nothing but log, although the status returns a dummy value of 0x04 (no command in flight, not waiting for anything, and no errors). --- src/device/unittester.c | 57 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 55 insertions(+), 2 deletions(-) diff --git a/src/device/unittester.c b/src/device/unittester.c index 5c0932847..2f706c8e3 100644 --- a/src/device/unittester.c +++ b/src/device/unittester.c @@ -79,9 +79,46 @@ unittester_log(const char *fmt, ...) # define unittester_log(fmt, ...) #endif +static void +unittester_write(uint16_t port, uint8_t val, UNUSED(void *priv)) +{ + if (port == unittester.iobase_port+0x00) { + /* Command port */ + /* TODO! --GM */ + unittester_log("[UT] W %02X Command\n", val); + } else if (port == unittester.iobase_port+0x01) { + /* Data port */ + /* TODO! --GM */ + unittester_log("[UT] W %02X Data\n", val); + } else { + /* Not handled here - possibly open bus! */ + } +} + +static uint8_t +unittester_read(uint16_t port, UNUSED(void *priv)) +{ + if (port == unittester.iobase_port+0x00) { + /* Status port */ + /* TODO! --GM */ + unittester_log("[UT] R -- Status\n"); + return 0x04; + } else if (port == unittester.iobase_port+0x01) { + /* Data port */ + /* TODO! --GM */ + unittester_log("[UT] R -- Data\n"); + return 0xFE; + } else { + /* Not handled here - possibly open bus! */ + return 0xFF; + } +} + static void unittester_trigger_write(UNUSED(uint16_t port), uint8_t val, UNUSED(void *priv)) { + unittester_log("[UT] Trigger value %02X -> FSM1 = %02X, FSM2 = %02X, IOBASE = %04X\n", val, unittester.fsm1, unittester.fsm2, unittester.iobase_port); + /* Update FSM2 */ switch (unittester.fsm2) { /* IDLE: Do nothing - FSM1 will put us in the right state. */ @@ -98,7 +135,20 @@ unittester_trigger_write(UNUSED(uint16_t port), uint8_t val, UNUSED(void *priv)) /* WAIT IOBASE 0: Set high byte of temporary IOBASE and commit to the real IOBASE. */ case UT_FSM2_WAIT_IOBASE_1: unittester.fsm2_new_iobase |= ((uint16_t)val)<<8; + + unittester_log("[UT] Remapping IOBASE: %04X -> %04X\n", val, unittester.iobase_port, unittester.fsm2_new_iobase); + + /* Unmap old IOBASE */ + if (unittester.iobase_port != 0xFFFF) + io_removehandler(unittester.iobase_port, 2, unittester_read, NULL, NULL, unittester_write, NULL, NULL, NULL); + unittester.iobase_port = 0xFFFF; + + /* Map new IOBASE */ unittester.iobase_port = unittester.fsm2_new_iobase; + if (unittester.iobase_port != 0xFFFF) + io_sethandler(unittester.iobase_port, 2, unittester_read, NULL, NULL, unittester_write, NULL, NULL, NULL); + + /* Reset FSM2 to IDLE */ unittester.fsm2 = UT_FSM2_IDLE; break; } @@ -128,6 +178,7 @@ unittester_trigger_write(UNUSED(uint16_t port), uint8_t val, UNUSED(void *priv)) break; case 'x': if (unittester.fsm1 == UT_FSM1_WAIT_x) { + unittester_log("[UT] Config activated, awaiting new IOBASE\n"); unittester.fsm2 = UT_FSM2_WAIT_IOBASE_0; } unittester.fsm1 = UT_FSM1_WAIT_8; @@ -137,8 +188,6 @@ unittester_trigger_write(UNUSED(uint16_t port), uint8_t val, UNUSED(void *priv)) unittester.fsm1 = UT_FSM1_WAIT_8; break; } - - unittester_log("[UT] Trigger value %02X -> FSM1 = %02X, FSM2 = %02X, IOBASE = %04X\n", val, unittester.fsm1, unittester.fsm2, unittester.iobase_port); } static void * @@ -157,6 +206,10 @@ unittester_close(UNUSED(void *priv)) { io_removehandler(unittester.trigger_port, 1, NULL, NULL, NULL, unittester_trigger_write, NULL, NULL, NULL); + if (unittester.iobase_port != 0xFFFF) + io_removehandler(unittester.iobase_port, 2, unittester_read, NULL, NULL, unittester_write, NULL, NULL, NULL); + unittester.iobase_port = 0xFFFF; + unittester_log("[UT] 86Box Unit Tester closed\n"); } From 04eb9ffc3e82418c5e45d1edb55a139e6b496bf7 Mon Sep 17 00:00:00 2001 From: GreaseMonkey Date: Sun, 7 Jan 2024 17:07:24 +1300 Subject: [PATCH 05/20] unittester: Add WIP specification document --- doc/specifications/86box-unit-tester.md | 240 ++++++++++++++++++++++++ src/device/unittester.c | 1 + src/include/86box/unittester.h | 1 + 3 files changed, 242 insertions(+) create mode 100644 doc/specifications/86box-unit-tester.md diff --git a/doc/specifications/86box-unit-tester.md b/doc/specifications/86box-unit-tester.md new file mode 100644 index 000000000..77279a3ee --- /dev/null +++ b/doc/specifications/86box-unit-tester.md @@ -0,0 +1,240 @@ +# 86Box Unit Tester device specification + +**TODO: DESCRIBE ME!** + +---------------------------------------------------------------------------- + +## Conventions + +### Integer types + +- `i8` denotes a signed 8-bit value. +- `u8` denotes an unsigned 8-bit value. +- `w8` denotes an 8-bit value which wraps around. +- `x8` denotes an 8-bit value where the signedness is irrelevant. +- `e8` ("either") denotes an 8-bit value where the most significant bit is clear - in effect, this is a 7-bit unsigned value. +- `u16L` denotes a little-endian unsigned 16-bit value. +- `u16B` would denote a big-endian unsigned 16-bit value if we had any big-endian values. +- `[N]T` denotes an array of `N` values of type `T`, whatever `N` and `T` are. + +---------------------------------------------------------------------------- + +## Usage + +### Accessing the device and configuring the I/O base address + +Find an area in I/O space where 2 addresses are confirmed (or assumed) to be unused. +There is no need for the 2 addresses to be 2-byte-aligned. + +Send the following sequence of bytes to port 0x80 with INTERRUPTS DISABLED: + + '8', '6', 'B', 'o', 'x', (IOBASE & 0xFF), (IOBASE >> 8) + +Alternatively denoted in hex: + + 38 36 42 6F 78 yy xx + +There are no timing constraints. This is an emulator, after all. + +To confirm that this has happened, read the status port at IOBASE+0x00. +If it's 0xFF, then the device is most likely not present. +Otherwise, one can potentially assume that it exists and has been configured successfully. +(You *did* make sure that the space was unused *before* doing this, right?) + +IOBASE is allowed to overlap the trigger port, but please don't do this! + +### Hiding the device + +Set the I/O base address to 0xFFFF using the above method. + +### Executing commands + +**TODO: IMPLEMENT ME!** + +The ports at IOBASE+0x00 and IOBASE+0x01 are all 8 bits wide. + +Writing to IOBASE+0x00 cancels any in-flight commands and sends a new command. + +Reading from IOBASE+0x00 reads the status: + +- bit 0: There is data to be read from this device + - If one reads with this bit clear, the returned data will be 0xFF. +- bit 1: The device is expecting data to be sent to it + - If one writes with this bit clear, the data will be ignored. +- bit 2: There is no command in flight + - If this is set, then bits 0 and 1 will be clear. +- bit 3: The previously-sent command does not exist. +- bits 4 .. 7: Reserved, should be 0. + +Writing to IOBASE+0x01 provides data to the device if said data is needed. + +Reading from IOBASE+0x01 fetches the next byte data to the device if said data is needed. + +### General flow of executing a command: + +This is how most commands will work. + +- Write the command to IOBASE+0x00. +- If data needs to be written or read: + - Read the status from IOBASE+0x00 and confirm that bit 2 is clear. + If it is set, then the command may not exist. + Check bit 3 if that's the case. +- If data needs to be written: + - Write all the data one needs to write. +- If data needs to be read: + - Read the status from IOBASE+0x00 and wait until bit 0 is set. + If it is set, then the command may not exist. + Check bit 3 if that's the case. + - Keep reading bytes until one is satisfied. +- Otherwise: + - Read the status from IOBASE+0x00 and wait until any of the bottom 3 bits are set. + +---------------------------------------------------------------------------- + +## Command reference + +### 0x00: No-op + +**TODO: IMPLEMENT ME!** + +This does nothing, takes no input, and gives no output. + +This is an easy way to reset the status to 0x04 (no command in flight, not waiting for reads or writes, and no errors). + +### 0x01: Capture Screen Snapshot + +**TODO: IMPLEMENT ME!** + +Captures a snapshot of the current screen state and stores it in the current snapshot buffer. + +The initial state of the screen snapshot buffer has an image area of 0x0, an overscanned area of 0x0, and an image start offset of (0,0). + +Input: + +* u8 monitor + - 0x00 = no monitor - clear the screen snapshot + - 0x01 = primary monitor + - 0x02 = secondary monitor + - Any monitor which is not available is treated as 0x00, and clears the screen snapshot. + +Output: + +* `e16L` image width in pixels +* `e16L` image height in pixels +* `e16L` overscanned width in pixels +* `e16L` overscanned height in pixels +* `e16L` X offset of image start +* `e16L` Y offset of image start + +If there is no screen snapshot, then all values will be 0 as per the initial screen snapshot buffer state. + +### 0x02: Read Screen Snapshot Rectangle + +**TODO: IMPLEMENT ME!** + +Returns a rectangular snapshot of the screen snapshot buffer. + +Input: + +* `e16L` w: rectangle width in pixels +* `e16L` h: rectangle height in pixels +* `i16L` x: X offset relative to image start +* `i16L` y: Y offset relative to image start + +Output: + +* `[h][w][4]u8`: image data + - `[y][x][0]` is the blue component, or 0x00 if the pixel is outside the snapshot area. + - `[y][x][1]` is the green component, or 0x00 if the pixel is outside the snapshot area. + - `[y][x][2]` is the red component, or 0x00 if the pixel is outside the snapshot area. + - `[y][x][3]` is 0x00, or 0xFF if the pixel is outside the snapshot area. + +### 0x03: Verify Screen Snapshot Rectangle + +**TODO: IMPLEMENT ME!** + +As per 0x02 "Read Screen Snapshot Rectangle", except instead of returning the pixel data, it returns a CRC-32 of the data. + +The CRC is as per zlib's `crc32()` function. Specifically, one uses a right-shifting Galois LFSR with a polynomial of 0xEDB88320, bytes XORed against the least significant byte, the initial seed is 0xFFFFFFFF, and all bits of the output are inverted. + +(Rationale: There are better CRCs, but this one is ubiquitous and still really good... and we don't need to protect against deliberate tampering.) + +Input: + +* `e16L` w: rectangle width in pixels +* `e16L` h: rectangle height in pixels +* `i16L` x: X offset relative to image start +* `i16L` y: Y offset relative to image start + +Output: + +* `u32L` crc: CRC-32 of rectangle data + +### 0x04: Exit 86Box + +**TODO: IMPLEMENT ME!** + +Exits 86Box, unless this command is disabled. + +- If the command is enabled, then program execution terminates immediately. +- If the command is disabled, it still counts as having executed correctly, but program execution continues. This makes it useful to show a "results" screen for a unit test. + +Input: + +* u8 exit code: + - The actual exit code is clamped to no greater than the maximum valid exit code. + - In practice, this is probably going to be 0x7F. + +---------------------------------------------------------------------------- + +## Implementation notes + +### Port 0x80 sequence detection + +In order to ensure that one can always trigger the activation sequence, there are effectively two finite state machines in action. + +FSM1: +- Wait for 8. +- Wait for 6. +- Wait for B. +- Wait for o. +- Wait for x. + Once received, set FSM2 to "Wait for low byte", + then go back to "Wait for 8". + +If at any point an 8 arrives, jump to the "Wait for 6" step. + +Otherwise, if any other unexpected byte arrives, jump to the "Wait for 8" step. + +FSM2: +- Idle. +- Wait for low byte. Once received, store this in a temporary location. +- Wait for high byte. + Once received, replace IOBASE with this byte in the high byte and the temporary value in the low byte, + then go back to "Idle". + +### Command processing state machine + +**TODO: IMPLEMENT ME!** + +**TODO: DOCUMENT ME!** + +---------------------------------------------------------------------------- + +## Extending the protocol + +### Adding new commands + +Commands 0x01 through 0x7F accept a single command byte. + +Command bytes 0x80 through 0xFB are reserved for 16-bit command IDs, to be written in a similar way to this: + +- Write the first command byte (0x80 through 0xFF) to the command register. +- If this block of commands does not exist, then the command is cancelled and the status is set to 0x0C. +- Otherwise, the status is set to 0x0 +- Write the next command byte (0x00 through 0xFF) to the data register. +- If this block of commands does not exist, then the command is cancelled and the status is set to 0x0C. +- Otherwise, the command exists and the status is set according to the command. + +Command bytes 0xFC through 0xFF are reserved for if we somehow need more than 16 bits worth of command ID. + diff --git a/src/device/unittester.c b/src/device/unittester.c index 2f706c8e3..ff605e119 100644 --- a/src/device/unittester.c +++ b/src/device/unittester.c @@ -7,6 +7,7 @@ * This file is part of the 86Box distribution. * * Debug device for assisting in unit testing. + * See doc/specifications/86box-unit-tester.md for more info. * * * diff --git a/src/include/86box/unittester.h b/src/include/86box/unittester.h index 1e1255f52..f6dedeae5 100644 --- a/src/include/86box/unittester.h +++ b/src/include/86box/unittester.h @@ -7,6 +7,7 @@ * This file is part of the 86Box distribution. * * Debug device for assisting in unit testing. + * See doc/specifications/86box-unit-tester.md for more info. * * * From d1133a7c7fdbd147caa8376c0c7347115bd24597 Mon Sep 17 00:00:00 2001 From: GreaseMonkey Date: Sun, 7 Jan 2024 17:20:19 +1300 Subject: [PATCH 06/20] unittester: Implement status register and 0x00 "No-op" command --- doc/specifications/86box-unit-tester.md | 2 -- src/device/unittester.c | 44 +++++++++++++++++++++---- 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/doc/specifications/86box-unit-tester.md b/doc/specifications/86box-unit-tester.md index 77279a3ee..9100f59de 100644 --- a/doc/specifications/86box-unit-tester.md +++ b/doc/specifications/86box-unit-tester.md @@ -95,8 +95,6 @@ This is how most commands will work. ### 0x00: No-op -**TODO: IMPLEMENT ME!** - This does nothing, takes no input, and gives no output. This is an easy way to reset the status to 0x04 (no command in flight, not waiting for reads or writes, and no errors). diff --git a/src/device/unittester.c b/src/device/unittester.c index ff605e119..e6deb45bc 100644 --- a/src/device/unittester.c +++ b/src/device/unittester.c @@ -39,6 +39,20 @@ enum fsm2_value { UT_FSM2_WAIT_IOBASE_1, }; +/* Status bit mask */ +#define UT_STATUS_AWAITING_READ (1<<0) +#define UT_STATUS_AWAITING_WRITE (1<<1) +#define UT_STATUS_IDLE (1<<2) +#define UT_STATUS_UNSUPPORTED_CMD (1<<3) + +/* Command list */ +enum unittester_cmd { + UT_CMD_NOOP = 0x00, + UT_CMD_CAPTURE_SCREEN_SNAPSHOT = 0x01, + UT_CMD_READ_SCREEN_SNAPSHOT_RECTANGLE = 0x02, + UT_CMD_VERIFY_SCREEN_SNAPSHOT_RECTANGLE = 0x03, +}; + struct unittester_state { /* I/O port settings */ uint16_t trigger_port; @@ -50,6 +64,10 @@ struct unittester_state { /* FSM2: IOBASE port selection, once trigger is activated */ enum fsm2_value fsm2; uint16_t fsm2_new_iobase; + + /* Runtime state */ + uint8_t status; + enum unittester_cmd cmd_id; }; static struct unittester_state unittester; static const struct unittester_state unittester_defaults = { @@ -57,6 +75,7 @@ static const struct unittester_state unittester_defaults = { .iobase_port = 0xFFFF, .fsm1 = UT_FSM1_WAIT_8, .fsm2 = UT_FSM2_IDLE, + .status = UT_STATUS_IDLE, }; /* FIXME TEMPORARY --GM */ @@ -85,8 +104,22 @@ unittester_write(uint16_t port, uint8_t val, UNUSED(void *priv)) { if (port == unittester.iobase_port+0x00) { /* Command port */ - /* TODO! --GM */ unittester_log("[UT] W %02X Command\n", val); + + switch (val) { + /* 0x00: No-op */ + case UT_CMD_NOOP: + unittester.cmd_id = UT_CMD_NOOP; + unittester.status = UT_STATUS_IDLE; + break; + + /* Unsupported command - terminate here */ + default: + unittester.cmd_id = UT_CMD_NOOP; + unittester.status = UT_STATUS_IDLE | UT_STATUS_UNSUPPORTED_CMD; + break; + } + } else if (port == unittester.iobase_port+0x01) { /* Data port */ /* TODO! --GM */ @@ -101,14 +134,13 @@ unittester_read(uint16_t port, UNUSED(void *priv)) { if (port == unittester.iobase_port+0x00) { /* Status port */ - /* TODO! --GM */ - unittester_log("[UT] R -- Status\n"); - return 0x04; + unittester_log("[UT] R -- Status = %02X\n", unittester.status); + return unittester.status; } else if (port == unittester.iobase_port+0x01) { /* Data port */ - /* TODO! --GM */ unittester_log("[UT] R -- Data\n"); - return 0xFE; + /* TODO! --GM */ + return 0xFF; } else { /* Not handled here - possibly open bus! */ return 0xFF; From ab7df4409b117b44f71362ce37252adb78386d63 Mon Sep 17 00:00:00 2001 From: GreaseMonkey Date: Sun, 7 Jan 2024 17:42:23 +1300 Subject: [PATCH 07/20] unittester: Implement 0x04 "Exit" --- doc/specifications/86box-unit-tester.md | 2 - src/device/unittester.c | 109 +++++++++++++++++++++++- 2 files changed, 106 insertions(+), 5 deletions(-) diff --git a/doc/specifications/86box-unit-tester.md b/doc/specifications/86box-unit-tester.md index 9100f59de..fe0203b46 100644 --- a/doc/specifications/86box-unit-tester.md +++ b/doc/specifications/86box-unit-tester.md @@ -170,8 +170,6 @@ Output: ### 0x04: Exit 86Box -**TODO: IMPLEMENT ME!** - Exits 86Box, unless this command is disabled. - If the command is enabled, then program execution terminates immediately. diff --git a/src/device/unittester.c b/src/device/unittester.c index e6deb45bc..8a6a81fdb 100644 --- a/src/device/unittester.c +++ b/src/device/unittester.c @@ -16,8 +16,10 @@ * Copyright 2024 GreaseMonkey. */ #include +#include #include #include +#include #include #include #define HAVE_STDARG_H @@ -51,6 +53,7 @@ enum unittester_cmd { UT_CMD_CAPTURE_SCREEN_SNAPSHOT = 0x01, UT_CMD_READ_SCREEN_SNAPSHOT_RECTANGLE = 0x02, UT_CMD_VERIFY_SCREEN_SNAPSHOT_RECTANGLE = 0x03, + UT_CMD_EXIT = 0x04, }; struct unittester_state { @@ -68,6 +71,14 @@ struct unittester_state { /* Runtime state */ uint8_t status; enum unittester_cmd cmd_id; + uint32_t write_offs; + uint32_t read_offs; + uint32_t write_len; + uint32_t read_len; + + /* Command-specific state */ + /* 0x04: Exit */ + uint8_t exit_code; }; static struct unittester_state unittester; static const struct unittester_state unittester_defaults = { @@ -76,8 +87,12 @@ static const struct unittester_state unittester_defaults = { .fsm1 = UT_FSM1_WAIT_8, .fsm2 = UT_FSM2_IDLE, .status = UT_STATUS_IDLE, + .cmd_id = UT_CMD_NOOP, }; +/* FIXME: This needs a config option! --GM */ +static bool unittester_exit_enabled = true; + /* FIXME TEMPORARY --GM */ #define ENABLE_UNITTESTER_LOG 1 @@ -106,6 +121,11 @@ unittester_write(uint16_t port, uint8_t val, UNUSED(void *priv)) /* Command port */ unittester_log("[UT] W %02X Command\n", val); + unittester.write_offs = 0; + unittester.write_len = 0; + unittester.read_offs = 0; + unittester.read_len = 0; + switch (val) { /* 0x00: No-op */ case UT_CMD_NOOP: @@ -113,6 +133,13 @@ unittester_write(uint16_t port, uint8_t val, UNUSED(void *priv)) unittester.status = UT_STATUS_IDLE; break; + /* 0x04: No-op */ + case UT_CMD_EXIT: + unittester.cmd_id = UT_CMD_EXIT; + unittester.status = UT_STATUS_AWAITING_WRITE; + unittester.write_len = 1; + break; + /* Unsupported command - terminate here */ default: unittester.cmd_id = UT_CMD_NOOP; @@ -122,8 +149,64 @@ unittester_write(uint16_t port, uint8_t val, UNUSED(void *priv)) } else if (port == unittester.iobase_port+0x01) { /* Data port */ - /* TODO! --GM */ unittester_log("[UT] W %02X Data\n", val); + + /* Skip if not awaiting */ + if ((unittester.status & UT_STATUS_AWAITING_WRITE) == 0) + return; + + switch (unittester.cmd_id) { + case UT_CMD_EXIT: + switch(unittester.write_offs) { + case 0: + unittester.exit_code = val; + break; + default: + break; + } + break; + + /* This should not be reachable, but just in case... */ + default: + break; + } + + /* Advance write buffer */ + unittester.write_offs += 1; + if (unittester.write_offs >= unittester.write_len) { + unittester.status &= ~UT_STATUS_AWAITING_WRITE; + /* Determine what we're doing here based on the command. */ + switch (unittester.cmd_id) { + case UT_CMD_EXIT: + unittester_log("[UT] Exit received - code = %02X\n", unittester.exit_code); + + /* CHECK: Do we actually exit? */ + if (unittester_exit_enabled) { + /* Yes - call exit! */ + /* Clamp exit code */ + if (unittester.exit_code > 0x7F) + unittester.exit_code = 0x7F; + + /* Exit somewhat quickly! */ + unittester_log("[UT] Exit enabled, exiting with code %02X\n", unittester.exit_code); + exit(unittester.exit_code); + + } else { + /* No - report successful command completion and continue program execution */ + unittester_log("[UT] Exit disabled, continuing execution\n"); + unittester.cmd_id = UT_CMD_NOOP; + unittester.status = UT_STATUS_IDLE; + } + break; + + default: + /* Nothing to write? Stop here. */ + unittester.cmd_id = UT_CMD_NOOP; + unittester.status = UT_STATUS_IDLE; + break; + } + } + } else { /* Not handled here - possibly open bus! */ } @@ -132,6 +215,8 @@ unittester_write(uint16_t port, uint8_t val, UNUSED(void *priv)) static uint8_t unittester_read(uint16_t port, UNUSED(void *priv)) { + uint8_t outval = 0xFF; + if (port == unittester.iobase_port+0x00) { /* Status port */ unittester_log("[UT] R -- Status = %02X\n", unittester.status); @@ -139,8 +224,26 @@ unittester_read(uint16_t port, UNUSED(void *priv)) } else if (port == unittester.iobase_port+0x01) { /* Data port */ unittester_log("[UT] R -- Data\n"); - /* TODO! --GM */ - return 0xFF; + + /* Skip if not awaiting */ + if ((unittester.status & UT_STATUS_AWAITING_READ) == 0) + return 0xFF; + + switch (unittester.cmd_id) { + /* This should not be reachable, but just in case... */ + default: + break; + } + + /* Advance read buffer */ + unittester.read_offs += 1; + if (unittester.read_offs >= unittester.read_len) { + /* Once fully read, we stop here. */ + unittester.cmd_id = UT_CMD_NOOP; + unittester.status = UT_STATUS_IDLE; + } + + return outval; } else { /* Not handled here - possibly open bus! */ return 0xFF; From 7dbbb0d12b1446c1238e1a29cad3ce26739745f8 Mon Sep 17 00:00:00 2001 From: GreaseMonkey Date: Sun, 7 Jan 2024 18:19:17 +1300 Subject: [PATCH 08/20] Fix a comment --- src/device/unittester.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/device/unittester.c b/src/device/unittester.c index 8a6a81fdb..b1f3efcc5 100644 --- a/src/device/unittester.c +++ b/src/device/unittester.c @@ -133,7 +133,7 @@ unittester_write(uint16_t port, uint8_t val, UNUSED(void *priv)) unittester.status = UT_STATUS_IDLE; break; - /* 0x04: No-op */ + /* 0x04: Exit */ case UT_CMD_EXIT: unittester.cmd_id = UT_CMD_EXIT; unittester.status = UT_STATUS_AWAITING_WRITE; From ae3e40706f951008655d62d29a9d2ba09bb0ef65 Mon Sep 17 00:00:00 2001 From: GreaseMonkey Date: Sun, 7 Jan 2024 18:19:27 +1300 Subject: [PATCH 09/20] unittester: Remove the worst of the log spam --- src/device/unittester.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/device/unittester.c b/src/device/unittester.c index b1f3efcc5..8de09e295 100644 --- a/src/device/unittester.c +++ b/src/device/unittester.c @@ -253,7 +253,8 @@ unittester_read(uint16_t port, UNUSED(void *priv)) static void unittester_trigger_write(UNUSED(uint16_t port), uint8_t val, UNUSED(void *priv)) { - unittester_log("[UT] Trigger value %02X -> FSM1 = %02X, FSM2 = %02X, IOBASE = %04X\n", val, unittester.fsm1, unittester.fsm2, unittester.iobase_port); + /* This one gets quite spammy. */ + /* unittester_log("[UT] Trigger value %02X -> FSM1 = %02X, FSM2 = %02X, IOBASE = %04X\n", val, unittester.fsm1, unittester.fsm2, unittester.iobase_port); */ /* Update FSM2 */ switch (unittester.fsm2) { From 59e51939adfc8342f81629af1c1b3eea95ccef97 Mon Sep 17 00:00:00 2001 From: GreaseMonkey Date: Sun, 7 Jan 2024 20:09:06 +1300 Subject: [PATCH 10/20] unittester: Fix erroneous debug message --- src/device/unittester.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/device/unittester.c b/src/device/unittester.c index 8de09e295..01972ef67 100644 --- a/src/device/unittester.c +++ b/src/device/unittester.c @@ -273,7 +273,7 @@ unittester_trigger_write(UNUSED(uint16_t port), uint8_t val, UNUSED(void *priv)) case UT_FSM2_WAIT_IOBASE_1: unittester.fsm2_new_iobase |= ((uint16_t)val)<<8; - unittester_log("[UT] Remapping IOBASE: %04X -> %04X\n", val, unittester.iobase_port, unittester.fsm2_new_iobase); + unittester_log("[UT] Remapping IOBASE: %04X -> %04X\n", unittester.iobase_port, unittester.fsm2_new_iobase); /* Unmap old IOBASE */ if (unittester.iobase_port != 0xFFFF) From d44c439bd819ebaf9d44d055d08c153537c550c8 Mon Sep 17 00:00:00 2001 From: GreaseMonkey Date: Sun, 7 Jan 2024 21:26:14 +1300 Subject: [PATCH 11/20] unittester: Implement most of 0x01 "Capture Screen Snapshot" The thing that isn't implemented is the actual snapshot capture. But the dimensions should be correct. I am feeling a bit iffy about the overscan, though. --- src/device/unittester.c | 129 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 126 insertions(+), 3 deletions(-) diff --git a/src/device/unittester.c b/src/device/unittester.c index 01972ef67..78086fd90 100644 --- a/src/device/unittester.c +++ b/src/device/unittester.c @@ -27,6 +27,7 @@ #include <86box/io.h> #include <86box/plat.h> #include <86box/unittester.h> +#include <86box/video.h> enum fsm1_value { UT_FSM1_WAIT_8, @@ -68,7 +69,7 @@ struct unittester_state { enum fsm2_value fsm2; uint16_t fsm2_new_iobase; - /* Runtime state */ + /* Command and data handling state */ uint8_t status; enum unittester_cmd cmd_id; uint32_t write_offs; @@ -76,6 +77,19 @@ struct unittester_state { uint32_t write_len; uint32_t read_len; + /* Screen snapshot state */ + /* Monitor to take snapshot on */ + uint8_t snap_monitor; + /* Main image width + height */ + uint16_t snap_img_width; + uint16_t snap_img_height; + /* Fully overscanned image width + height */ + uint16_t snap_overscan_width; + uint16_t snap_overscan_height; + /* Offset of actual image within overscanned area */ + uint16_t snap_img_xoffs; + uint16_t snap_img_yoffs; + /* Command-specific state */ /* 0x04: Exit */ uint8_t exit_code; @@ -90,6 +104,9 @@ static const struct unittester_state unittester_defaults = { .cmd_id = UT_CMD_NOOP, }; +/* Kept separate, as we will be reusing this object */ +static bitmap_t *unittester_screen_buffer = NULL; + /* FIXME: This needs a config option! --GM */ static bool unittester_exit_enabled = true; @@ -133,6 +150,13 @@ unittester_write(uint16_t port, uint8_t val, UNUSED(void *priv)) unittester.status = UT_STATUS_IDLE; break; + /* 0x01: Capture Screen Snapshot */ + case UT_CMD_CAPTURE_SCREEN_SNAPSHOT: + unittester.cmd_id = UT_CMD_CAPTURE_SCREEN_SNAPSHOT; + unittester.status = UT_STATUS_AWAITING_WRITE; + unittester.write_len = 1; + break; + /* 0x04: Exit */ case UT_CMD_EXIT: unittester.cmd_id = UT_CMD_EXIT; @@ -166,6 +190,16 @@ unittester_write(uint16_t port, uint8_t val, UNUSED(void *priv)) } break; + case UT_CMD_CAPTURE_SCREEN_SNAPSHOT: + switch(unittester.write_offs) { + case 0: + unittester.snap_monitor = val; + break; + default: + break; + } + break; + /* This should not be reachable, but just in case... */ default: break; @@ -194,9 +228,47 @@ unittester_write(uint16_t port, uint8_t val, UNUSED(void *priv)) } else { /* No - report successful command completion and continue program execution */ unittester_log("[UT] Exit disabled, continuing execution\n"); - unittester.cmd_id = UT_CMD_NOOP; - unittester.status = UT_STATUS_IDLE; } + unittester.cmd_id = UT_CMD_NOOP; + unittester.status = UT_STATUS_IDLE; + break; + + case UT_CMD_CAPTURE_SCREEN_SNAPSHOT: + /* Recompute screen */ + unittester.snap_img_width = 0; + unittester.snap_img_height = 0; + unittester.snap_img_xoffs = 0; + unittester.snap_img_yoffs = 0; + unittester.snap_overscan_width = 0; + unittester.snap_overscan_height = 0; + if (unittester.snap_monitor < 0x01 || (unittester.snap_monitor - 1) > MONITORS_NUM) { + /* No monitor here - clear snapshot */ + unittester.snap_monitor = 0x00; + } else if (video_get_type_monitor(unittester.snap_monitor - 1) == VIDEO_FLAG_TYPE_NONE) { + /* Monitor disabled - clear snapshot */ + unittester.snap_monitor = 0x00; + } else { + /* Capture snapshot from monitor */ + const monitor_t *m = &monitors[unittester.snap_monitor - 1]; + unittester.snap_img_width = m->mon_xsize; + unittester.snap_img_height = m->mon_ysize; + unittester.snap_overscan_width = m->mon_xsize + m->mon_overscan_x; + unittester.snap_overscan_height = m->mon_ysize + m->mon_overscan_y; + unittester.snap_img_xoffs = (m->mon_overscan_x >> 1); + unittester.snap_img_yoffs = (m->mon_overscan_y >> 1); + /* TODO: Actually take snapshot! --GM */ + } + + /* We have 12 bytes to read. */ + unittester_log("[UT] Screen snapshot - image %d x %d @ (%d, %d) in overscan %d x %d\n", + unittester.snap_img_width, + unittester.snap_img_height, + unittester.snap_img_xoffs, + unittester.snap_img_yoffs, + unittester.snap_overscan_width, + unittester.snap_overscan_height); + unittester.status = UT_STATUS_AWAITING_READ; + unittester.read_len = 12; break; default: @@ -230,6 +302,49 @@ unittester_read(uint16_t port, UNUSED(void *priv)) return 0xFF; switch (unittester.cmd_id) { + case UT_CMD_CAPTURE_SCREEN_SNAPSHOT: + switch(unittester.read_offs) { + case 0: + outval = (uint8_t)(unittester.snap_img_width); + break; + case 1: + outval = (uint8_t)(unittester.snap_img_width>>8); + break; + case 2: + outval = (uint8_t)(unittester.snap_img_height); + break; + case 3: + outval = (uint8_t)(unittester.snap_img_height>>8); + break; + case 4: + outval = (uint8_t)(unittester.snap_overscan_width); + break; + case 5: + outval = (uint8_t)(unittester.snap_overscan_width>>8); + break; + case 6: + outval = (uint8_t)(unittester.snap_overscan_height); + break; + case 7: + outval = (uint8_t)(unittester.snap_overscan_height>>8); + break; + case 8: + outval = (uint8_t)(unittester.snap_img_xoffs); + break; + case 9: + outval = (uint8_t)(unittester.snap_img_xoffs>>8); + break; + case 10: + outval = (uint8_t)(unittester.snap_img_yoffs); + break; + case 11: + outval = (uint8_t)(unittester.snap_img_yoffs>>8); + break; + default: + break; + } + break; + /* This should not be reachable, but just in case... */ default: break; @@ -330,6 +445,9 @@ unittester_trigger_write(UNUSED(uint16_t port), uint8_t val, UNUSED(void *priv)) static void * unittester_init(UNUSED(const device_t *info)) { + if (unittester_screen_buffer == NULL) + unittester_screen_buffer = create_bitmap(2048, 2048); + unittester = (struct unittester_state)unittester_defaults; io_sethandler(unittester.trigger_port, 1, NULL, NULL, NULL, unittester_trigger_write, NULL, NULL, NULL); @@ -347,6 +465,11 @@ unittester_close(UNUSED(void *priv)) io_removehandler(unittester.iobase_port, 2, unittester_read, NULL, NULL, unittester_write, NULL, NULL, NULL); unittester.iobase_port = 0xFFFF; + if (unittester_screen_buffer != NULL) { + destroy_bitmap(unittester_screen_buffer); + unittester_screen_buffer = NULL; + } + unittester_log("[UT] 86Box Unit Tester closed\n"); } From 2e020584cf8bd87f37b161a1060379278992eeac Mon Sep 17 00:00:00 2001 From: GreaseMonkey Date: Sun, 7 Jan 2024 21:32:59 +1300 Subject: [PATCH 12/20] unittester: Finish implementing 0x01 "Capture Screen Snapshot" And it's looking like the overscan bounds and offset calculation will need to be correct. Otherwise, things will break. Let's see what happens when I get command 0x02 working... --- doc/specifications/86box-unit-tester.md | 2 -- src/device/unittester.c | 9 +++++++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/doc/specifications/86box-unit-tester.md b/doc/specifications/86box-unit-tester.md index fe0203b46..c58b15960 100644 --- a/doc/specifications/86box-unit-tester.md +++ b/doc/specifications/86box-unit-tester.md @@ -101,8 +101,6 @@ This is an easy way to reset the status to 0x04 (no command in flight, not waiti ### 0x01: Capture Screen Snapshot -**TODO: IMPLEMENT ME!** - Captures a snapshot of the current screen state and stores it in the current snapshot buffer. The initial state of the screen snapshot buffer has an image area of 0x0, an overscanned area of 0x0, and an image start offset of (0,0). diff --git a/src/device/unittester.c b/src/device/unittester.c index 78086fd90..c020a81ee 100644 --- a/src/device/unittester.c +++ b/src/device/unittester.c @@ -248,7 +248,7 @@ unittester_write(uint16_t port, uint8_t val, UNUSED(void *priv)) /* Monitor disabled - clear snapshot */ unittester.snap_monitor = 0x00; } else { - /* Capture snapshot from monitor */ + /* Compute bounds for snapshot */ const monitor_t *m = &monitors[unittester.snap_monitor - 1]; unittester.snap_img_width = m->mon_xsize; unittester.snap_img_height = m->mon_ysize; @@ -256,7 +256,12 @@ unittester_write(uint16_t port, uint8_t val, UNUSED(void *priv)) unittester.snap_overscan_height = m->mon_ysize + m->mon_overscan_y; unittester.snap_img_xoffs = (m->mon_overscan_x >> 1); unittester.snap_img_yoffs = (m->mon_overscan_y >> 1); - /* TODO: Actually take snapshot! --GM */ + /* Take snapshot */ + for (size_t y = 0; y < unittester.snap_img_height; y++) { + for (size_t x = 0; x < unittester.snap_img_width; x++) { + unittester_screen_buffer->line[y][x] = m->target_buffer->line[y][x]; + } + } } /* We have 12 bytes to read. */ From 678874cd42289b5a873b4c9a7dd7c2b97adb26c9 Mon Sep 17 00:00:00 2001 From: GreaseMonkey Date: Sun, 7 Jan 2024 22:24:32 +1300 Subject: [PATCH 13/20] unittester: Implement 0x02 "Read Screen Snapshot Rectangle" This will need some extra testing but it does appear to be at least somewhat functional. --- doc/specifications/86box-unit-tester.md | 4 +- src/device/unittester.c | 94 ++++++++++++++++++++++++- 2 files changed, 93 insertions(+), 5 deletions(-) diff --git a/doc/specifications/86box-unit-tester.md b/doc/specifications/86box-unit-tester.md index c58b15960..ee77de8d4 100644 --- a/doc/specifications/86box-unit-tester.md +++ b/doc/specifications/86box-unit-tester.md @@ -126,9 +126,7 @@ If there is no screen snapshot, then all values will be 0 as per the initial scr ### 0x02: Read Screen Snapshot Rectangle -**TODO: IMPLEMENT ME!** - -Returns a rectangular snapshot of the screen snapshot buffer. +Returns a rectangular snapshot of the screen snapshot buffer as an array of 32bpp 8:8:8:8 B:G:R:X pixels. Input: diff --git a/src/device/unittester.c b/src/device/unittester.c index c020a81ee..fc864b965 100644 --- a/src/device/unittester.c +++ b/src/device/unittester.c @@ -91,6 +91,13 @@ struct unittester_state { uint16_t snap_img_yoffs; /* Command-specific state */ + /* 0x02: Read Screen Snapshot Rectangle */ + /* 0x03: Verify Screen Snapshot Rectangle */ + uint16_t read_snap_width; + uint16_t read_snap_height; + int16_t read_snap_xoffs; + int16_t read_snap_yoffs; + /* 0x04: Exit */ uint8_t exit_code; }; @@ -157,6 +164,13 @@ unittester_write(uint16_t port, uint8_t val, UNUSED(void *priv)) unittester.write_len = 1; break; + /* 0x02: Read Screen Snapshot Rectangle */ + case UT_CMD_READ_SCREEN_SNAPSHOT_RECTANGLE: + unittester.cmd_id = UT_CMD_READ_SCREEN_SNAPSHOT_RECTANGLE; + unittester.status = UT_STATUS_AWAITING_WRITE; + unittester.write_len = 8; + break; + /* 0x04: Exit */ case UT_CMD_EXIT: unittester.cmd_id = UT_CMD_EXIT; @@ -200,6 +214,37 @@ unittester_write(uint16_t port, uint8_t val, UNUSED(void *priv)) } break; + case UT_CMD_READ_SCREEN_SNAPSHOT_RECTANGLE: + switch(unittester.write_offs) { + case 0: + unittester.read_snap_width = (uint16_t)val; + break; + case 1: + unittester.read_snap_width |= ((uint16_t)val) << 8; + break; + case 2: + unittester.read_snap_height = (uint16_t)val; + break; + case 3: + unittester.read_snap_height |= ((uint16_t)val) << 8; + break; + case 4: + unittester.read_snap_xoffs = (uint16_t)val; + break; + case 5: + unittester.read_snap_xoffs |= ((uint16_t)val) << 8; + break; + case 6: + unittester.read_snap_yoffs = (uint16_t)val; + break; + case 7: + unittester.read_snap_yoffs |= ((uint16_t)val) << 8; + break; + default: + break; + } + break; + /* This should not be reachable, but just in case... */ default: break; @@ -257,8 +302,8 @@ unittester_write(uint16_t port, uint8_t val, UNUSED(void *priv)) unittester.snap_img_xoffs = (m->mon_overscan_x >> 1); unittester.snap_img_yoffs = (m->mon_overscan_y >> 1); /* Take snapshot */ - for (size_t y = 0; y < unittester.snap_img_height; y++) { - for (size_t x = 0; x < unittester.snap_img_width; x++) { + for (size_t y = 0; y < unittester.snap_overscan_height; y++) { + for (size_t x = 0; x < unittester.snap_overscan_width; x++) { unittester_screen_buffer->line[y][x] = m->target_buffer->line[y][x]; } } @@ -276,6 +321,32 @@ unittester_write(uint16_t port, uint8_t val, UNUSED(void *priv)) unittester.read_len = 12; break; + case UT_CMD_READ_SCREEN_SNAPSHOT_RECTANGLE: + /* Offset the X,Y offsets by the overscan offsets. */ + unittester.read_snap_xoffs += (int16_t)unittester.snap_img_xoffs; + unittester.read_snap_yoffs += (int16_t)unittester.snap_img_yoffs; + /*BUG: If the width and height are too large, this ends up with a multiplication overflow. + In practice, this means we end up sending less than we would want to. + However: + - A width and height of that size is obscene, and goes beyond what one would reasonably want. + - The consequences of triggering this bug are that one ends up with a bunch of FF FF FF FF pixels. + - Special-casing this would add unnecessary complexity. + So, this bug is kept here. + If there is any need to fix this bug... then go and make the variables 64-bit :P + */ + unittester.read_len = ((uint32_t)unittester.read_snap_width) * ((uint32_t)unittester.read_snap_height) * 4; + + /* Do we have anything to read? */ + if (unittester.read_len >= 1) { + /* Yes - start reads! */ + unittester.status = UT_STATUS_AWAITING_READ; + } else { + /* No - stop here. */ + unittester.cmd_id = UT_CMD_NOOP; + unittester.status = UT_STATUS_IDLE; + } + break; + default: /* Nothing to write? Stop here. */ unittester.cmd_id = UT_CMD_NOOP; @@ -350,6 +421,25 @@ unittester_read(uint16_t port, UNUSED(void *priv)) } break; + case UT_CMD_READ_SCREEN_SNAPSHOT_RECTANGLE: + /* WARNING: If the width is somehow 0 and wasn't caught earlier, you'll probably get a divide by zero crash. */ + { + uint32_t idx = unittester.read_offs & 0x3; + int32_t x = (unittester.read_offs >> 2) % unittester.read_snap_width; + int32_t y = (unittester.read_offs >> 2) / unittester.read_snap_width; + x += unittester.read_snap_xoffs; + y += unittester.read_snap_yoffs; + + if (x < 0 || y < 0 || x >= unittester.snap_overscan_width || y >= unittester.snap_overscan_height) { + /* Out of range! */ + outval = (idx == 3 ? 0xFF : 0x00); + } else { + /* In range */ + outval = (unittester_screen_buffer->line[y][x] & 0x00FFFFFF)>>(idx*8); + } + } + break; + /* This should not be reachable, but just in case... */ default: break; From f35dd209745aaa09cf133c2f9adfbbcdd4b3e4f0 Mon Sep 17 00:00:00 2001 From: GreaseMonkey Date: Mon, 8 Jan 2024 10:45:13 +1300 Subject: [PATCH 14/20] unittester: Reduce spam --- src/device/unittester.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/device/unittester.c b/src/device/unittester.c index fc864b965..1736a70d8 100644 --- a/src/device/unittester.c +++ b/src/device/unittester.c @@ -371,7 +371,7 @@ unittester_read(uint16_t port, UNUSED(void *priv)) return unittester.status; } else if (port == unittester.iobase_port+0x01) { /* Data port */ - unittester_log("[UT] R -- Data\n"); + /* unittester_log("[UT] R -- Data\n"); */ /* Skip if not awaiting */ if ((unittester.status & UT_STATUS_AWAITING_READ) == 0) From 30aacb2a1a7734e5ef8802869a318f32746f0791 Mon Sep 17 00:00:00 2001 From: GreaseMonkey Date: Mon, 8 Jan 2024 12:07:14 +1300 Subject: [PATCH 15/20] unittester: Implement 0x03 "Verify Screen Snapshot Rectangle" Basic quick tests show that this is probably consistent with command 0x02. --- doc/specifications/86box-unit-tester.md | 4 -- src/device/unittester.c | 79 ++++++++++++++++++------- 2 files changed, 58 insertions(+), 25 deletions(-) diff --git a/doc/specifications/86box-unit-tester.md b/doc/specifications/86box-unit-tester.md index ee77de8d4..8d8d5bbb6 100644 --- a/doc/specifications/86box-unit-tester.md +++ b/doc/specifications/86box-unit-tester.md @@ -49,8 +49,6 @@ Set the I/O base address to 0xFFFF using the above method. ### Executing commands -**TODO: IMPLEMENT ME!** - The ports at IOBASE+0x00 and IOBASE+0x01 are all 8 bits wide. Writing to IOBASE+0x00 cancels any in-flight commands and sends a new command. @@ -145,8 +143,6 @@ Output: ### 0x03: Verify Screen Snapshot Rectangle -**TODO: IMPLEMENT ME!** - As per 0x02 "Read Screen Snapshot Rectangle", except instead of returning the pixel data, it returns a CRC-32 of the data. The CRC is as per zlib's `crc32()` function. Specifically, one uses a right-shifting Galois LFSR with a polynomial of 0xEDB88320, bytes XORed against the least significant byte, the initial seed is 0xFFFFFFFF, and all bits of the output are inverted. diff --git a/src/device/unittester.c b/src/device/unittester.c index 1736a70d8..55bc270e9 100644 --- a/src/device/unittester.c +++ b/src/device/unittester.c @@ -97,6 +97,7 @@ struct unittester_state { uint16_t read_snap_height; int16_t read_snap_xoffs; int16_t read_snap_yoffs; + uint32_t read_snap_crc; /* 0x04: Exit */ uint8_t exit_code; @@ -138,6 +139,25 @@ unittester_log(const char *fmt, ...) # define unittester_log(fmt, ...) #endif +static uint8_t +unittester_read_snap_rect_idx(uint32_t offs) +{ + /* WARNING: If the width is somehow 0 and wasn't caught earlier, you'll probably get a divide by zero crash. */ + uint32_t idx = (offs & 0x3); + int32_t x = (offs >> 2) % unittester.read_snap_width; + int32_t y = (offs >> 2) / unittester.read_snap_width; + x += unittester.read_snap_xoffs; + y += unittester.read_snap_yoffs; + + if (x < 0 || y < 0 || x >= unittester.snap_overscan_width || y >= unittester.snap_overscan_height) { + /* Out of range! */ + return (idx == 3 ? 0xFF : 0x00); + } else { + /* In range */ + return (unittester_screen_buffer->line[y][x] & 0x00FFFFFF)>>(idx*8); + } +} + static void unittester_write(uint16_t port, uint8_t val, UNUSED(void *priv)) { @@ -171,6 +191,13 @@ unittester_write(uint16_t port, uint8_t val, UNUSED(void *priv)) unittester.write_len = 8; break; + /* 0x03: Verify Screen Snapshot Rectangle */ + case UT_CMD_VERIFY_SCREEN_SNAPSHOT_RECTANGLE: + unittester.cmd_id = UT_CMD_VERIFY_SCREEN_SNAPSHOT_RECTANGLE; + unittester.status = UT_STATUS_AWAITING_WRITE; + unittester.write_len = 8; + break; + /* 0x04: Exit */ case UT_CMD_EXIT: unittester.cmd_id = UT_CMD_EXIT; @@ -215,6 +242,7 @@ unittester_write(uint16_t port, uint8_t val, UNUSED(void *priv)) break; case UT_CMD_READ_SCREEN_SNAPSHOT_RECTANGLE: + case UT_CMD_VERIFY_SCREEN_SNAPSHOT_RECTANGLE: switch(unittester.write_offs) { case 0: unittester.read_snap_width = (uint16_t)val; @@ -322,6 +350,7 @@ unittester_write(uint16_t port, uint8_t val, UNUSED(void *priv)) break; case UT_CMD_READ_SCREEN_SNAPSHOT_RECTANGLE: + case UT_CMD_VERIFY_SCREEN_SNAPSHOT_RECTANGLE: /* Offset the X,Y offsets by the overscan offsets. */ unittester.read_snap_xoffs += (int16_t)unittester.snap_img_xoffs; unittester.read_snap_yoffs += (int16_t)unittester.snap_img_yoffs; @@ -335,15 +364,34 @@ unittester_write(uint16_t port, uint8_t val, UNUSED(void *priv)) If there is any need to fix this bug... then go and make the variables 64-bit :P */ unittester.read_len = ((uint32_t)unittester.read_snap_width) * ((uint32_t)unittester.read_snap_height) * 4; + unittester.read_snap_crc = 0xFFFFFFFF; - /* Do we have anything to read? */ - if (unittester.read_len >= 1) { - /* Yes - start reads! */ + if (unittester.cmd_id == UT_CMD_VERIFY_SCREEN_SNAPSHOT_RECTANGLE) { + /* Read everything and compute CRC */ + uint32_t crc = 0xFFFFFFFF; + for (uint32_t i = 0; i < unittester.read_len; i++) { + crc ^= 0xFF & (uint32_t)unittester_read_snap_rect_idx(i); + /* Use some bit twiddling until we have a table-based fast CRC-32 implementation */ + for (uint32_t j = 0; j < 8; j++) { + crc = (crc >> 1) ^ ((-(crc&0x1)) & 0xEDB88320); + } + } + unittester.read_snap_crc = crc ^ 0xFFFFFFFF; + + /* Set actual read length for CRC result */ + unittester.read_len = 4; unittester.status = UT_STATUS_AWAITING_READ; + } else { - /* No - stop here. */ - unittester.cmd_id = UT_CMD_NOOP; - unittester.status = UT_STATUS_IDLE; + /* Do we have anything to read? */ + if (unittester.read_len >= 1) { + /* Yes - start reads! */ + unittester.status = UT_STATUS_AWAITING_READ; + } else { + /* No - stop here. */ + unittester.cmd_id = UT_CMD_NOOP; + unittester.status = UT_STATUS_IDLE; + } } break; @@ -422,22 +470,11 @@ unittester_read(uint16_t port, UNUSED(void *priv)) break; case UT_CMD_READ_SCREEN_SNAPSHOT_RECTANGLE: - /* WARNING: If the width is somehow 0 and wasn't caught earlier, you'll probably get a divide by zero crash. */ - { - uint32_t idx = unittester.read_offs & 0x3; - int32_t x = (unittester.read_offs >> 2) % unittester.read_snap_width; - int32_t y = (unittester.read_offs >> 2) / unittester.read_snap_width; - x += unittester.read_snap_xoffs; - y += unittester.read_snap_yoffs; + outval = unittester_read_snap_rect_idx(unittester.read_offs); + break; - if (x < 0 || y < 0 || x >= unittester.snap_overscan_width || y >= unittester.snap_overscan_height) { - /* Out of range! */ - outval = (idx == 3 ? 0xFF : 0x00); - } else { - /* In range */ - outval = (unittester_screen_buffer->line[y][x] & 0x00FFFFFF)>>(idx*8); - } - } + case UT_CMD_VERIFY_SCREEN_SNAPSHOT_RECTANGLE: + outval = (uint8_t)(unittester.read_snap_crc >> (8*unittester.read_offs)); break; /* This should not be reachable, but just in case... */ From e5f467918ce791f1b391437ffffd2e32d6b30bc6 Mon Sep 17 00:00:00 2001 From: GreaseMonkey Date: Mon, 8 Jan 2024 12:54:31 +1300 Subject: [PATCH 16/20] unittester: Cleanups and specification v1.0.0 finalisation --- doc/specifications/86box-unit-tester.md | 57 +++++++++++++++++++++---- src/device/unittester.c | 5 +-- src/include/86box/unittester.h | 2 + 3 files changed, 52 insertions(+), 12 deletions(-) diff --git a/doc/specifications/86box-unit-tester.md b/doc/specifications/86box-unit-tester.md index 8d8d5bbb6..48817b27c 100644 --- a/doc/specifications/86box-unit-tester.md +++ b/doc/specifications/86box-unit-tester.md @@ -1,6 +1,51 @@ -# 86Box Unit Tester device specification +# 86Box Unit Tester device specification v1.0.0 -**TODO: DESCRIBE ME!** +By GreaseMonkey + other 86Box contributors, 2024. +This specification, including any code samples included, has been released into the Public Domain under the Creative Commons CC0 licence version 1.0 or later, as described here: + +The 86Box Unit Tester is a facility for allowing one to unit-test various parts of 86Box's emulation which would otherwise not be exposed to the emulated system. + +The original purpose of this was to make it possible to analyse and verify aspects of the monitor framebuffers in order to detect and prevent regressions in certain pieces of video hardware. + +---------------------------------------------------------------------------- + +## Versioning + +This specification follows the rules of Semantic Versioning 2.0.0 as documented here: + +The format is `major.minor.patch`. + +- Before you mess with this specification, talk to the other contributors first! +- Any changes need to be tracked in the Version History below, mostly in the event that this document escapes into the wild and doesn't have the Git history attached to it. +- If it clarifies something without introducing any behaviour changes (e.g. formatting changes, spelling fixes), increment the patch version. +- If it introduces a backwards-compatible change, increment the minor version and reset the patch version to 0. +- If it introduces a backwards-incompatible change, increment the major version and reset the minor and patch versions to 0. + - If you make a mistake and accidentally introduce a backward-incompatible change, fix the mistake and increment the minor version. + - To clarify, modifications to *this* section are to be classified as a *patch* version update. +- If you understand SemVer 2.0.0, you may also do other things to the version number according to the specification. + +And lastly, the 3 golden rules of protocol specifications: + +1. If it's not documented, it doesn't exist. +2. If it's not documented but somehow exists, it's a bug. +3. If it's a bug, it needs to be fixed. (Yes, I'm talking to you. You who introduced the bug. Go fix it.) + +The checklist: + +- Work out what kind of version number this document needs. +- Update the version number at the top of the file. +- Add an entry to the "Version History" section below describing roughly what was changed. + +---------------------------------------------------------------------------- + +## Version History + +Dates are based on what day it was in UTC at the time of publication. + +New entries are placed at the top. That is, immediately following this paragraph. + +### v1.0.0 (2024-01-08) +Initial release. Authored by GreaseMonkey. ---------------------------------------------------------------------------- @@ -12,7 +57,7 @@ - `u8` denotes an unsigned 8-bit value. - `w8` denotes an 8-bit value which wraps around. - `x8` denotes an 8-bit value where the signedness is irrelevant. -- `e8` ("either") denotes an 8-bit value where the most significant bit is clear - in effect, this is a 7-bit unsigned value. +- `e8` ("either") denotes an 8-bit value where the most significant bit is clear - in effect, this is a 7-bit unsigned value, and can be interepreted identically as a signed 8-bit value. - `u16L` denotes a little-endian unsigned 16-bit value. - `u16B` would denote a big-endian unsigned 16-bit value if we had any big-endian values. - `[N]T` denotes an array of `N` values of type `T`, whatever `N` and `T` are. @@ -201,12 +246,6 @@ FSM2: Once received, replace IOBASE with this byte in the high byte and the temporary value in the low byte, then go back to "Idle". -### Command processing state machine - -**TODO: IMPLEMENT ME!** - -**TODO: DOCUMENT ME!** - ---------------------------------------------------------------------------- ## Extending the protocol diff --git a/src/device/unittester.c b/src/device/unittester.c index 55bc270e9..047137752 100644 --- a/src/device/unittester.c +++ b/src/device/unittester.c @@ -8,6 +8,8 @@ * * Debug device for assisting in unit testing. * See doc/specifications/86box-unit-tester.md for more info. + * If modifying the protocol, you MUST modify the specification + * and increment the version number. * * * @@ -118,9 +120,6 @@ static bitmap_t *unittester_screen_buffer = NULL; /* FIXME: This needs a config option! --GM */ static bool unittester_exit_enabled = true; -/* FIXME TEMPORARY --GM */ -#define ENABLE_UNITTESTER_LOG 1 - #ifdef ENABLE_UNITTESTER_LOG int unittester_do_log = ENABLE_UNITTESTER_LOG; diff --git a/src/include/86box/unittester.h b/src/include/86box/unittester.h index f6dedeae5..e83add67f 100644 --- a/src/include/86box/unittester.h +++ b/src/include/86box/unittester.h @@ -8,6 +8,8 @@ * * Debug device for assisting in unit testing. * See doc/specifications/86box-unit-tester.md for more info. + * If modifying the protocol, you MUST modify the specification + * and increment the version number. * * * From 4648092b123bbafbdb484375ae45a77395d206ff Mon Sep 17 00:00:00 2001 From: GreaseMonkey Date: Mon, 8 Jan 2024 12:59:02 +1300 Subject: [PATCH 17/20] unittester: Fix that one bug I wasn't going to fix I might as well not be a hypocrite here. --- src/device/unittester.c | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/src/device/unittester.c b/src/device/unittester.c index 047137752..ab948cb87 100644 --- a/src/device/unittester.c +++ b/src/device/unittester.c @@ -75,9 +75,9 @@ struct unittester_state { uint8_t status; enum unittester_cmd cmd_id; uint32_t write_offs; - uint32_t read_offs; uint32_t write_len; - uint32_t read_len; + uint64_t read_offs; + uint64_t read_len; /* Screen snapshot state */ /* Monitor to take snapshot on */ @@ -139,12 +139,12 @@ unittester_log(const char *fmt, ...) #endif static uint8_t -unittester_read_snap_rect_idx(uint32_t offs) +unittester_read_snap_rect_idx(uint64_t offs) { /* WARNING: If the width is somehow 0 and wasn't caught earlier, you'll probably get a divide by zero crash. */ uint32_t idx = (offs & 0x3); - int32_t x = (offs >> 2) % unittester.read_snap_width; - int32_t y = (offs >> 2) / unittester.read_snap_width; + int64_t x = (offs >> 2) % unittester.read_snap_width; + int64_t y = (offs >> 2) / unittester.read_snap_width; x += unittester.read_snap_xoffs; y += unittester.read_snap_yoffs; @@ -353,22 +353,18 @@ unittester_write(uint16_t port, uint8_t val, UNUSED(void *priv)) /* Offset the X,Y offsets by the overscan offsets. */ unittester.read_snap_xoffs += (int16_t)unittester.snap_img_xoffs; unittester.read_snap_yoffs += (int16_t)unittester.snap_img_yoffs; - /*BUG: If the width and height are too large, this ends up with a multiplication overflow. - In practice, this means we end up sending less than we would want to. - However: - - A width and height of that size is obscene, and goes beyond what one would reasonably want. - - The consequences of triggering this bug are that one ends up with a bunch of FF FF FF FF pixels. - - Special-casing this would add unnecessary complexity. - So, this bug is kept here. - If there is any need to fix this bug... then go and make the variables 64-bit :P + /* NOTE: Width * Height * 4 can potentially exceed a 32-bit number. + So, we use 64-bit numbers instead. + In practice, this will only happen if someone decides to request e.g. a 65535 x 65535 image, + of which most of the pixels will be out of range anyway. */ - unittester.read_len = ((uint32_t)unittester.read_snap_width) * ((uint32_t)unittester.read_snap_height) * 4; + unittester.read_len = ((uint64_t)unittester.read_snap_width) * ((uint64_t)unittester.read_snap_height) * 4; unittester.read_snap_crc = 0xFFFFFFFF; if (unittester.cmd_id == UT_CMD_VERIFY_SCREEN_SNAPSHOT_RECTANGLE) { /* Read everything and compute CRC */ uint32_t crc = 0xFFFFFFFF; - for (uint32_t i = 0; i < unittester.read_len; i++) { + for (uint64_t i = 0; i < unittester.read_len; i++) { crc ^= 0xFF & (uint32_t)unittester_read_snap_rect_idx(i); /* Use some bit twiddling until we have a table-based fast CRC-32 implementation */ for (uint32_t j = 0; j < 8; j++) { From 5a2e3611d95468d9429c39c6e538edc03e8ee44b Mon Sep 17 00:00:00 2001 From: GreaseMonkey Date: Mon, 8 Jan 2024 13:01:55 +1300 Subject: [PATCH 18/20] unittester: Apply clang-format --- src/device/unittester.c | 156 ++++++++++++++++----------------- src/include/86box/unittester.h | 1 - 2 files changed, 78 insertions(+), 79 deletions(-) diff --git a/src/device/unittester.c b/src/device/unittester.c index ab948cb87..f8f7fa56e 100644 --- a/src/device/unittester.c +++ b/src/device/unittester.c @@ -45,18 +45,18 @@ enum fsm2_value { }; /* Status bit mask */ -#define UT_STATUS_AWAITING_READ (1<<0) -#define UT_STATUS_AWAITING_WRITE (1<<1) -#define UT_STATUS_IDLE (1<<2) -#define UT_STATUS_UNSUPPORTED_CMD (1<<3) +#define UT_STATUS_AWAITING_READ (1 << 0) +#define UT_STATUS_AWAITING_WRITE (1 << 1) +#define UT_STATUS_IDLE (1 << 2) +#define UT_STATUS_UNSUPPORTED_CMD (1 << 3) /* Command list */ enum unittester_cmd { - UT_CMD_NOOP = 0x00, - UT_CMD_CAPTURE_SCREEN_SNAPSHOT = 0x01, - UT_CMD_READ_SCREEN_SNAPSHOT_RECTANGLE = 0x02, + UT_CMD_NOOP = 0x00, + UT_CMD_CAPTURE_SCREEN_SNAPSHOT = 0x01, + UT_CMD_READ_SCREEN_SNAPSHOT_RECTANGLE = 0x02, UT_CMD_VERIFY_SCREEN_SNAPSHOT_RECTANGLE = 0x03, - UT_CMD_EXIT = 0x04, + UT_CMD_EXIT = 0x04, }; struct unittester_state { @@ -81,7 +81,7 @@ struct unittester_state { /* Screen snapshot state */ /* Monitor to take snapshot on */ - uint8_t snap_monitor; + uint8_t snap_monitor; /* Main image width + height */ uint16_t snap_img_width; uint16_t snap_img_height; @@ -97,14 +97,14 @@ struct unittester_state { /* 0x03: Verify Screen Snapshot Rectangle */ uint16_t read_snap_width; uint16_t read_snap_height; - int16_t read_snap_xoffs; - int16_t read_snap_yoffs; + int16_t read_snap_xoffs; + int16_t read_snap_yoffs; uint32_t read_snap_crc; /* 0x04: Exit */ uint8_t exit_code; }; -static struct unittester_state unittester; +static struct unittester_state unittester; static const struct unittester_state unittester_defaults = { .trigger_port = 0x0080, .iobase_port = 0xFFFF, @@ -143,8 +143,8 @@ unittester_read_snap_rect_idx(uint64_t offs) { /* WARNING: If the width is somehow 0 and wasn't caught earlier, you'll probably get a divide by zero crash. */ uint32_t idx = (offs & 0x3); - int64_t x = (offs >> 2) % unittester.read_snap_width; - int64_t y = (offs >> 2) / unittester.read_snap_width; + int64_t x = (offs >> 2) % unittester.read_snap_width; + int64_t y = (offs >> 2) / unittester.read_snap_width; x += unittester.read_snap_xoffs; y += unittester.read_snap_yoffs; @@ -153,21 +153,21 @@ unittester_read_snap_rect_idx(uint64_t offs) return (idx == 3 ? 0xFF : 0x00); } else { /* In range */ - return (unittester_screen_buffer->line[y][x] & 0x00FFFFFF)>>(idx*8); + return (unittester_screen_buffer->line[y][x] & 0x00FFFFFF) >> (idx * 8); } } static void unittester_write(uint16_t port, uint8_t val, UNUSED(void *priv)) { - if (port == unittester.iobase_port+0x00) { + if (port == unittester.iobase_port + 0x00) { /* Command port */ unittester_log("[UT] W %02X Command\n", val); unittester.write_offs = 0; - unittester.write_len = 0; - unittester.read_offs = 0; - unittester.read_len = 0; + unittester.write_len = 0; + unittester.read_offs = 0; + unittester.read_len = 0; switch (val) { /* 0x00: No-op */ @@ -178,29 +178,29 @@ unittester_write(uint16_t port, uint8_t val, UNUSED(void *priv)) /* 0x01: Capture Screen Snapshot */ case UT_CMD_CAPTURE_SCREEN_SNAPSHOT: - unittester.cmd_id = UT_CMD_CAPTURE_SCREEN_SNAPSHOT; - unittester.status = UT_STATUS_AWAITING_WRITE; + unittester.cmd_id = UT_CMD_CAPTURE_SCREEN_SNAPSHOT; + unittester.status = UT_STATUS_AWAITING_WRITE; unittester.write_len = 1; break; /* 0x02: Read Screen Snapshot Rectangle */ case UT_CMD_READ_SCREEN_SNAPSHOT_RECTANGLE: - unittester.cmd_id = UT_CMD_READ_SCREEN_SNAPSHOT_RECTANGLE; - unittester.status = UT_STATUS_AWAITING_WRITE; + unittester.cmd_id = UT_CMD_READ_SCREEN_SNAPSHOT_RECTANGLE; + unittester.status = UT_STATUS_AWAITING_WRITE; unittester.write_len = 8; break; /* 0x03: Verify Screen Snapshot Rectangle */ case UT_CMD_VERIFY_SCREEN_SNAPSHOT_RECTANGLE: - unittester.cmd_id = UT_CMD_VERIFY_SCREEN_SNAPSHOT_RECTANGLE; - unittester.status = UT_STATUS_AWAITING_WRITE; + unittester.cmd_id = UT_CMD_VERIFY_SCREEN_SNAPSHOT_RECTANGLE; + unittester.status = UT_STATUS_AWAITING_WRITE; unittester.write_len = 8; break; /* 0x04: Exit */ case UT_CMD_EXIT: - unittester.cmd_id = UT_CMD_EXIT; - unittester.status = UT_STATUS_AWAITING_WRITE; + unittester.cmd_id = UT_CMD_EXIT; + unittester.status = UT_STATUS_AWAITING_WRITE; unittester.write_len = 1; break; @@ -211,7 +211,7 @@ unittester_write(uint16_t port, uint8_t val, UNUSED(void *priv)) break; } - } else if (port == unittester.iobase_port+0x01) { + } else if (port == unittester.iobase_port + 0x01) { /* Data port */ unittester_log("[UT] W %02X Data\n", val); @@ -221,7 +221,7 @@ unittester_write(uint16_t port, uint8_t val, UNUSED(void *priv)) switch (unittester.cmd_id) { case UT_CMD_EXIT: - switch(unittester.write_offs) { + switch (unittester.write_offs) { case 0: unittester.exit_code = val; break; @@ -231,7 +231,7 @@ unittester_write(uint16_t port, uint8_t val, UNUSED(void *priv)) break; case UT_CMD_CAPTURE_SCREEN_SNAPSHOT: - switch(unittester.write_offs) { + switch (unittester.write_offs) { case 0: unittester.snap_monitor = val; break; @@ -242,30 +242,30 @@ unittester_write(uint16_t port, uint8_t val, UNUSED(void *priv)) case UT_CMD_READ_SCREEN_SNAPSHOT_RECTANGLE: case UT_CMD_VERIFY_SCREEN_SNAPSHOT_RECTANGLE: - switch(unittester.write_offs) { + switch (unittester.write_offs) { case 0: - unittester.read_snap_width = (uint16_t)val; + unittester.read_snap_width = (uint16_t) val; break; case 1: - unittester.read_snap_width |= ((uint16_t)val) << 8; + unittester.read_snap_width |= ((uint16_t) val) << 8; break; case 2: - unittester.read_snap_height = (uint16_t)val; + unittester.read_snap_height = (uint16_t) val; break; case 3: - unittester.read_snap_height |= ((uint16_t)val) << 8; + unittester.read_snap_height |= ((uint16_t) val) << 8; break; case 4: - unittester.read_snap_xoffs = (uint16_t)val; + unittester.read_snap_xoffs = (uint16_t) val; break; case 5: - unittester.read_snap_xoffs |= ((uint16_t)val) << 8; + unittester.read_snap_xoffs |= ((uint16_t) val) << 8; break; case 6: - unittester.read_snap_yoffs = (uint16_t)val; + unittester.read_snap_yoffs = (uint16_t) val; break; case 7: - unittester.read_snap_yoffs |= ((uint16_t)val) << 8; + unittester.read_snap_yoffs |= ((uint16_t) val) << 8; break; default: break; @@ -307,11 +307,11 @@ unittester_write(uint16_t port, uint8_t val, UNUSED(void *priv)) case UT_CMD_CAPTURE_SCREEN_SNAPSHOT: /* Recompute screen */ - unittester.snap_img_width = 0; - unittester.snap_img_height = 0; - unittester.snap_img_xoffs = 0; - unittester.snap_img_yoffs = 0; - unittester.snap_overscan_width = 0; + unittester.snap_img_width = 0; + unittester.snap_img_height = 0; + unittester.snap_img_xoffs = 0; + unittester.snap_img_yoffs = 0; + unittester.snap_overscan_width = 0; unittester.snap_overscan_height = 0; if (unittester.snap_monitor < 0x01 || (unittester.snap_monitor - 1) > MONITORS_NUM) { /* No monitor here - clear snapshot */ @@ -321,13 +321,13 @@ unittester_write(uint16_t port, uint8_t val, UNUSED(void *priv)) unittester.snap_monitor = 0x00; } else { /* Compute bounds for snapshot */ - const monitor_t *m = &monitors[unittester.snap_monitor - 1]; - unittester.snap_img_width = m->mon_xsize; - unittester.snap_img_height = m->mon_ysize; - unittester.snap_overscan_width = m->mon_xsize + m->mon_overscan_x; + const monitor_t *m = &monitors[unittester.snap_monitor - 1]; + unittester.snap_img_width = m->mon_xsize; + unittester.snap_img_height = m->mon_ysize; + unittester.snap_overscan_width = m->mon_xsize + m->mon_overscan_x; unittester.snap_overscan_height = m->mon_ysize + m->mon_overscan_y; - unittester.snap_img_xoffs = (m->mon_overscan_x >> 1); - unittester.snap_img_yoffs = (m->mon_overscan_y >> 1); + unittester.snap_img_xoffs = (m->mon_overscan_x >> 1); + unittester.snap_img_yoffs = (m->mon_overscan_y >> 1); /* Take snapshot */ for (size_t y = 0; y < unittester.snap_overscan_height; y++) { for (size_t x = 0; x < unittester.snap_overscan_width; x++) { @@ -344,38 +344,38 @@ unittester_write(uint16_t port, uint8_t val, UNUSED(void *priv)) unittester.snap_img_yoffs, unittester.snap_overscan_width, unittester.snap_overscan_height); - unittester.status = UT_STATUS_AWAITING_READ; + unittester.status = UT_STATUS_AWAITING_READ; unittester.read_len = 12; break; case UT_CMD_READ_SCREEN_SNAPSHOT_RECTANGLE: case UT_CMD_VERIFY_SCREEN_SNAPSHOT_RECTANGLE: /* Offset the X,Y offsets by the overscan offsets. */ - unittester.read_snap_xoffs += (int16_t)unittester.snap_img_xoffs; - unittester.read_snap_yoffs += (int16_t)unittester.snap_img_yoffs; + unittester.read_snap_xoffs += (int16_t) unittester.snap_img_xoffs; + unittester.read_snap_yoffs += (int16_t) unittester.snap_img_yoffs; /* NOTE: Width * Height * 4 can potentially exceed a 32-bit number. So, we use 64-bit numbers instead. In practice, this will only happen if someone decides to request e.g. a 65535 x 65535 image, of which most of the pixels will be out of range anyway. */ - unittester.read_len = ((uint64_t)unittester.read_snap_width) * ((uint64_t)unittester.read_snap_height) * 4; + unittester.read_len = ((uint64_t) unittester.read_snap_width) * ((uint64_t) unittester.read_snap_height) * 4; unittester.read_snap_crc = 0xFFFFFFFF; if (unittester.cmd_id == UT_CMD_VERIFY_SCREEN_SNAPSHOT_RECTANGLE) { /* Read everything and compute CRC */ uint32_t crc = 0xFFFFFFFF; for (uint64_t i = 0; i < unittester.read_len; i++) { - crc ^= 0xFF & (uint32_t)unittester_read_snap_rect_idx(i); + crc ^= 0xFF & (uint32_t) unittester_read_snap_rect_idx(i); /* Use some bit twiddling until we have a table-based fast CRC-32 implementation */ for (uint32_t j = 0; j < 8; j++) { - crc = (crc >> 1) ^ ((-(crc&0x1)) & 0xEDB88320); + crc = (crc >> 1) ^ ((-(crc & 0x1)) & 0xEDB88320); } } unittester.read_snap_crc = crc ^ 0xFFFFFFFF; /* Set actual read length for CRC result */ unittester.read_len = 4; - unittester.status = UT_STATUS_AWAITING_READ; + unittester.status = UT_STATUS_AWAITING_READ; } else { /* Do we have anything to read? */ @@ -408,11 +408,11 @@ unittester_read(uint16_t port, UNUSED(void *priv)) { uint8_t outval = 0xFF; - if (port == unittester.iobase_port+0x00) { + if (port == unittester.iobase_port + 0x00) { /* Status port */ unittester_log("[UT] R -- Status = %02X\n", unittester.status); return unittester.status; - } else if (port == unittester.iobase_port+0x01) { + } else if (port == unittester.iobase_port + 0x01) { /* Data port */ /* unittester_log("[UT] R -- Data\n"); */ @@ -422,42 +422,42 @@ unittester_read(uint16_t port, UNUSED(void *priv)) switch (unittester.cmd_id) { case UT_CMD_CAPTURE_SCREEN_SNAPSHOT: - switch(unittester.read_offs) { + switch (unittester.read_offs) { case 0: - outval = (uint8_t)(unittester.snap_img_width); + outval = (uint8_t) (unittester.snap_img_width); break; case 1: - outval = (uint8_t)(unittester.snap_img_width>>8); + outval = (uint8_t) (unittester.snap_img_width >> 8); break; case 2: - outval = (uint8_t)(unittester.snap_img_height); + outval = (uint8_t) (unittester.snap_img_height); break; case 3: - outval = (uint8_t)(unittester.snap_img_height>>8); + outval = (uint8_t) (unittester.snap_img_height >> 8); break; case 4: - outval = (uint8_t)(unittester.snap_overscan_width); + outval = (uint8_t) (unittester.snap_overscan_width); break; case 5: - outval = (uint8_t)(unittester.snap_overscan_width>>8); + outval = (uint8_t) (unittester.snap_overscan_width >> 8); break; case 6: - outval = (uint8_t)(unittester.snap_overscan_height); + outval = (uint8_t) (unittester.snap_overscan_height); break; case 7: - outval = (uint8_t)(unittester.snap_overscan_height>>8); + outval = (uint8_t) (unittester.snap_overscan_height >> 8); break; case 8: - outval = (uint8_t)(unittester.snap_img_xoffs); + outval = (uint8_t) (unittester.snap_img_xoffs); break; case 9: - outval = (uint8_t)(unittester.snap_img_xoffs>>8); + outval = (uint8_t) (unittester.snap_img_xoffs >> 8); break; case 10: - outval = (uint8_t)(unittester.snap_img_yoffs); + outval = (uint8_t) (unittester.snap_img_yoffs); break; case 11: - outval = (uint8_t)(unittester.snap_img_yoffs>>8); + outval = (uint8_t) (unittester.snap_img_yoffs >> 8); break; default: break; @@ -469,7 +469,7 @@ unittester_read(uint16_t port, UNUSED(void *priv)) break; case UT_CMD_VERIFY_SCREEN_SNAPSHOT_RECTANGLE: - outval = (uint8_t)(unittester.read_snap_crc >> (8*unittester.read_offs)); + outval = (uint8_t) (unittester.read_snap_crc >> (8 * unittester.read_offs)); break; /* This should not be reachable, but just in case... */ @@ -507,13 +507,13 @@ unittester_trigger_write(UNUSED(uint16_t port), uint8_t val, UNUSED(void *priv)) /* WAIT IOBASE 0: Set low byte of temporary IOBASE. */ case UT_FSM2_WAIT_IOBASE_0: - unittester.fsm2_new_iobase = ((uint16_t)val); - unittester.fsm2 = UT_FSM2_WAIT_IOBASE_1; + unittester.fsm2_new_iobase = ((uint16_t) val); + unittester.fsm2 = UT_FSM2_WAIT_IOBASE_1; break; /* WAIT IOBASE 0: Set high byte of temporary IOBASE and commit to the real IOBASE. */ case UT_FSM2_WAIT_IOBASE_1: - unittester.fsm2_new_iobase |= ((uint16_t)val)<<8; + unittester.fsm2_new_iobase |= ((uint16_t) val) << 8; unittester_log("[UT] Remapping IOBASE: %04X -> %04X\n", unittester.iobase_port, unittester.fsm2_new_iobase); @@ -575,12 +575,12 @@ unittester_init(UNUSED(const device_t *info)) if (unittester_screen_buffer == NULL) unittester_screen_buffer = create_bitmap(2048, 2048); - unittester = (struct unittester_state)unittester_defaults; + unittester = (struct unittester_state) unittester_defaults; io_sethandler(unittester.trigger_port, 1, NULL, NULL, NULL, unittester_trigger_write, NULL, NULL, NULL); unittester_log("[UT] 86Box Unit Tester initialised\n"); - return &unittester; /* Dummy non-NULL value */ + return &unittester; /* Dummy non-NULL value */ } static void diff --git a/src/include/86box/unittester.h b/src/include/86box/unittester.h index e83add67f..00abed3ff 100644 --- a/src/include/86box/unittester.h +++ b/src/include/86box/unittester.h @@ -35,4 +35,3 @@ extern const device_t unittester_device; #endif #endif /*UNITTESTER_H*/ - From 276e43428e11a869b53a2792df822a5e0191c7c0 Mon Sep 17 00:00:00 2001 From: GreaseMonkey Date: Mon, 8 Jan 2024 13:48:33 +1300 Subject: [PATCH 19/20] Allow one to enable/disable unit tester exit Memo to self: Hardware renderers often exit in a silent segfault. Look into this at some point. --- src/device/unittester.c | 17 ++++++++++++++--- src/qt/qt_settingsotherperipherals.cpp | 7 +++++++ src/qt/qt_settingsotherperipherals.hpp | 1 + src/qt/qt_settingsotherperipherals.ui | 19 ++++++++++++++++++- 4 files changed, 40 insertions(+), 4 deletions(-) diff --git a/src/device/unittester.c b/src/device/unittester.c index f8f7fa56e..0442d0695 100644 --- a/src/device/unittester.c +++ b/src/device/unittester.c @@ -114,10 +114,18 @@ static const struct unittester_state unittester_defaults = { .cmd_id = UT_CMD_NOOP, }; +static const device_config_t unittester_config[] = { + { .name = "exit_enabled", + .description = "Enable 0x04 \"Exit 86Box\" command", + .type = CONFIG_BINARY, + .default_int = 1, + .default_string = "" }, + { .type = CONFIG_END } +}; + /* Kept separate, as we will be reusing this object */ static bitmap_t *unittester_screen_buffer = NULL; -/* FIXME: This needs a config option! --GM */ static bool unittester_exit_enabled = true; #ifdef ENABLE_UNITTESTER_LOG @@ -572,10 +580,13 @@ unittester_trigger_write(UNUSED(uint16_t port), uint8_t val, UNUSED(void *priv)) static void * unittester_init(UNUSED(const device_t *info)) { + unittester = (struct unittester_state) unittester_defaults; + + unittester_exit_enabled = !!device_get_config_int("exit_enabled"); + if (unittester_screen_buffer == NULL) unittester_screen_buffer = create_bitmap(2048, 2048); - unittester = (struct unittester_state) unittester_defaults; io_sethandler(unittester.trigger_port, 1, NULL, NULL, NULL, unittester_trigger_write, NULL, NULL, NULL); unittester_log("[UT] 86Box Unit Tester initialised\n"); @@ -611,5 +622,5 @@ const device_t unittester_device = { { .available = NULL }, .speed_changed = NULL, .force_redraw = NULL, - .config = NULL + .config = unittester_config, }; diff --git a/src/qt/qt_settingsotherperipherals.cpp b/src/qt/qt_settingsotherperipherals.cpp index ad7a00bb4..beb484848 100644 --- a/src/qt/qt_settingsotherperipherals.cpp +++ b/src/qt/qt_settingsotherperipherals.cpp @@ -23,6 +23,7 @@ extern "C" { #include <86box/machine.h> #include <86box/isamem.h> #include <86box/isartc.h> +#include <86box/unittester.h> } #include "qt_deviceconfig.hpp" @@ -199,3 +200,9 @@ SettingsOtherPeripherals::on_pushButtonConfigureCard4_clicked() { DeviceConfig::ConfigureDevice(isamem_get_device(ui->comboBoxCard4->currentData().toInt()), 4, qobject_cast(Settings::settings)); } + +void +SettingsOtherPeripherals::on_pushButtonConfigureUT_clicked() +{ + DeviceConfig::ConfigureDevice(&unittester_device); +} diff --git a/src/qt/qt_settingsotherperipherals.hpp b/src/qt/qt_settingsotherperipherals.hpp index 97e47c90e..b521e7829 100644 --- a/src/qt/qt_settingsotherperipherals.hpp +++ b/src/qt/qt_settingsotherperipherals.hpp @@ -30,6 +30,7 @@ private slots: void on_comboBoxCard1_currentIndexChanged(int index); void on_pushButtonConfigureRTC_clicked(); void on_comboBoxRTC_currentIndexChanged(int index); + void on_pushButtonConfigureUT_clicked(); private: Ui::SettingsOtherPeripherals *ui; diff --git a/src/qt/qt_settingsotherperipherals.ui b/src/qt/qt_settingsotherperipherals.ui index 481d80ebe..af953a984 100644 --- a/src/qt/qt_settingsotherperipherals.ui +++ b/src/qt/qt_settingsotherperipherals.ui @@ -190,10 +190,27 @@ + + + + - 86Box unit tester + 86Box Unit Tester + + + + 0 + 0 + + + + + + + + Configure From 9448fc044f28e12c762ff6b50d2f89fc78ed361f Mon Sep 17 00:00:00 2001 From: GreaseMonkey Date: Mon, 8 Jan 2024 13:53:13 +1300 Subject: [PATCH 20/20] Enable and disable "configure" button for unit tester Now ready for review! --- src/qt/qt_settingsotherperipherals.cpp | 7 +++++++ src/qt/qt_settingsotherperipherals.hpp | 1 + 2 files changed, 8 insertions(+) diff --git a/src/qt/qt_settingsotherperipherals.cpp b/src/qt/qt_settingsotherperipherals.cpp index beb484848..f662b644c 100644 --- a/src/qt/qt_settingsotherperipherals.cpp +++ b/src/qt/qt_settingsotherperipherals.cpp @@ -47,6 +47,7 @@ SettingsOtherPeripherals::onCurrentMachineChanged(int machineId) ui->checkBoxPOSTCard->setChecked(postcard_enabled > 0 ? true : false); ui->checkBoxUnitTester->setChecked(unittester_enabled > 0 ? true : false); ui->checkBoxISABugger->setEnabled(machineHasIsa); + ui->pushButtonConfigureUT->setEnabled(unittester_enabled > 0); ui->comboBoxRTC->setEnabled(machineHasIsa); ui->pushButtonConfigureRTC->setEnabled(machineHasIsa); @@ -201,6 +202,12 @@ SettingsOtherPeripherals::on_pushButtonConfigureCard4_clicked() DeviceConfig::ConfigureDevice(isamem_get_device(ui->comboBoxCard4->currentData().toInt()), 4, qobject_cast(Settings::settings)); } +void +SettingsOtherPeripherals::on_checkBoxUnitTester_stateChanged(int arg1) +{ + ui->pushButtonConfigureUT->setEnabled(arg1 != 0); +} + void SettingsOtherPeripherals::on_pushButtonConfigureUT_clicked() { diff --git a/src/qt/qt_settingsotherperipherals.hpp b/src/qt/qt_settingsotherperipherals.hpp index b521e7829..feaa7a001 100644 --- a/src/qt/qt_settingsotherperipherals.hpp +++ b/src/qt/qt_settingsotherperipherals.hpp @@ -30,6 +30,7 @@ private slots: void on_comboBoxCard1_currentIndexChanged(int index); void on_pushButtonConfigureRTC_clicked(); void on_comboBoxRTC_currentIndexChanged(int index); + void on_checkBoxUnitTester_stateChanged(int arg1); void on_pushButtonConfigureUT_clicked(); private: