fix(timer): OCR values should be buffered #76

fix #76

2 files changed, 125 insertions, 11 deletions

diff --git a/src/peripherals/timer.spec.ts b/src/peripherals/timer.spec.ts
index 43768d2..11c0264 100644
--- a/src/peripherals/timer.spec.ts
+++ b/src/peripherals/timer.spec.ts
@@ -415,13 +415,13 @@ describe('timer', () => {
   describe('Phase-correct PWM mode', () => {
     it('should count up to TOP, down to 0, and then set TOV flag', () => {
       const { program, instructionCount } = asmProgram(`
+        LDI r16, 0x3   ; OCR0A = 0x3;   // <- TOP value
+        OUT 0x27, r16  
         ; Set waveform generation mode (WGM) to PWM, Phase Correct, top OCR0A
         LDI r16, 0x1   ; TCCR0A = 1 << WGM00;
         OUT 0x24, r16  
         LDI r16, 0x9   ; TCCR0B = (1 << WGM02) | (1 << CS00);
         OUT 0x25, r16  
-        LDI r16, 0x3   ; OCR0A = 0x3;
-        OUT 0x27, r16  
         LDI r16, 0x2   ; TCNT0 = 0x2;
         OUT 0x26, r16
 
@@ -448,13 +448,13 @@ describe('timer', () => {
 
     it('should clear OC0A when TCNT0=OCR0A and counting up', () => {
       const { program, lines, instructionCount } = asmProgram(`
+        LDI r16, 0xfe   ; OCR0A = 0xfe;   // <- TOP value
+        OUT 0x27, r16  
         ; Set waveform generation mode (WGM) to PWM, Phase Correct
-        LDI r16, 0x81   ; TCCR0A = (1 << COM0A1) || (1 << WGM01);
+        LDI r16, 0x81   ; TCCR0A = (1 << COM0A1) | (1 << WGM00);
         OUT 0x24, r16  
         LDI r16, 0x1   ; TCCR0B = (1 << CS00);
         OUT 0x25, r16  
-        LDI r16, 0xfe   ; OCR0A = 0xfe;
-        OUT 0x27, r16  
         LDI r16, 0xfd   ; TCNT0 = 0xfd;
         OUT 0x26, r16  
 
@@ -491,6 +491,46 @@ describe('timer', () => {
       expect(cpu.readData(TCNT0)).toEqual(0xfe);
       expect(gpioCallback).toHaveBeenCalledWith(6, PinOverrideMode.Set, 0x2b);
     });
+
+    it('should only update OCR0A when TCNT0=TOP in PWM Phase Correct mode (issue #76)', () => {
+      const { program, instructionCount } = asmProgram(`
+        LDI r16, 0x4   ; OCR0A = 0x4;
+        OUT 0x27, r16  
+        ; Set waveform generation mode (WGM) to PWM, Phase Correct
+        LDI r16, 0x01   ; TCCR0A = (1 << WGM00);
+        OUT 0x24, r16  
+        LDI r16, 0x09   ; TCCR0B = (1 << WGM02) | (1 << CS00);
+        OUT 0x25, r16  
+        LDI r16, 0x0   ; TCNT0 = 0x0;
+        OUT 0x26, r16  
+
+        LDI r16, 0x2    ; OCR0A = 0x2; // TCNT0 should read 0x0
+        OUT 0x27, r16   ; // TCNT0 should read 0x1
+        NOP             ; // TCNT0 should read 0x2
+        NOP             ; // TCNT0 should read 0x3
+        IN r17, 0x26    ; R17 = TCNT;  // TCNT0 should read 0x4 (that's old OCR0A / TOP)
+        NOP             ; // TCNT0 should read 0x3
+        NOP             ; // TCNT0 should read 0x2
+        NOP             ; // TCNT0 should read 0x1
+        NOP             ; // TCNT0 should read 0x0
+        NOP             ; // TCNT0 should read 0x1
+        NOP             ; // TCNT0 should read 0x2
+        IN r18, 0x26    ; R18 = TCNT; // TCNT0 should read 0x1
+      `);
+
+      const cpu = new CPU(program);
+      new AVRTimer(cpu, timer0Config);
+
+      // Listen to Port D's internal callback
+      const gpioCallback = jest.fn();
+      cpu.gpioTimerHooks[PORTD] = gpioCallback;
+
+      const runner = new TestProgramRunner(cpu);
+      runner.runInstructions(instructionCount);
+
+      expect(cpu.readData(R17)).toEqual(0x4);
+      expect(cpu.readData(R18)).toEqual(0x1);
+    });
   });
 
   describe('16 bit timers', () => {
@@ -638,5 +678,52 @@ describe('timer', () => {
       expect(cpu.readData(TCNT1)).toEqual(0x4a);
       expect(gpioCallback).toHaveBeenCalledWith(2, PinOverrideMode.Toggle, 0x25);
     });
+
+    it('should only update OCR0A when TCNT0=BOTTOM in PWM Phase/Frequency Correct mode (issue #76)', () => {
+      const { program, instructionCount } = asmProgram(`
+        LDI r16, 0x0    ; OCR1AH = 0x0;
+        STS 0x89, r16  
+        LDI r16, 0x4   ; OCR1AL = 0x4;
+        STS 0x88, r16  
+        ; Set waveform generation mode (WGM) to PWM Phase/Frequency Correct mode (9)
+        LDI r16, 0x01   ; TCCR1A = (1 << WGM10);
+        STS 0x80, r16  
+        LDI r16, 0x11   ; TCCR1B = (1 << WGM13) | (1 << CS00);
+        STS 0x81, r16  
+        LDI r16, 0x0    ; TCNT1H = 0x0;
+        STS 0x85, r16  
+        LDI r16, 0x0    ; TCNT1L = 0x0;
+        STS 0x84, r16  
+
+        LDI r16, 0x8   ; OCR1AL = 0x8; // TCNT1 should read 0x0
+        STS 0x88, r16  ; // TCNT1 should read 0x2 (going up)
+        LDS r17, 0x84  ; // TCNT1 should read 0x4 (going down)
+        LDS r18, 0x84  ; // TCNT1 should read 0x2 (going down)
+        NOP            ; // TCNT1 should read 0x0 (going up)
+        NOP            ; // TCNT1 should read 0x1 (going up)
+        NOP            ; // TCNT1 should read 0x2 (going up)
+        NOP            ; // TCNT1 should read 0x3 (going up)
+        NOP            ; // TCNT1 should read 0x4 (going up)
+        NOP            ; // TCNT1 should read 0x5 (going up)
+        LDS r19, 0x84  ; // TCNT1 should read 0x6 (going up)
+        NOP            ; // TCNT1 should read 0x8 (going up)
+        LDS r20, 0x84  ; // TCNT1 should read 0x7 (going up)
+      `);
+
+      const cpu = new CPU(program);
+      new AVRTimer(cpu, timer1Config);
+
+      // Listen to Port D's internal callback
+      const gpioCallback = jest.fn();
+      cpu.gpioTimerHooks[PORTD] = gpioCallback;
+
+      const runner = new TestProgramRunner(cpu);
+      runner.runInstructions(instructionCount);
+
+      expect(cpu.readData(R17)).toEqual(0x4);
+      expect(cpu.readData(R18)).toEqual(0x2);
+      expect(cpu.readData(R19)).toEqual(0x6);
+      expect(cpu.readData(R20)).toEqual(0x7);
+    });
   });
 });
diff --git a/src/peripherals/timer.ts b/src/peripherals/timer.ts
index 188832d..0f4d837 100644
--- a/src/peripherals/timer.ts
+++ b/src/peripherals/timer.ts
@@ -240,7 +240,10 @@ function compToOverride(comp: CompBitsValue) {
 export class AVRTimer {
   private lastCycle = 0;
   private ocrA: u16 = 0;
+  private nextOcrA: u16 = 0;
   private ocrB: u16 = 0;
+  private nextOcrB: u16 = 0;
+  private ocrUpdateMode = OCRUpdateMode.Immediate;
   private icr: u16 = 0; // only for 16-bit timers
   private timerMode: TimerMode;
   private topValue: TimerTopValue;
@@ -297,12 +300,16 @@ export class AVRTimer {
       this.timerUpdated();
     };
     this.cpu.writeHooks[config.OCRA] = (value: u8) => {
-      // TODO implement buffering when timer running in PWM mode
-      this.ocrA = (this.highByteTemp << 8) | value;
+      this.nextOcrA = (this.highByteTemp << 8) | value;
+      if (this.ocrUpdateMode === OCRUpdateMode.Immediate) {
+        this.ocrA = this.nextOcrA;
+      }
     };
     this.cpu.writeHooks[config.OCRB] = (value: u8) => {
-      // TODO implement buffering when timer running in PWM mode
-      this.ocrB = (this.highByteTemp << 8) | value;
+      this.nextOcrB = (this.highByteTemp << 8) | value;
+      if (this.ocrUpdateMode === OCRUpdateMode.Immediate) {
+        this.ocrB = this.nextOcrB;
+      }
     };
     this.cpu.writeHooks[config.ICR] = (value: u8) => {
       this.icr = (this.highByteTemp << 8) | value;
@@ -351,7 +358,9 @@ export class AVRTimer {
     this.divider = 0;
     this.lastCycle = 0;
     this.ocrA = 0;
+    this.nextOcrA = 0;
     this.ocrB = 0;
+    this.nextOcrB = 0;
     this.icr = 0;
     this.tcnt = 0;
     this.tcntNext = 0;
@@ -394,9 +403,10 @@ export class AVRTimer {
 
   private updateWGMConfig() {
     const wgmModes = this.config.bits === 16 ? wgmModes16Bit : wgmModes8Bit;
-    const [timerMode, topValue] = wgmModes[this.WGM];
+    const [timerMode, topValue, ocrUpdateMode] = wgmModes[this.WGM];
     this.timerMode = timerMode;
     this.topValue = topValue;
+    this.ocrUpdateMode = ocrUpdateMode;
   }
 
   count = (reschedule = true) => {
@@ -410,14 +420,23 @@ export class AVRTimer {
       const { timerMode } = this;
       const phasePwm =
         timerMode === TimerMode.PWMPhaseCorrect || timerMode === TimerMode.PWMPhaseFrequencyCorrect;
+      const limit = this.TOP + 1;
       const newVal = phasePwm
         ? this.phasePwmCount(val, counterDelta)
-        : (val + counterDelta) % (this.TOP + 1);
+        : (val + counterDelta) % limit;
+      const overflow = val + counterDelta >= limit;
       // A CPU write overrides (has priority over) all counter clear or count operations.
       if (!this.tcntUpdated) {
         this.tcnt = newVal;
         this.timerUpdated();
       }
+
+      if (!phasePwm && this.ocrUpdateMode == OCRUpdateMode.Bottom && overflow) {
+        // OCRUpdateMode.Top only occurs in Phase Correct modes, handled by phasePwmCount()
+        this.ocrA = this.nextOcrA;
+        this.ocrB = this.nextOcrB;
+      }
+
       if ((timerMode === TimerMode.Normal || timerMode === TimerMode.FastPWM) && val > newVal) {
         cpu.setInterruptFlag(this.OVF);
       }
@@ -447,12 +466,20 @@ export class AVRTimer {
         value++;
         if (value === this.TOP && !this.tcntUpdated) {
           this.countingUp = false;
+          if (this.ocrUpdateMode === OCRUpdateMode.Top) {
+            this.ocrA = this.nextOcrA;
+            this.ocrB = this.nextOcrB;
+          }
         }
       } else {
         value--;
         if (!value && !this.tcntUpdated) {
           this.countingUp = true;
           this.cpu.setInterruptFlag(this.OVF);
+          if (this.ocrUpdateMode === OCRUpdateMode.Bottom) {
+            this.ocrA = this.nextOcrA;
+            this.ocrB = this.nextOcrB;
+          }
         }
       }
       delta--;