Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
drf5n opened this issue Apr 10, 2022 · 9 comments
Open

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

drf5n opened this issue Apr 10, 2022 · 9 comments

Comments

@drf5n
Copy link

drf5n commented Apr 10, 2022

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 drf5n changed the title Timer1 WGMmode=8 issue? Timer1 WGMmode=0b0100 / CTC(TOP=OCR1A) issue? Apr 10, 2022
@drf5n
Copy link
Author

drf5n commented Apr 11, 2022

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:

image

image

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

@urish
Copy link
Contributor

urish commented Apr 11, 2022

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
Copy link
Author

drf5n commented Apr 11, 2022

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
Copy link
Contributor

urish commented Apr 12, 2022

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
Copy link
Author

drf5n commented Apr 12, 2022

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
Copy link
Author

drf5n commented Mar 31, 2023

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

image

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

image

image

Fixing this should make grbl run on Wokwi:

https://wokwi.com/projects/328163210406396500

versus debugging in

https://wokwi.com/projects/328612457250554450

@drf5n
Copy link
Author

drf5n commented Apr 1, 2023

Maybe in line 705 here?

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?

@drf5n
Copy link
Author

drf5n commented Apr 7, 2023

Here's an example with an oscilloscope:

https://wokwi.com/projects/361352450358452225

image

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 

@drf5n drf5n changed the title Timer1 WGMmode=0b0100 / CTC(TOP=OCR1A) issue? Timer1 WGMmode=0b0100 / CTC(TOP=OCR1A=0) issue? Apr 7, 2023
@urish
Copy link
Contributor

urish commented Apr 16, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants