-
Notifications
You must be signed in to change notification settings - Fork 80
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
Comments
Here's a demo of using Timer1's 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. |
it looks like I can compile it inside Wokwi, and "F1 / Start Web GDB Session (debug build)" and set a breakpoint like 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. |
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) |
You are right. It is really amazing to be able to singlestep the simulated Arduino and type |
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 |
Maybe in line 705 here? avr8js/src/peripherals/timer.ts Lines 702 to 726 in 67638ca
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:
Or will timerUpdated() even be called on a 0->0 transition?
|
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:
|
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. |
So something like: avr8js/src/peripherals/timer.spec.ts Lines 306 to 321 in 320007d
but reworked for timer1 and OCR1A=0
...but tuned properly. I just hacked the writes and am unclear on the cycles and ticks. It also:
|
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:
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?The text was updated successfully, but these errors were encountered: