From 36edfedfcd1e5f7f277dd20186692ce128fa9235 Mon Sep 17 00:00:00 2001 From: Walt Drummond Date: Wed, 3 Jan 2024 08:22:32 -0800 Subject: [PATCH] 6830 cleanup, 6502 class visibility cleanup --- apple1/mos6820.h | 122 +++++++++++++++++++++++-------------------- src/6502.cc | 12 +++-- src/6502.h | 11 +++- src/CMakeLists.txt | 4 +- tests/CMakeLists.txt | 56 +++++++++++--------- 5 files changed, 115 insertions(+), 90 deletions(-) diff --git a/apple1/mos6820.h b/apple1/mos6820.h index 60aed99..cd957f8 100644 --- a/apple1/mos6820.h +++ b/apple1/mos6820.h @@ -24,6 +24,8 @@ #include #include +#include "memory.h" + #if defined(__linux__) || defined(__MACH__) #include # ifdef __linux__ @@ -36,12 +38,8 @@ #elif defined(_WIN64) # include # include -#else -# error "No platform-specific implementation of getch()" #endif -#include "memory.h" - template class MOS6820 : public MemMappedDevice { public: @@ -141,34 +139,37 @@ class MOS6820 : public MemMappedDevice { private: // Offsets from MemMappedDevice::_baseAddress for these memory-mapped I/O ports. These are in order - // and cannot change. + // and cannot change as they, when added to _baseAddress, represent hardware addresses of where this + // chip is mapped in memory. static constexpr Word KEYBOARD = 0; static constexpr Word KEYBOARDCR = 1; static constexpr Word DISPLAY = 2; static constexpr Word DISPLAYCR = 3; // Apple 1 keycodes - static constexpr char NEWLINE = 0x0a; - static constexpr char CARRAGE_RETURN = 0x0d; - static constexpr char BELL = 0x0a; + static constexpr char NEWLINE = 0x0a; + static constexpr char CARRIAGE_RETURN = 0x0d; + static constexpr char BACKSPACE = '_'; + static constexpr char BELL = 0x0a; + static constexpr char CTRL_C = 0x03; #ifdef _WIN64 - static constexpr char CTRL_C = 0x03; - static constexpr char DEL = '\b'; - static constexpr char CTRL_BACKSPACE = 0x7f; // Quit emulator + static constexpr char DEL = '\b'; + static constexpr char CTRL_BACKSPACE = 0x7f; // Quit emulator #else - static constexpr char DEL = 0x7f; // Backspace on unix/linux - static constexpr char CTRL_BACKSPACE = 0x08; + static constexpr char DEL = 0x7f; // Backspace on unix/linux + static constexpr char CTRL_BACKSPACE = 0x08; #endif - static constexpr char CTRL_LBRACKET = 0x1b; // Clear screen - static constexpr char CTRL_BACKSLASH = 0x1c; // Reset/Jump to Wozmon - static constexpr char CTRL_RBRACKET = 0x1d; // Enter debugger + static constexpr char CTRL_LBRACKET = 0x1b; // Clear screen + static constexpr char CTRL_BACKSLASH = 0x1c; // Reset/Jump to Wozmon + static constexpr char CTRL_RBRACKET = 0x1d; // Enter debugger // Platform agnostic remapping of control keycodes. These are encoded in getch() and returned - // as a signed char, and should be non-printable ASCII characters. - static constexpr char CLEARSCR_CHAR = 0x00; - static constexpr char RESET_CHAR = 0x01; - static constexpr char DEBUGGER_CHAR = 0x02; // 0x03 is Control-C - static constexpr char EXIT_CHAR = 0x04; + // as a Cell. They must be non-printable ASCII characters on the Apple 1 (ie, extended + // ASCII codes). + static constexpr Cell CLEAR_SCREEN = 0xff; + static constexpr Cell RESET = 0xfe; + static constexpr Cell DEBUGGER = 0xfd; + static constexpr Cell EXIT = 0xfc; // Display bool _haveDspData = false; @@ -180,7 +181,7 @@ class MOS6820 : public MemMappedDevice { #if defined(__linux__) || defined(__MACH__) - bool getch(char &kbdCh) const { + bool getch(Cell &kbdCh) const { int byteswaiting; char ch; @@ -192,16 +193,16 @@ class MOS6820 : public MemMappedDevice { switch (ch) { case CTRL_BACKSPACE: - kbdCh = EXIT_CHAR; + kbdCh = EXIT; break; case CTRL_BACKSLASH: - kbdCh = RESET_CHAR; + kbdCh = RESET; break; case CTRL_RBRACKET: - kbdCh = DEBUGGER_CHAR; + kbdCh = DEBUGGER; break; case CTRL_LBRACKET: - kbdCh = CLEARSCR_CHAR; + kbdCh = CLEAR_SCREEN; break; default: kbdCh = ch; @@ -233,7 +234,7 @@ class MOS6820 : public MemMappedDevice { system("cls"); } - bool getch(char& kbdCh) const { + bool getch(Cell& kbdCh) const { if (_CtrlC_Pressed) { _CtrlC_Pressed = false; @@ -248,16 +249,16 @@ class MOS6820 : public MemMappedDevice { if (GetAsyncKeyState(VK_CONTROL) < 0) { // Control was held when key was pressed switch (c) { case CTRL_BACKSPACE: - kbdCh = EXIT_CHAR; + kbdCh = EXIT; return true; case CTRL_BACKSLASH: - kbdCh = RESET_CHAR; + kbdCh = RESET; return true; case CTRL_LBRACKET: - kbdCh = CLEARSCR_CHAR; + kbdCh = CLEAR_SCREEN; return true; case CTRL_RBRACKET: - kbdCh = DEBUGGER_CHAR; + kbdCh = DEBUGGER; return true; } } @@ -267,16 +268,7 @@ class MOS6820 : public MemMappedDevice { } #else - - // Clear the Apple 1 display - void clearScreen() const {} - - // Check and get a key from the keyboard, returning true and setting kbdCh if a key was ready - // for us. Map Control keys to platform agnostic values (see above) - bool getch(char& kbdCh) { - return false; - } - +# error "No platform specific clearScreen() or getch() functions defined" #endif Device::Lines displayHousekeeping() { @@ -285,13 +277,13 @@ class MOS6820 : public MemMappedDevice { auto c = _dspData & 0x7f; // clear hi bit switch (c) { - case CARRAGE_RETURN: // \r + case CARRIAGE_RETURN: fmt::print("\n"); break; - case '_': // Backspace + case BACKSPACE: fmt::print("\b"); break; - case BELL: // Bell + case BELL: fmt::print("\a"); break; default: @@ -304,7 +296,8 @@ class MOS6820 : public MemMappedDevice { } Device::Lines keyboardHousekeeping() { - char ch; + Cell ch; + bool clobberQueue = false; auto retval = Device::None; auto charsPending = getch(ch); @@ -314,32 +307,43 @@ class MOS6820 : public MemMappedDevice { // Handle control characters or map modern ascii to Apple 1 keycodes switch (ch) { - // Control characters; don't queue these. - case RESET_CHAR: + // Control values; don't queue these. + case RESET: return Device::Reset; - case DEBUGGER_CHAR: + case DEBUGGER: return Device::Debug; - case EXIT_CHAR: + case EXIT: return Device::Exit; - case CLEARSCR_CHAR: + case CLEAR_SCREEN: clearScreen(); return Device::None; + // Control-C is special, see the Applesoft basic comments below. + case CTRL_C: + clobberQueue = true; + break; + // Regular characters; do queue these case NEWLINE: - ch = CARRAGE_RETURN; + ch = CARRIAGE_RETURN; break; case DEL: - ch = '_'; + ch = BACKSPACE; break; } + // Apple 1 expects only upper case characters and that the high bit will be set ch = std::toupper(ch); - ch |= 0x80; + ch |= 0x80; + + if (clobberQueue) + while (!_charQueue.empty()) + _charQueue.pop(); + _charQueue.push(ch); return retval; @@ -386,9 +390,12 @@ class MOS6820 : public MemMappedDevice { // Applesoft Basic Lite does a blind, unchecked read on the keyboard port // looking for a ^C. If it sees one, it then does a read on the keyboard // control register followed by a read of the keyboard port, expecting to - // get the same ^C. This logic forces a keyboard control register read - // before removing the character from the queue, thus preventing an - // infinite loop. + // get the same ^C. This logic forces a keyboard control register read + // before removing the character from the queue, thus preventing an + // infinite loop. This also means that if the user hits ^C but the data element + // at the head of _charQueue is something else, the ^C will never be seen and processed. + // We fix this by clobbering the _charQueue before queuing the ^C. We do this in + // keyboardHousekeeping() above. if (_kbdCRRead) { _charQueue.pop(); _kbdCRRead = false; @@ -404,6 +411,5 @@ class MOS6820 : public MemMappedDevice { }; #ifdef _WIN64 -template -bool MOS6820::_CtrlC_Pressed = false; +template bool MOS6820::_CtrlC_Pressed = false; #endif diff --git a/src/6502.cc b/src/6502.cc index 4d66357..c5d3276 100644 --- a/src/6502.cc +++ b/src/6502.cc @@ -37,16 +37,18 @@ void CPU::setInterruptVector(Word address) { } void CPU::exitReset() { + PC = readWord(RESET_VECTOR); + SP = INITIAL_SP; +#ifdef TEST_BUILD + // If we're here via TestReset() clobber the PC and SP with test values if (_testReset) { SP = testResetSP; PC = testResetPC; - } else { - PC = readWord(RESET_VECTOR); - SP = INITIAL_SP; } - _testReset = false; +#endif + _debuggingEnabled = false; debug_alwaysShowPS = false; @@ -62,6 +64,7 @@ void CPU::exitReset() { // allows tests to set specific starting Program Counter and // Stack Pointer values, and arranges for the next call to execute...() // to exit the CPU from reset. +#ifdef TEST_BUILD void CPU::TestReset(Word initialPC, Byte initialSP) { _inReset = false; _pendingReset = true; @@ -69,6 +72,7 @@ void CPU::TestReset(Word initialPC, Byte initialSP) { testResetPC = initialPC; testResetSP = initialSP; } +#endif // 'Asserts' the Reset line if not asserted, de-asserts the Reset line // if asserted. diff --git a/src/6502.h b/src/6502.h index 4618a5d..1d8fd8e 100644 --- a/src/6502.h +++ b/src/6502.h @@ -66,7 +66,6 @@ class CPU { // CPU Setup & reset CPU(cMemory &); void Reset(); - void TestReset(Word initialPC = RESET_VECTOR, Byte initialSP = INITIAL_SP); void setResetVector(Word); void setPendingReset() { if (!_debuggingEnabled) @@ -97,7 +96,11 @@ class CPU { // Debugger bool executeDebug(); +#ifdef TEST_BUILD public: +#else +private: +#endif ////////// // Used by tests @@ -127,7 +130,9 @@ class CPU { Byte PS = 0; struct ProcessorStatusBits Flags; }; - +#ifdef TEST_BUILD + void TestReset(Word initialPC = RESET_VECTOR, Byte initialSP = INITIAL_SP); +#endif void executeOneInstructionWithCycleCount(Cycles_t &, Cycles_t &); bool executeOneInstruction(); void traceOneInstruction(Cycles_t &, Cycles_t &); @@ -182,9 +187,11 @@ class CPU { std::atomic_bool _pendingIRQ = false; std::atomic_bool _pendingNMI = false; bool _hitException = false; +#ifdef TEST_BUILD Word testResetPC = 0; Byte testResetSP = INITIAL_SP; bool _testReset = false; +#endif Address_t _haltAddress = 0; bool _haltAddressSet = false; diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index f5559bd..afa4cf4 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -16,7 +16,7 @@ cmake_minimum_required(VERSION 3.7) project( lib6502 ) -set (SOURCES +set (SOURCES "6502.h" "clock.h" "memory.h" @@ -30,3 +30,5 @@ include_directories(AFTER ${fmt_INCLUDE_DIRS}) add_library( 6502 ${SOURCES} ) target_compile_definitions(6502 PRIVATE FMT_HEADER_ONLY) +add_library( 6502-test ${SOURCES}) +target_compile_definitions(6502-test PRIVATE FMT_HEADER_ONLY TEST_BUILD) \ No newline at end of file diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 393107a..f0fefe0 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -16,6 +16,7 @@ cmake_minimum_required(VERSION 3.0...3.7) project( 6502_tests ) +## Google Test configuration # Download and unpack googletest at configure time configure_file(CMakeLists.txt.in googletest-download/CMakeLists.txt) execute_process(COMMAND ${CMAKE_COMMAND} -G "${CMAKE_GENERATOR}" . @@ -42,7 +43,33 @@ add_subdirectory(${CMAKE_CURRENT_BINARY_DIR}/googletest-src EXCLUDE_FROM_ALL) include_directories(${CMAKE_CURRENT_BINARY_DIR}/googletest-src/googletest/include) include_directories(${CMAKE_CURRENT_BINARY_DIR}/googletest-src/googletest/include/gtest) +## +########## +# Memory tests +set (MEMORY_TEST_SOURCES + "memory_tests.cc" +) + +source_group("src" FILE ${MEMORY_TEST_SOURCES}) +add_executable(memory_tests ${MEMORY_TEST_SOURCES}) +add_dependencies(memory_tests gtest_main) +target_link_libraries(memory_tests gtest_main) +if(MSVC) + target_link_libraries(memory_tests fmt::fmt-header-only) +elseif(CMAKE_CXX_COMPILER_ID STREQUAL "Clang") + message(STATUS "Adding ${LIBCXXABI_LIBRARY} to link_libraries for memory_test") + target_link_libraries(memory_tests ${LIBCXXABI_LIBRARY}) +else() + target_compile_definitions(memory_tests PRIVATE FMT_HEADER_ONLY) + target_link_libraries(memory_tests readline) +endif() +add_custom_target(run_memory_tests + DEPENDS memory_tests + COMMAND memory_tests) + +########## +# 6502 tests # source for the test executable set (6502_TEST_SOURCES "6502_tests_adc.cc" @@ -73,46 +100,25 @@ set (6502_TEST_SOURCES "6502_tests_xxx_functional_test_suite.cc" ) -set (MEMORY_TEST_SOURCES - "memory_tests.cc" -) - -source_group("src" FILE ${MEMORY_TEST_SOURCES}) -add_executable(memory_tests ${MEMORY_TEST_SOURCES}) -add_dependencies(memory_tests gtest_main) -target_link_libraries(memory_tests gtest_main) -if(MSVC) - target_link_libraries(memory_tests fmt::fmt-header-only) -elseif(CMAKE_CXX_COMPILER_ID STREQUAL "Clang") - message(STATUS "Adding ${LIBCXXABI_LIBRARY} to link_libraries for memory_test") - target_link_libraries(memory_tests ${LIBCXXABI_LIBRARY}) -else() - target_compile_definitions(memory_tests PRIVATE FMT_HEADER_ONLY) - target_link_libraries(memory_tests readline) -endif() -add_custom_target(run_memory_tests - DEPENDS memory_tests - COMMAND memory_tests) - source_group("src" FILES ${6502_TEST_SOURCES}) add_executable( 6502_tests ${6502_TEST_SOURCES}) -add_dependencies(6502_tests 6502) +add_dependencies(6502_tests 6502-test) add_dependencies(6502_tests gtest_main) -target_link_libraries(6502_tests gtest_main 6502) +target_link_libraries(6502_tests gtest_main 6502-test) if(MSVC) target_link_libraries(6502_tests fmt::fmt-header-only) else() target_compile_definitions(6502_tests PRIVATE FMT_HEADER_ONLY) target_link_libraries(6502_tests readline) endif() - +target_compile_definitions(6502_tests PRIVATE TEST_BUILD) add_custom_target(run_6502_tests DEPENDS 6502_tests COMMAND 6502_tests) +# Phony targets add_custom_target(tests DEPENDS memory_tests 6502_tests) - add_custom_target(runtests DEPENDS run_memory_tests run_6502_tests)