wokwi / avr8js

Arduino (8-bit AVR) simulator, written in JavaScript and runs in the browser / Node.js
https://blog.wokwi.com/avr8js-simulate-arduino-in-javascript/
MIT License
461 stars 73 forks source link

timer1: mode 9 is not working anymore #111

Open fjangfaragesh opened 2 years ago

fjangfaragesh commented 2 years ago

In real (tested with Arduino Uno), the LED on pin 10 is blinking fast. In simulation nothing happens.

#ifndef F_CPU
#define F_CPU 16000000UL // 16 MHz clock speed
#endif

int main(void){
  DDRB |=  (1 << PORTB2); //Define OCR1B as Output (pin number arduino: 10)
  TCCR1A |= (1 << COM1A0) | (1 << COM1B1) | (1 << WGM10); // PB2: output, PWM mode 9
  TCCR1B |= (1 << CS10) | (1 << CS11) | (1 << WGM13);// prescaler=64, mode 9
  OCR1A = 4000;
  OCR1B = 1000;
  while(1) {
  }
}
urish commented 2 years ago

Thanks for reporting! Do you have any idea if it worked in previous version of the simulator?

urish commented 2 years ago

Initial analysis: it seems like the issue happens if OCR1A is still 0 when you configure the timer. A workaround would be to first set OCR1A / OCR1B, and then enable the timer:

int main(void){
  DDRB |=  (1 << PORTB2); //Define OCR1B as Output (pin number arduino: 10)
  OCR1A = 4000;
  OCR1B = 1000;
  TCCR1A |= (1 << COM1A0) | (1 << COM1B1) | (1 << WGM10); // PB2: output, PWM mode 9
  TCCR1B |= (1 << CS10) | (1 << CS11) | (1 << WGM13);// prescaler=64, mode 9
  while(1) {
  }
}

Here's a complete functional example: https://wokwi.com/arduino/projects/313183501205635648

To fix it, we need to figure out what to do if the timer starts and TOP (OCR1A) == 0. Should it generate compare match on every single timer cycle?

fjangfaragesh commented 2 years ago

Thanks for reporting! Do you have any idea if it worked in previous version of the simulator?

It worked in version 0.11.4.

The change of the OCR1A register is apparently only adopted after the change of direction at the old top:
(i think this behavior is wrong) grafik (blue: ORC1A, red:TCNT1, yellow:ORC1B, at tick 26,000,000 and 32,500,000 OCR1A is changed and TCNT1 is set to 0)

urish commented 2 years ago

Thanks!

Is the graph based on version 0.11.4 or on an actual chip?

fjangfaragesh commented 2 years ago

version 0.18.2

urish commented 2 years ago

The change of the OCR1A register is apparently only adopted after the change of direction at the old top:

As far as I can tell from the datasheet, this is the correct behavior, isn't it?

image

fjangfaragesh commented 2 years ago

I've tested it. That doesn't seem like the right behavior. I have written a program that reads TCNT1 and changes OCR1A. At the end, the results are transmitted via the serial interface.

int results[500];
int main(void){
  Serial.begin(9600);
  DDRB |=  (1 << PORTB2); //Define OCR1B as Output (pin number arduino: 10)
  OCR1A = 2000;
  OCR1B = 500;
  TCCR1A |= (1 << COM1A0) | (1 << COM1B1) | (1 << WGM10); // PB2: output, PWM mode 9
  TCCR1B |= (1 << CS10) | (1 << CS11) | (1 << WGM13);// prescaler=64, mode 9
  for (int i = 0; i < 250; i++) {
    results[i] = TCNT1;
    _delay_us(100);
  }
  OCR1A = 1000;
  OCR1B = 100;
  TCNT1 = 0;
  for (int i = 0; i < 250; i++) {
    results[i+250] = TCNT1;
    _delay_us(100);
  }
  for (int i = 0; i < 500; i++) Serial.println(results[i]);
  Serial.flush();
  while(1) {
  }
}

Results: (wimulation compared with real Arduino Uno) grafik

urish commented 2 years ago

Thank you for doing this comparison, it's really useful!

I need to search the data sheet to see if I can find where the exact behavior is defined, and adjust the simulation accordingly

urish commented 2 years ago

Ok, we have a fix. I verified it it the following test program:

int main(void) {
  uint8_t v0, v1, v2, v3;
  Serial.begin(9600);
  asm(R"(
    CLR r1          ; r1 is our zero register
    LDI r16, 0x0    ; OCR1AH = 0x0;
    STS 0x89, r1    
    LDI r16, 0x8    ; OCR1AL = 0x8;
    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  
    STS 0x85, r1    ; TCNT1H = 0x0;
    STS 0x84, r1    ; TCNT1L = 0x0;

    LDI r16, 0x5   ; OCR1AL = 0x5; // TCNT1 should read 0x0
    STS 0x88, r16  ; // TCNT1 should read 0x2 (going up)
    STS 0x84, r1   ; TCNT1L = 0x0;
    LDS %0, 0x84  ; // TCNT1 should read 0x1 (going up)
    LDS %1, 0x84  ; // TCNT1 should read 0x3 (going up)
    LDS %2, 0x84  ; // TCNT1 should read 0x5 (going down)
    LDS %3, 0x84  ; // TCNT1 should read 0x3 (going down)
  )" : "=r"(v0) , "=r"(v1), "=r"(v2), "=r"(v3) );
  Serial.println(v0);
  Serial.println(v1);
  Serial.println(v2);
  Serial.println(v3);
  Serial.flush();
  for (;;);
}

When running on a physical board, I get the following output:

1
3
5
3

I added a test case that validates this behavior (it fails with the previous version, passes with the fix).

Would love to get your feedback on the new version with the fix (0.18.5)

fjangfaragesh commented 2 years ago

The behavior of TCNT1 now seems to be correct.

urish commented 2 years ago

Lovely! Thanks again for reporting it and all your help in tracing the issue!

fjangfaragesh commented 2 years ago

But the timer output does not seem to be correct at the beginning.
The first pulse is missing.

#define DLY 135
#define N 128

int results[N*2];
int resultsPINB[N*2];
int main(void){
  Serial.begin(9600);
  DDRB |=  (1 << PORTB2); //Define OCR1B as Output (pin number arduino: 10)
  OCR1A = 2000;
  OCR1B = 1500;
  TCCR1A |= (1 << COM1A0) | (1 << COM1B1) | (1 << WGM10); // PB2: output, PWM mode 9
  TCCR1B |= (1 << CS10) | (1 << CS11) | (1 << WGM13);// prescaler=64, mode 9
  TCNT1 = 0;
  for (int i = 0; i < N; i++) {
    results[i] = TCNT1;
    resultsPINB[i] = PINB;
    _delay_us(DLY);
  }
  OCR1B = 100;
  for (int i = 0; i < N; i++) {
    results[i+N] = TCNT1;
    resultsPINB[i+N] = PINB;
    _delay_us(DLY);
  }
  // send results
  for (int i = 0; i < N*2; i++) {
    Serial.print(results[i]);
    Serial.print("\t");
    Serial.println(((resultsPINB[i] >> 2) & 1)*1000);
  }
  Serial.flush();
  while(1) {
  }
}

There is a difference in the behavior of the physical and the simulated:
image

urish commented 2 years ago

Thanks for providing detailed reproduction with graphs! I haven't had the time to look into it yet, but it's on my list.