Timer1 WGMmode=0b0100 / CTC(TOP=OCR1A=0) issue?

[drf5n]

I was trying to make the Grbl CNC software (https://github.com/gnea/grbl-Mega) work with some steppers in a couple sketches like https://wokwi.com/projects/328566930816369236 or the Uno like https://wokwi.com/projects/328163210406396500

In the Description/README.md file on https://wokwi.com/projects/328566930816369236 I wrote this:

This code does not appear to run the stepper timer correctly on wokwi as compared to a silicon Arduino Mega

Test: Run this simulation, and send this:

G1 X1 F1
??
$?
This initates a move to X=1mm, but as can be seen by the ?? output:

ok
<Run|MPos:0.000,0.000,0.000|FS:1,0>
error:8
The position doesn't update, and it remains stuck in "Run" mode, blocking and erroring commands like $?.

On a silicon Mega, this code produces:

ok
<Idle|MPos:1.000,0.000,0.000|FS:0,0>
[HLP:$$ $# $G $I $N $x=val $Nx=line $J=line $SLP $C $X $H ~ ! ? ctrl-x]
showing that it did move to the target position and then Idle. The $? command will function in Idle mode

The stepper scheduler is using Timer1 WGM=8 CTC mode with OCR1A in stepper.c

I'm not sure what the problem is. -- drf 2022-04-10

The grbl released hex file https://github.com/gnea/grbl/releases/download/v1.1h.20190825/grbl_v1.1h.20190825.hex from https://github.com/gnea/grbl/releases on a simulated Uno exhibits the same blocked behavior.

I think the TIMER1_COMPA_vect in stepper.h may not be triggering, but I'm not sure how to diagnose it. Do you have a test setup for timer modes that I could play with to see if can duplicate the problem with a simpler demo?

[drf5n]

Here's a demo of using Timer1's TIMER1_COMPA_vect and TIMER1_OVF_vect in WGM=4 CTC(TOP=OCR1A) mode:

https://wokwi.com/projects/328669448533705299

It seems to be working fine in the test sketch:

...but grbl, grbl-Mega, and grbl-Mega-5X all still appear hang in Wokwi but not in silicon.

[urish]

Thanks for reporting @drf5n!

It's probably best to recompile grbl inside Wokwi (so we have the ELF file), then debug it using GDB to figure out where exactly it hangs.

The AVR timers are super complex, so we'll need to narrow this down to a minimal test case, similar to what you did in #119

[drf5n]

it looks like I can compile it inside Wokwi, and "F1 / Start Web GDB Session (debug build)" and set a breakpoint liketbreak st_wake_up... but it looks very complex.

Is there a Wokwi simulation interface running attached to the GDB? I'm not sure how to trigger the error without interacting with the serial interface.

The grbl event loop code continues to run as expected, but the background stepping/motion planning software that uses the AVR timers and interrupts might be where things are going wrong.

I'm not familiar enough with the code to trim it down into a minimal case.

[urish]

When you start a GDB session, the Wokwi interface should continue to function in the browser tab where you started the simulation from. You can see an example here:

https://youtu.be/YUUHw-bU9yk?t=434 (Starting at 7:14)

[drf5n]

You are right.

It is really amazing to be able to singlestep the simulated Arduino and type p TCNT1 to see the counter tick up.

[drf5n]

I poked at this a little more, and I think the difference is on CTC mode when OCR1A = 0.

Per Pg 100 of https://ww1.microchip.com/downloads/en/DeviceDoc/Atmel-7810-Automotive-Microcontrollers-ATmega328P_Datasheet.pdf

And in a Wokwi: (Note that pin 9/OC1A isn't toggling as expected when OCR1A = 0, and is toggling when OCR1A=1)

https://wokwi.com/projects/360749451904483329

Fixing this should make grbl run on Wokwi:

https://wokwi.com/projects/328163210406396500

versus debugging in

https://wokwi.com/projects/328612457250554450

[drf5n]

Maybe in line 705 here?

avr8js/src/peripherals/timer.ts

Lines 702 to 726 in 67638ca

	private timerUpdated(value: number, prevValue: number) {
	const { ocrA, ocrB, ocrC, hasOCRC } = this;
	const overflow = prevValue > value;
	if (((prevValue < ocrA || overflow) && value >= ocrA) || (prevValue < ocrA && overflow)) {
	this.cpu.setInterruptFlag(this.OCFA);
	if (this.compA) {
	this.updateCompPin(this.compA, 'A');
	}
	}
	if (((prevValue < ocrB || overflow) && value >= ocrB) || (prevValue < ocrB && overflow)) {
	this.cpu.setInterruptFlag(this.OCFB);
	if (this.compB) {
	this.updateCompPin(this.compB, 'B');
	}
	}
	if (
	hasOCRC &&
	(((prevValue < ocrC || overflow) && value >= ocrC) || (prevValue < ocrC && overflow))
	) {
	this.cpu.setInterruptFlag(this.OCFC);
	if (this.compC) {
	this.updateCompPin(this.compC, 'C');
	}
	}
	}

   if (((prevValue < ocrA || overflow ) && value >= ocrA) || (prevValue < ocrA && overflow)) { 

If OCR1A == 0 in CTC mode, then TCNT1 won't not be zero*, value == prevValue, so overflow == False, prevValue will never be < ocrA, and all the if conditions are false.

Maybe it needs another case like this completely untested:

   if (((prevValue < ocrA || overflow  ) && value >= ocrA) || (prevValue < ocrA && overflow) || (ocrA == 0 && value == 0) { 

Or will timerUpdated() even be called on a 0->0 transition?

• -- in normal cases. I've seen a use of setting top = 0, and then setting TCNTx > 0 to schedule a one-shot: https://wp.josh.com/2015/03/12/avr-timer-based-one-shot-explained/

[drf5n]

Here's an example with an oscilloscope:

https://wokwi.com/projects/361352450358452225

Every 10 seconds, the top trace should toggle between a 3906.25Hz and 7812.50Hz, 50% PWM, but the output goes flat for the 7812.50Hz case because when TOP=0, the simulation won't toggle the output pin.

This output shows the timer1 registers, the expected toggle frequency on OC1A (pin9) and som random samples of TCNT1 during operation:

TCCR1A=0b1010000;
TCCR1B=0b1101;
TIMSK1=0b0;
OCR1A=0;
F_OC1A = fclk/(2*N*(1+OCR1A))=7812.50Hz per page 101 of ATmega48A-PA-88A-PA-168A-PA-328-P-DS-DS40002061A.pdf
https://ww1.microchip.com/downloads/en/DeviceDoc/Atmel-7810-Automotive-Microcontrollers-ATmega328P_Datasheet.pdf
0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 
10000004us Setting OCR1A = 0x1
TCCR1A=0b1010000;
TCCR1B=0b1101;
TIMSK1=0b0;
OCR1A=1;
F_OC1A = fclk/(2*N*(1+OCR1A))=3906.25Hz per page 101 of ATmega48A-PA-88A-PA-168A-PA-328-P-DS-DS40002061A.pdf
https://ww1.microchip.com/downloads/en/DeviceDoc/Atmel-7810-Automotive-Microcontrollers-ATmega328P_Datasheet.pdf
0 0 1 0 1 0 1 1 1 0 1 0 1 0 0 1 0 0 0 0 1 1 1 1 0 0 0 0 1 0 0 1 0 1 0 1 0 0 0 1 1 1 0 0 1 

[urish]

Thanks for the example! I'll have to find a time to sit down and look into it more thoroughly. From a quick look, your suggested fix might work - but we'd also need to create a minimal test case, similar to the ones in this file so we can be sure we fixed the issue and also that it won't come back when we make additional changes to the code in the future.

[drf5n]

So something like:

avr8js/src/peripherals/timer.spec.ts

Lines 306 to 321 in 320007d

	it('should reset the counter in CTC mode if it equals to OCRA', () => {
	const cpu = new CPU(new Uint16Array(0x1000));
	new AVRTimer(cpu, timer0Config);
	cpu.writeData(TCNT0, 0x10);
	cpu.writeData(OCR0A, 0x11);
	cpu.writeData(TCCR0A, WGM01); // WGM: CTC
	cpu.writeData(TCCR0B, CS00); // Set prescaler to 1
	cpu.cycles = 1;
	cpu.tick();
	cpu.cycles = 3;
	cpu.tick();
	const tcnt = cpu.readData(TCNT0);
	expect(tcnt).toEqual(0);
	expect(cpu.pc).toEqual(0);
	expect(cpu.cycles).toEqual(3);
	});

but reworked for timer1 and OCR1A=0

  it('should reset the counter in CTC mode if it equals to OCR1A equals 0 (issue #122)', () => {
    const cpu = new CPU(new Uint16Array(0x1000));
    new AVRTimer(cpu, timer0Config);
    cpu.writeData(TCNT1, 0x0);
    cpu.writeData(OCR1A, 0x0);
    cpu.writeData(TCCR1A, WGM12); // WGM: CTC TOP=OCR1A
    cpu.writeData(TCCR1B, CS10); // Set prescaler to 1
    cpu.cycles = 1;
    cpu.tick();
    cpu.cycles = 3;
    cpu.tick();
    const tcnt = cpu.readData(TCNT1);
    expect(tcnt).toEqual(0);
    expect(cpu.pc).toEqual(0);
    expect(cpu.cycles).toEqual(3);
  });

...but tuned properly. I just hacked the writes and am unclear on the cycles and ticks.

It also: 'should set the OCF1A flag...'

expect(cpu.data[TIFR1] & (OCF1A ).toEqual(OCF1A);

[bygreencn]

@urish @drf5n Any update for this issue?