fix: address latching timing and cleanup debug code

.gitignore

@@ -1 +1,3 @@
 /target
+.idea/
+.kilocode/

src/components/common/intel_400x.rs

@@ -204,6 +204,10 @@ pub trait Intel400xAddressHandling {
 
     /// Handle address nibble latching during address phase
     /// Returns: (high_nibble, low_nibble, address_ready)
+    ///
+    /// After the full address is latched (both nibbles received), the nibbles
+    /// are cleared to prepare for the next address cycle. The full address
+    /// should be assembled and stored before calling this function returns.
     fn handle_address_latching(
         &self,
         nibble: u8,
@@ -217,10 +221,16 @@ pub trait Intel400xAddressHandling {
             // First cycle: latch high nibble (bits 7-4)
             *address_high_nibble = Some(nibble);
         } else if address_low_nibble.is_none() {
-            // Second cycle: latch low nibble (bits 3-0) and transition to latency wait
+            // Second cycle: latch low nibble (bits 3-0)
             *address_low_nibble = Some(nibble);
+            // Mark address as ready and record latch time
             *full_address_ready = true;
             *address_latch_time = Some(Instant::now());
+            // Clear nibbles after full address is latched to prepare for next cycle
+            // The full address should be assembled from the nibbles before this returns
+            // (e.g., by the caller reading the nibble values before they're cleared)
+            *address_high_nibble = None;
+            *address_low_nibble = None;
         }
     }
 
@@ -909,16 +919,17 @@ impl Intel400xStateMachine {
     }
 
     /// Handle address nibble latching
+    /// Returns true only when BOTH nibbles have been latched (address complete)
     pub fn handle_address_nibble(&mut self, nibble: u8) -> bool {
         if self.address_high_nibble.is_none() {
             // First cycle: latch high nibble (bits 7-4)
             self.address_high_nibble = Some(nibble);
-            true
+            false // Address not complete yet - still need low nibble
         } else if self.address_low_nibble.is_none() {
             // Second cycle: latch low nibble (bits 3-0) and transition to latency wait
             self.address_low_nibble = Some(nibble);
             self.address_latch_time = Some(Instant::now());
-            true // Address complete
+            true // Address complete - both nibbles latched
         } else {
             // Address already latched, ignore
             false
@@ -1381,13 +1392,15 @@ mod tests {
             Duration::from_nanos(500),
         );
 
-        // First nibble should latch high nibble
-        assert!(sm.handle_address_nibble(0xA));
+        // First nibble should latch high nibble but NOT complete the address
+        // Returns false because we still need the low nibble
+        assert!(!sm.handle_address_nibble(0xA));
         assert_eq!(sm.get_address_high_nibble(), Some(0xA));
         assert_eq!(sm.get_address_low_nibble(), None);
         assert!(!sm.is_full_address_ready());
 
         // Second nibble should latch low nibble and complete address
+        // Returns true because address is now complete
         assert!(sm.handle_address_nibble(0x5));
         assert_eq!(sm.get_address_high_nibble(), Some(0xA));
         assert_eq!(sm.get_address_low_nibble(), Some(0x5));

src/components/cpu/intel_4004.rs

@@ -125,6 +125,7 @@ pub struct Intel4004 {
     instruction_phase: InstructionPhase, // Current instruction execution phase
     current_instruction: u8,             // Currently executing instruction
     address_latch: u8,                   // Latched address for memory operations
+    #[allow(dead_code)] // Reserved for future memory operations implementation
     data_latch: u8,                      // Latched data for memory operations
     clock_speed: f64,                    // Target clock speed in Hz
     rom_port: u8,                        // Currently selected ROM port (0-15)
@@ -536,14 +537,16 @@ impl Intel4004 {
             }
 
             MemoryState::WaitLatency => {
-                // Address sent, waiting for ROM to respond
+                // Address sent (A2 complete), now in A3 cycle
                 // Tri-state the bus so ROM can drive it
                 self.tri_state_data_bus();
                 self.handle_latency_wait();
             }
 
             MemoryState::DriveData => {
-                // Data phase - ROM should be driving the bus, CPU reads
+                // A3 cycle complete - ROM is driving the bus with data
+                // CPU must tri-state to avoid bus contention
+                // The actual data read happens in update() via read_data_bus()
                 self.tri_state_data_bus();
             }
         }
@@ -618,24 +621,23 @@ impl Intel4004 {
         }
     }
 
-    /// Transition to data driving state
+    /// Transition to data reading state (ROM drives data, CPU reads)
     fn start_data_driving(&mut self) {
         self.memory_state = MemoryState::DriveData;
-    }
-
-    /// Handle data driving during DriveData state
-    fn handle_data_driving(&mut self) {
-        let (sync, cm, _) = self.read_control_pins();
-
-        // Memory read: SYNC=1, CM=1, valid address
-        if sync && cm && self.full_address_ready {
-            // All conditions met: drive data on bus
-            let data = self.data_latch;
-            self.write_data_bus(data);
-        } else {
-            // Conditions not met, tri-state
+        // Note: Despite the name "DriveData", for instruction fetch the CPU does NOT drive.
+        // The ROM drives the data bus during A3 cycle. The CPU reads from the bus.
+        // We ensure the bus is tri-stated so ROM can drive it.
         self.tri_state_data_bus();
     }
+
+    /// Handle data phase during DriveData state
+    /// For instruction fetch: CPU tri-states bus, ROM drives data, CPU reads
+    /// The actual reading happens in update() when memory_state == DriveData
+    fn handle_data_driving(&mut self) {
+        // During instruction fetch (A3 cycle), the ROM drives the data bus.
+        // The CPU must tri-state its outputs to avoid bus contention.
+        // The actual data read happens in the update() method.
+        self.tri_state_data_bus();
     }
 
     /// Reset memory state machine to idle

src/components/memory/intel_4001.rs

@@ -387,15 +387,15 @@ impl Intel4001 {
         data
     }
 
-    /// Handle Φ1 rising edge - Address and control phase
-    /// Hardware: Φ1 high = CPU drives bus with address/control information
+    /// Handle Φ1 rising edge - Start of address phase
+    /// Hardware: Φ1 high = CPU starts driving bus with address/control information
     /// Memory operations start when SYNC goes high during Φ1 rising edge
-    /// Focus: Address latching, reset, I/O operations
+    /// Focus: Start memory operations, I/O operations
     ///
     /// Intel 4004 Address Timing (CPU drives address during PHI1):
     /// - A1 cycle: CPU drives PC high nibble on PHI1 rising
     /// - A2 cycle: CPU drives PC low nibble on PHI1 rising
-    /// - ROM latches each nibble when it sees them on the bus during AddressPhase
+    /// - ROM latches each nibble on PHI1 FALLING edge (after data is stable)
     fn handle_phi1_rising(&mut self) {
         // Handle system reset first (highest priority)
         self.handle_reset("RESET");
@@ -415,17 +415,8 @@ impl Intel4001 {
             if self.state_machine.get_state() == Intel400xUnifiedState::Idle {
                 self.process_chip_select();
             }
-
-            // CRITICAL: Latch address nibbles during PHI1 when CPU is driving them
-            // The CPU drives address nibbles on PHI1 rising, so we must latch here
-            if self.state_machine.get_state() == Intel400xUnifiedState::AddressPhase {
-                let nibble = self.read_data_bus() & 0x0F;
-
-                if self.state_machine.handle_address_nibble(nibble) {
-                    // Address is complete (both nibbles latched), transition to latency wait
-                    self.process_address_complete();
-                }
-            }
+            // NOTE: Address nibble latching moved to PHI1 falling edge
+            // This gives the data bus time to stabilize after CPU drives it on PHI1 rising
         }
 
         // Handle I/O operations (higher priority than memory operations)
@@ -436,19 +427,42 @@ impl Intel4001 {
         }
     }
 
-    /// Handle Φ1 falling edge - End of address phase
-    /// Hardware: Φ1 low = End of CPU address driving phase
+    /// Handle Φ1 falling edge - Latch address nibbles
+    /// Hardware: Φ1 low = CPU has finished driving address, data is stable
+    /// ROM latches address nibbles on PHI1 falling edge to ensure data is stable
     fn handle_phi1_falling(&mut self) {
-        // Currently no specific operations needed on Φ1 falling
-        // Address latching happens on Φ1 rising, data driving on Φ2 rising
+        // Only latch if we're in AddressPhase
+        // Note: We do NOT check chip_select here because:
+        // 1. Chip select was already verified when we entered AddressPhase
+        // 2. CM pin may be lowered between A1 and A2 cycles (this is valid)
+        // 3. Once address phase starts, we must complete both nibbles
+        // 4. The io_select (CI) pin distinguishes I/O vs memory operations
+        if self.state_machine.get_state() == Intel400xUnifiedState::AddressPhase {
+            // Latch address nibble on PHI1 falling edge (data is now stable)
+            let nibble = self.read_data_bus() & 0x0F;
+
+            if self.state_machine.handle_address_nibble(nibble) {
+                // Address is complete (both nibbles latched), transition to latency wait
+                self.process_address_complete();
+            }
+        }
     }
 
     /// Handle Φ2 rising edge - Data phase
     /// Hardware: Φ2 high = Peripherals drive bus with data
     /// Focus: Data driving operations
     fn handle_phi2_rising(&mut self) {
-        // Execute current state logic
-        self.execute_state_machine();
+        // Check for latency elapsed and transition to DriveData if needed
+        if self.state_machine.get_state() == Intel400xUnifiedState::WaitLatency {
+            if self.state_machine.is_latency_elapsed() {
+                self.process_latency_elapsed();
+            }
+        }
+
+        // Drive data if in DriveData state
+        if self.state_machine.get_state() == Intel400xUnifiedState::DriveData {
+            self.handle_data_driving();
+        }
     }
 
     /// Handle Φ2 falling edge - End of data phase
@@ -803,6 +817,9 @@ impl Intel4001 {
     }
 
     /// Helper function to execute unified state machine logic
+    /// Note: This method is reserved for future use when state machine execution
+    /// needs to be triggered explicitly rather than through clock edge handlers
+    #[allow(dead_code)]
     fn execute_state_machine(&mut self) {
         // Execute the current state logic using the unified state machine
         let current_state = self.state_machine.get_state();
@@ -840,40 +857,36 @@ impl Component for Intel4001 {
         self.prev_phi1 = phi1;
         self.prev_phi2 = phi2;
 
-        // Collect all events to process
-        let mut events = Vec::new();
+        // Check for reset first (highest priority, independent of clock edges)
+        let reset_active = self.read_reset_pin();
+        if reset_active {
+            self.perform_reset();
+            // Skip further processing if reset is active
+            return;
+        }
 
+        // Process clock events through component-specific handlers
+        // These handlers contain the actual logic for chip select, address latching, etc.
         if phi1_rising {
-            events.push(Intel400xEvent::ClockPhi1Rising);
+            self.handle_phi1_rising();
         }
 
         if phi1_falling {
-            events.push(Intel400xEvent::ClockPhi1Falling);
+            self.handle_phi1_falling();
         }
 
         if phi2_rising {
-            events.push(Intel400xEvent::ClockPhi2Rising);
+            self.handle_phi2_rising();
         }
 
         if phi2_falling {
-            events.push(Intel400xEvent::ClockPhi2Falling);
+            self.handle_phi2_falling();
         }
 
-        // Process all events
-        for event in events {
-            // Use process_event_direct method to avoid borrowing conflicts
-            let new_state = process_event_direct(&mut self.state_machine, event);
-
-            // Handle state entry/exit manually
-            if new_state != self.state_machine.get_state() {
-                self.on_state_exit(self.state_machine.get_state());
-                self.state_machine.current_state = new_state;
-                self.on_state_entry(new_state);
-            }
-        }
-
-        // Execute current state logic
-        self.execute_state_machine();
+        // Note: execute_state_machine() is NOT called here because:
+        // 1. Address latching is handled directly in handle_phi1_rising()
+        // 2. Data driving is handled directly in handle_phi2_rising()
+        // 3. Calling it would cause double-latching of address nibbles
     }
 
     /// Run component in time-slice mode (manual control)
@@ -1198,7 +1211,8 @@ mod tests {
         println!("DEBUG: After update - prev_phi1: {:?}", rom.prev_phi1);
 
         // Should have started memory operation and transitioned to AddressPhase
-        assert_eq!(rom.state_machine.current_state, Intel400xUnifiedState::Idle);
+        // because SYNC=1, CM=1, CI=0 triggers chip select on PHI1 rising edge
+        assert_eq!(rom.state_machine.current_state, Intel400xUnifiedState::AddressPhase);
         println!(
             "Memory state after Φ1 rising: {:?}",
             rom.state_machine.current_state

src/components/memory/intel_4002.rs

@@ -486,10 +486,6 @@ impl Intel4002 {
     /// Hardware: Φ2 high = Peripherals drive bus with data
     /// Focus: Data driving operations
     fn handle_phi2_rising(&mut self) {
-        println!(
-            "DEBUG: [RAM] Φ2 rising edge detected, current state: {:?}",
-            self.state_machine.get_state()
-        );
         // Execute current state logic
         self.execute_state_machine();
     }
@@ -499,10 +495,6 @@ impl Intel4002 {
     /// Data is held on bus during Φ2 high, tri-stated when Φ2 falls
     /// Focus: Clean up and return to idle state
     fn handle_phi2_falling(&mut self) {
-        println!(
-            "DEBUG: [RAM] Φ2 falling edge detected, current state: {:?}",
-            self.state_machine.get_state()
-        );
         match self.state_machine.get_state() {
             Intel400xUnifiedState::ReadData
             | Intel400xUnifiedState::WriteData
@@ -532,10 +524,6 @@ impl Intel4002 {
     /// Handle address nibble latching during address phase
     /// Hardware: CPU provides 8-bit address as two 4-bit nibbles
     fn handle_address_latching(&mut self) {
-        println!(
-            "DEBUG: handle_address_latching called, current state: {:?}",
-            self.state_machine.current_state
-        );
         // Use common address latching functionality
         let nibble = self.read_data_bus() & 0x0F; // Only 4 bits are valid for address nibbles
 
@@ -551,21 +539,12 @@ impl Intel4002 {
         if self.state_machine.is_full_address_ready() {
             let address = self.state_machine.get_full_address().unwrap_or(0) as u8;
             if let Some(data) = self.read_ram(address) {
-                println!(
-                    "DEBUG: [RAM] Reading 0x{:02X} from address 0x{:02X} onto bus",
-                    data, address
-                );
                 self.write_data_bus(data);
                 self.last_address = address as u16;
             } else {
-                println!(
-                    "DEBUG: [RAM] Invalid address 0x{:02X}, tri-stating bus",
-                    address
-                );
                 self.tri_state_data_bus();
             }
         } else {
-            println!("DEBUG: [RAM] Address not ready, tri-stating bus");
             self.tri_state_data_bus();
         }
     }
@@ -576,27 +555,15 @@ impl Intel4002 {
         if self.state_machine.is_full_address_ready() {
             let address = self.state_machine.get_full_address().unwrap_or(0) as u8;
             let data = self.read_data_bus();
-            println!(
-                "DEBUG: [RAM] Writing 0x{:02X} from bus to address 0x{:02X}",
-                data, address
-            );
-            if let Err(e) = self.write_ram(address, data) {
-                println!("DEBUG: [RAM] Write error: {}", e);
-            } else {
+            if self.write_ram(address, data).is_ok() {
                 self.last_address = address as u16;
             }
-        } else {
-            println!("DEBUG: [RAM] Address not ready, ignoring write");
         }
     }
 
     /// Handle status character output operation
     /// Hardware: RAM drives status character pins with current status
     fn handle_status_output(&mut self) {
-        println!(
-            "DEBUG: [RAM] Outputting status character 0x{:02X}",
-            self.status_character
-        );
         self.drive_status_character();
     }
 
@@ -680,9 +647,7 @@ impl Component for Intel4002 {
     /// Main update cycle - handles clock edge detection and operation dispatch
     /// Hardware: Responds to Φ1 and Φ2 clock edges from CPU
     fn update(&mut self) {
-        println!("DEBUG: Update called");
         if !self.is_running() {
-            println!("DEBUG: Component not running, returning");
             return;
         }
         // Handle both rising and falling edges for proper two-phase operation
@@ -700,7 +665,6 @@ impl Component for Intel4002 {
         let mut events = Vec::new();
 
         if phi1_rising {
-            println!("DEBUG: phi1_rising detected");
             events.push(Intel400xEvent::ClockPhi1Rising);
         }