From d06bc3fd87cb1e4e036269cdbaf17ad54becc502 Mon Sep 17 00:00:00 2001 From: Trevor Merritt Date: Tue, 8 Oct 2024 10:30:20 -0400 Subject: [PATCH] stack now traps overflow and underflow fixes missing whitespace adds multi_sprite test output ---- Timendus' Chip-8 Test Suite Runs! switch to hard test --- coverage/tarpaulin-report.html | 671 ++++++++++++++++++++++++ emma/src/bin/support/emmagui_support.rs | 19 +- emma/src/chip8/computer.rs | 13 + emma/src/chip8/instructions.rs | 73 ++- emma/src/chip8/keypad.rs | 32 +- emma/src/chip8/registers.rs | 28 +- emma/src/chip8/stack.rs | 57 +- emma/src/chip8/video.rs | 83 +-- resources/test/test_multi_sprite.asc | 32 ++ resources/test/test_video_zero.asc | 64 +-- 10 files changed, 981 insertions(+), 91 deletions(-) create mode 100644 coverage/tarpaulin-report.html create mode 100644 resources/test/test_multi_sprite.asc diff --git a/coverage/tarpaulin-report.html b/coverage/tarpaulin-report.html new file mode 100644 index 0000000..f07883e --- /dev/null +++ b/coverage/tarpaulin-report.html @@ -0,0 +1,671 @@ + + + + + + + +
+ + + + + + \ No newline at end of file diff --git a/emma/src/bin/support/emmagui_support.rs b/emma/src/bin/support/emmagui_support.rs index d274f94..fae7bf6 100644 --- a/emma/src/bin/support/emmagui_support.rs +++ b/emma/src/bin/support/emmagui_support.rs @@ -27,8 +27,8 @@ impl EmmaGui { let origin = ui.cursor_screen_pos(); let fg = ui.get_window_draw_list(); ui.text("This is the video display."); - for current_row in 0..31 { - for current_column in 0..63 { + for current_row in 0..=31 { + for current_column in 0..=63 { let x_offset = origin[0] as i32 + (current_column * cell_width); let y_offset = origin[1] as i32 + (current_row * cell_height); let current_origin = [x_offset as f32, y_offset as f32]; @@ -58,14 +58,25 @@ impl EmmaGui { let mut buffer = Vec::new(); println!("PREPARING TO LOAD 1-chip8-logo.ch8"); - let mut input_file = File::open(Path::new("./1-chip8-logo.ch8")).expect("put 1-chip8-logo.ch8 in this directory"); - // let mut input_file = File::open(Path::new("./coraxhard.ch8")).expect("put 1-chip8-logo.ch8 in this directory"); + // let mut input_file = File::open(Path::new("./1-chip8-logo.ch8")).expect("put 1-chip8-logo.ch8 in this directory"); + let mut input_file = File::open(Path::new("./coraxhard.ch8")).expect("put 1-chip8-logo.ch8 in this directory"); input_file.read_to_end(&mut buffer).expect("unable to read file"); system_to_control.load_bytes_to_memory(0x200, buffer.into()); } if ui.button("Reset") { *system_to_control = Chip8Computer::new(); } + + if ui.button("Dump Video Memory") { + println!("{}", system_to_control.dump_video_to_string()); + } + if ui.button("Dump Keypad State") { + println!("{}", system_to_control.dump_keypad_to_string()); + } + if ui.button("Dump Registers") { + println!("{}", system_to_control.dump_registers_to_string()); + } + }); } diff --git a/emma/src/chip8/computer.rs b/emma/src/chip8/computer.rs index 8cad874..8b9d11c 100644 --- a/emma/src/chip8/computer.rs +++ b/emma/src/chip8/computer.rs @@ -41,6 +41,19 @@ impl Default for Chip8Computer { } impl Chip8Computer { + + pub fn dump_keypad_to_string(&self) -> String { + self.keypad.format_as_string() + } + + pub fn dump_registers_to_string(&self) -> String { + self.registers.format_as_string() + } + + pub fn dump_video_to_string(&self) -> String { + self.video_memory.format_as_string() + } + pub fn new_with_program(new_program: Box>) -> Self { let mut working = Chip8Computer::new(); for i in 0..new_program.len() { diff --git a/emma/src/chip8/instructions.rs b/emma/src/chip8/instructions.rs index c6482ee..7a2f379 100644 --- a/emma/src/chip8/instructions.rs +++ b/emma/src/chip8/instructions.rs @@ -436,7 +436,9 @@ impl Chip8CpuInstructions { } // 0x00EE Return from Subroutine Chip8CpuInstructions::RET => { - debug!("RET"); + let return_address = input.stack.pop(); + debug!("Returning from subroutine to {return_address:03x}"); + input.registers.poke_pc(return_address); } // 0x1nnn Jump to Address Chip8CpuInstructions::JpAddr(new_address) => { @@ -446,6 +448,9 @@ impl Chip8CpuInstructions { // 0x2nnn Call Subroutine Chip8CpuInstructions::CallAddr(new_address) => { debug!("CALL ADDR 0x{new_address:3x}"); + let return_address = input.registers.peek_pc(); + input.registers.poke_pc(*new_address as u16); + input.stack.push(&return_address); } // 0x3xkk Skip next instruction if Vx = kk. Chip8CpuInstructions::SeVxByte(vx_register, byte) => { @@ -479,8 +484,10 @@ impl Chip8CpuInstructions { } // 0x7xkk Set Vx = Vx + kk Chip8CpuInstructions::AddVxByte(vx_register, byte) => { - input.registers.poke(*vx_register as u8, (input.registers.peek(*vx_register as u8) + *byte as u8)); - debug!("AddVxByte [0x{vx_register:1x}] [0x{byte:2x}]"); + let x_value: u16 = input.registers.peek(*vx_register as u8) as u16; + let value_to_poke = (x_value + *byte) as u8; + input.registers.poke(*vx_register as u8, value_to_poke); + debug!("AddVxByte [0x{vx_register:1x}] [0x{byte:02x}] [0x{value_to_poke:02x}"); } // 0x8xy0 Set value of Vy in Vx Chip8CpuInstructions::LdVxVy(x, y) => { @@ -516,7 +523,17 @@ impl Chip8CpuInstructions { // Set Vx = Vx - Vy, set VF = NOT borrow. // // If Vx > Vy, then VF is set to 1, otherwise 0. Then Vy is subtracted from Vx, and the results stored in Vx. - input.registers.poke(*x as u8, input.registers.peek(*x as u8) - input.registers.peek(*y as u8)); + let mut x_value: u16 = input.registers.peek(*x as u8) as u16; + let y_value = input.registers.peek(*y as u8); + // do we borrow? + if y_value >= x_value as u8 { + x_value += 256; + input.registers.poke(0xf, 1); + } else { + input.registers.poke(0xf, 0); + } + let result = (x_value - y_value as u16) as u8; + input.registers.poke(*x as u8, result); } Chip8CpuInstructions::ShrVxVy(x, _) => { // 8xy6 - SHR Vx {, Vy} @@ -595,7 +612,7 @@ impl Chip8CpuInstructions { let new_value: u8 = random(); input.registers.poke(*x as u8, (new_value & *byte as u8)) } - Chip8CpuInstructions::DrawVxVyNibble(x, y, n) => { + Chip8CpuInstructions::DrawVxVyNibble(y,x, n) => { // Display n-byte sprite starting at memory location I at (Vx, Vy), set VF = collision. // The interpreter reads n bytes from memory, starting at the address stored in I. @@ -773,6 +790,7 @@ impl Chip8CpuInstructions { mod test { use dimensioned::typenum::assert_type; use ratatui::crossterm::execute; + use crate::chip8::cpu_states::Chip8CpuStates; use crate::chip8::system_memory::{CHIP8FONT_0, CHIP8FONT_1, CHIP8FONT_2, CHIP8FONT_9}; use super::*; @@ -1395,15 +1413,6 @@ mod test { assert_eq!(x.registers.peek_pc(), 0x206); } - #[test] - fn ldvxk_test() { - println!("THIS TEST DOES NOT REALLY DO ANYTHING AS IT DEPENDS ON THE LOCAL IMPLEMENTATION"); - println!("OF HOW TO DELAY AND GET KEYBOARD INPUTS. THIS EXPECTS THAT THE SYSTEM GOES INTO"); - println!("ITS HOLDING STATE AND ONCE THE KEY IS PRESSED THE KEYBOARD 'PRESS_KEY' IS COMPLETED"); - println!("THE PC IS ALREADY ADVANCED AT THIS POINT SO THE SKPVX OR WHATEVER CAN HANDLE IT"); - let mut x = Chip8Computer::new(); - } - #[test] fn draw_nibble_vx_vy_n_test() { let mut x = Chip8Computer::new(); @@ -1443,4 +1452,40 @@ mod test { } } } + + #[test] + fn sub_test() { + // 2nnn + // Call a subroutine at 2nnn + let mut x = Chip8Computer::new(); + Chip8CpuInstructions::CallAddr(0x124).execute(&mut x); + assert_eq!(x.registers.peek_pc(), 0x124); + assert_eq!(x.stack.depth(), 1); + Chip8CpuInstructions::CallAddr(0x564).execute(&mut x); + assert_eq!(x.registers.peek_pc(), 0x564); + assert_eq!(x.stack.depth(), 2); + } + + #[test] + fn ret_test() { + // Return from a subroutine. + let mut x = Chip8Computer::new(); + x.stack.push(&0x132); + x.stack.push(&0xabc); + Chip8CpuInstructions::RET.execute(&mut x); + assert_eq!(x.registers.peek_pc(), 0xabc); + assert_eq!(x.stack.depth(), 1); + Chip8CpuInstructions::RET.execute(&mut x); + assert_eq!(x.registers.peek_pc(), 0x132); + assert_eq!(x.stack.depth(), 0); + } + + + #[test] + fn ldvxk_test() { + let mut x = Chip8Computer::new(); + Chip8CpuInstructions::LdVxByte(0x1, 0x1).execute(&mut x); + Chip8CpuInstructions::LdVxK(0x1).execute(&mut x); + assert!(matches!(x.state, WaitingForKey)); + } } diff --git a/emma/src/chip8/keypad.rs b/emma/src/chip8/keypad.rs index 1962058..9ae148d 100644 --- a/emma/src/chip8/keypad.rs +++ b/emma/src/chip8/keypad.rs @@ -1,14 +1,38 @@ use imgui::Key; -use ratatui::{ - style::{Style, Stylize}, - widgets::Widget, -}; + #[derive(Clone, Copy)] pub struct Keypad { keys: [bool; 0x10], } +impl Keypad { + pub fn format_as_string(&self) -> String { + let mut return_value = String::new(); + // draw a 4x4 grid showing the keys with * filling the cells that are depressed + let keys = [ + [0x1, 0x2, 0x3, 0xc],[0x4, 0x5, 0x6, 0xd],[0x7, 0x8, 0x9, 0xe],[0xa, 0x0, 0xb, 0xf] + ]; + + for row in keys.iter() { + for (index, key) in row.iter().enumerate() { + let is_lit = if self.keys[*key] { "*".to_string() } else { char::from_digit(*key as u32, 16).unwrap_or(' ').to_string() }; + match index { + 3 => { + // last in col + return_value += format!("|{}|\n", is_lit).as_str(); + } + _=> { + return_value += format!("|{}", is_lit).as_str(); + } + } + } + } + + return_value + } +} + impl Default for Keypad { fn default() -> Self { Keypad { diff --git a/emma/src/chip8/registers.rs b/emma/src/chip8/registers.rs index 611b9ad..a9e5a2a 100644 --- a/emma/src/chip8/registers.rs +++ b/emma/src/chip8/registers.rs @@ -7,7 +7,7 @@ pub struct Chip8Registers { registers: [u8; 16], i_register: u16, pc: u16, - sp: u16 + sp: u16, } impl Chip8Registers { @@ -22,7 +22,7 @@ impl Default for Chip8Registers { registers: [0x00; 16], i_register: 0x00, pc: 0x200, - sp: 0x100 + sp: 0x100, } } } @@ -53,6 +53,29 @@ impl Chip8Registers { pub fn set_pc(&mut self, new_pc: u16) { self.pc = new_pc } + + pub fn format_as_string(&self) -> String { + format!("Vx: 0x{:02x} 0x{:02x} 0x{:02x} 0x{:02x} 0x{:02x} 0x{:02x} 0x{:02x} 0x{:02x}\n 0x{:02x} 0x{:02x} 0x{:02x} 0x{:02x} 0x{:02x} 0x{:02x} 0x{:02x} 0x{:02x}\nI: 0x{:04x}\tPC: 0x{:04x}", + self.registers[0x0], + self.registers[0x1], + self.registers[0x2], + self.registers[0x3], + self.registers[0x4], + self.registers[0x5], + self.registers[0x6], + self.registers[0x7], + self.registers[0x8], + self.registers[0x9], + self.registers[0xa], + self.registers[0xb], + self.registers[0xc], + self.registers[0xd], + self.registers[0xe], + self.registers[0xf], + self.i_register, + self.pc + ) + } } #[cfg(test)] @@ -62,7 +85,6 @@ mod test { #[test] fn smoke() { assert!(true) } - #[test] fn register_rw_test() { let mut x = Chip8Registers::default(); diff --git a/emma/src/chip8/stack.rs b/emma/src/chip8/stack.rs index 9794da0..1be7eaf 100644 --- a/emma/src/chip8/stack.rs +++ b/emma/src/chip8/stack.rs @@ -13,10 +13,16 @@ impl Default for Chip8Stack { impl Chip8Stack { pub fn push(&mut self, new_value: &u16) { + if self.depth() == 16 { + panic!("Stack Overflow"); + } self.items.push(*new_value ); } pub fn pop(&mut self) -> u16 { - self.items.pop().unwrap_or(0) + if self.items.is_empty() { + panic!("Stack Underflow"); + } + self.items.pop().unwrap() } pub fn depth(&self) -> u16 { self.items.len() as u16 @@ -31,6 +37,7 @@ impl Chip8Stack { #[cfg(test)] mod test { + use rand::random; use super::*; #[test] @@ -46,4 +53,52 @@ mod test { x.pop(); assert_eq!(x.depth(), 1); } + + #[test] + #[should_panic] + fn stack_overflow_test() { + let mut x = Chip8Stack::new(); + for i in 0..17 { + x.push(&i); + } + } + + #[test] + #[should_panic] + fn stack_underflow_test() { + let mut x = Chip8Stack::new(); + x.pop(); + } + + #[test] + fn lots_of_subs() { + let mut x = Chip8Stack::new(); + let stack_contents = [0x123, 0x321, 0xabc, 0xdef, + 0xbad, 0xbef, 0xfed, 0xcab, + 0xbed, 0xcad, 0xfeb, 0xcab, + 0xfff, 0x000, 0x001]; + for i in stack_contents { + x.push(&i); + } + + assert_eq!(x.depth(), 15); + + // up to 50 random loops + let num_loops: u8 = random::() % 50; + for i in 0..num_loops { + let start_count = x.depth(); + let num_pop = random::() % x.depth() as u8; + for current_pop in 0..num_pop { + x.pop(); + } + + let post_pop_count = x.depth(); + assert_eq!(post_pop_count as u8, start_count as u8 - num_pop); + for current_push in 0..num_pop { + x.push(&stack_contents[(current_push + post_pop_count as u8) as usize]); + } + assert_eq!(x.depth(), 15); + } + + } } \ No newline at end of file diff --git a/emma/src/chip8/video.rs b/emma/src/chip8/video.rs index a64d864..f045baf 100644 --- a/emma/src/chip8/video.rs +++ b/emma/src/chip8/video.rs @@ -1,7 +1,4 @@ -use log::debug; -use ratatui::prelude::*; -use ratatui::{layout::Rect, style::Style, widgets::Widget}; -use crate::chip8::computer::Chip8Computer; +use log::{debug, trace}; use crate::constants::CHIP8_VIDEO_MEMORY; #[derive(Clone, Copy)] @@ -33,14 +30,13 @@ impl Chip8Video { } pub fn poke(&mut self, address: u16, new_value: bool) -> Self { - let col = address % 64; - let row = address / 32; - let offset = col * 64 + row; - println!("OFFSET: {offset} - POKING {new_value} to {address} at {col} x {row}"); + trace!("OFFSET: {address} - POKING {new_value}"); let old_value = self.memory[address as usize]; if old_value != new_value { - println!("**VIDEO** TOGGLING {new_value} TO {address} / {col}x{row}"); + trace!("**VIDEO** TOGGLING"); self.has_frame_changed = true; + } else { + trace!("NOT TOGGLING"); } self.memory[address as usize] = new_value; self.to_owned() @@ -70,19 +66,18 @@ impl Chip8Video { pub fn format_as_string(self) -> String { let mut output = String::new(); for row in 0..32 { - let row_offset = row * 64; for column in 0..64 { - let data_position = row_offset + column; - output += if self.memory[data_position] { - println!("DOT AT OFFSET:{row_offset} {row}x{column} {data_position:03x}/{data_position}"); - "*" + let data_offset = row * 64 + column; + debug!("Rendering {data_offset} with value {}", self.memory[data_offset]); + if self.memory[data_offset] { + output += "*" } else { - " " - }; + output += " " + } } output += "\n"; } - // println!("{}", output); + output } } @@ -97,7 +92,8 @@ impl Default for Chip8Video { mod test { use std::fs::File; use std::io::Read; - use crate::chip8::system_memory::CHIP8FONT_0; + use crate::chip8::system_memory::{CHIP8FONT_0, CHIP8FONT_1, CHIP8FONT_2, CHIP8FONT_3, CHIP8FONT_4, CHIP8FONT_5, CHIP8FONT_6, CHIP8FONT_7}; + use crate::constants::{CHIP8_VIDEO_HEIGHT, CHIP8_VIDEO_WIDTH}; use super::*; #[test] @@ -339,10 +335,13 @@ mod test { #[test] fn write_checkboard() { let mut v = Chip8Video::default(); - for i in 0..(CHIP8_VIDEO_MEMORY / 8) { - v.poke_byte((i * 8) as u16, 0b10101010); + for current_row in 0..CHIP8_VIDEO_WIDTH { + for current_col in 0..(CHIP8_VIDEO_HEIGHT / 8){ + let offset = current_row * CHIP8_VIDEO_HEIGHT + current_col; + println!("CHECKBOARD OFFSET = {offset}"); + v.poke(offset as u16, offset % 2 == 0); + } } - println!("{}", v.format_as_string()); println!("fsck is a cool tool"); } @@ -352,18 +351,36 @@ mod test { let mut x = Chip8Video::default(); x.poke_byte(0x00, CHIP8FONT_0[0]); - x.poke_byte(0x08, CHIP8FONT_0[1]); - x.poke_byte(0x10, CHIP8FONT_0[2]); - x.poke_byte(0x18, CHIP8FONT_0[3]); - x.poke_byte(0x20, CHIP8FONT_0[4]); + x.poke_byte(0x40, CHIP8FONT_0[1]); + x.poke_byte(0x80, CHIP8FONT_0[2]); + x.poke_byte(0xC0, CHIP8FONT_0[3]); + x.poke_byte(0x100, CHIP8FONT_0[4]); - println!("SHOULD HAVE 0 AT ORIGIN"); - println!("{}", x.format_as_string()); - - let mut buf: Vec = vec![]; - // let read_file_size = File::open("../resources/test/test_video_zero.asc").unwrap().read_to_end(&mut buf).unwrap(); - - // println!("READ {read_file_size} bytes."); - // assert_eq!(String::from_utf8_lossy(&buf), x.format_as_string()); + assert_eq!(std::fs::read_to_string("../resources/test/test_video_zero.asc") + .unwrap(), + x.format_as_string()); } + + #[test] + fn multi_sprite_test() { + let mut x = Chip8Video::default(); + // draw a row of digits 01234567 + + let start_offset = 0x00; + let per_row_skip = 0x40; + let to_draw = [CHIP8FONT_0, CHIP8FONT_1, CHIP8FONT_2, CHIP8FONT_3, CHIP8FONT_4, CHIP8FONT_5, CHIP8FONT_6, CHIP8FONT_7]; + for (index, sprite) in to_draw.iter().enumerate() { + let data_base_offset = index * 0x8; + println!("STARTING {index} at 0x{data_base_offset:04x} ({data_base_offset})"); + x.poke_byte(data_base_offset as u16, sprite[0]); + x.poke_byte((data_base_offset + 0x40) as u16, sprite[1]); + x.poke_byte((data_base_offset + 0x80) as u16, sprite[2]); + x.poke_byte((data_base_offset + 0xC0) as u16, sprite[3]); + x.poke_byte((data_base_offset + 0x100) as u16, sprite[4]); + } + + + assert_eq!(std::fs::read_to_string("../resources/test/test_multi_sprite.asc") + .unwrap(), + x.format_as_string()); } } diff --git a/resources/test/test_multi_sprite.asc b/resources/test/test_multi_sprite.asc new file mode 100644 index 0000000..4d96dfb --- /dev/null +++ b/resources/test/test_multi_sprite.asc @@ -0,0 +1,32 @@ +**** * **** **** * * **** **** **** +* * ** * * * * * * * +* * * **** **** **** **** **** * +* * * * * * * * * * +**** *** **** **** * **** **** * + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/resources/test/test_video_zero.asc b/resources/test/test_video_zero.asc index 9776375..116a865 100644 --- a/resources/test/test_video_zero.asc +++ b/resources/test/test_video_zero.asc @@ -1,32 +1,32 @@ -***** -* * -* * -* * -***** - - - - - - - - - - - - - - - - - - - - - - - - - - - +**** +* * +* * +* * +**** + + + + + + + + + + + + + + + + + + + + + + + + + + +