mkwired:
Can someone tell me why the following code doesn't print "done"? ...
...
TIMSK1 = 1; // OCIE1A: Timer/Counter1, Output Compare A Match Interrupt Enable
...
That line is setting "Bit 0 – TOIE1: Timer/Counter1, Overflow Interrupt Enable", but you have an interrupt on "compare" not "overflow". Change that to 2 and you see "done".
Yes, it doesn't print "done" for me either using that IDE. Never was a saying more apt:
Upgrade: Replace your old bugs with new ones.
OK, try this:
volatile boolean done = 0;
ISR(TIMER1_COMPA_vect)
{
done = true;
}
void setup() {
for (byte i = 2; i <= 13; i++)
pinMode (i, OUTPUT);
Serial.begin(115200);
OCR1A = 1000; // compare A register value
TCCR1A = 0; // normal operation
TCCR1B = _BV(WGM12) | _BV(CS10); // CTC, no prescaling
TIMSK1 = _BV (OCIE1A); // interrupt on Compare A Match
while (!done)
{} // wait for interrupt
Serial.println("done");
for (byte i = 2; i <= 13; i++)
{
digitalWrite (i, HIGH);
delay (200);
}
}
void loop() { }
With my LED board plugged in, LEDs 2 to 13 do indeed light up after a brief delay, so the timer is firing. Why the Serial.println is not working I can't say. But it convinces me to stay with 0022 of the IDE for a while longer.
The IDE 1.0 uses interrupts for Serial.print, so the interrupts are affecting that somehow. I can't say why right now, but at least that explains it partly. The version 0022 of HardwareSerial didn't rely on interrupts.
People got what they wanted. Although I think interrupt based Serial is a good thing, it causes what the Arduino team wanted to avoid in the beginning, which is software stopping because of interrupts. By using interrupts you do need to know how things work instead of copying and pasting from examples. Maybe giving the option for the user to choose interrupt based or polled Serial would be best.
Regarding this issue:
Without looking into the files from the Arduino IDE, I'd make a suggestion for you guys to put your interrupt based code in the loop() section of the program. It's just a hunch. But if we were using C, the software would be written like this:
#include <stdio.h>
#define EVER (;;)
int main () {
//do your hardware setups here...
sei();//notice this... in the end of setups and before infinite loop.
for EVER {
//your cyclic code goes in here.
}
}
Having said this, in the first example, no one actually writes the sei() instruction in any of your programs. Just a hunch, but I may be right. Can someone have a look at the IDE internals to look for this?
That's 29 clock cycles, plus 7 to enter the ISR giving 36. Multiply by 62.5 nS per cycle, and it would take 2.250 uS to execute that. So clearly, very clearly, this is the only interrupt firing. The interrupt for serial transfers is lower priority, so the Serial.println never gets a chance to empty its buffer.
I'm not sure why the timer keeps firing, but I am no expert on timers. A different mode, perhaps?
And I can't say I am very impressed with main.cpp in version 1.0:
#include <Arduino.h>
int main(void)
{
init();
#if defined(USBCON)
USB.attach();
#endif
setup();
for (;;) {
loop();
if (serialEventRun) serialEventRun();
}
return 0;
}
Yuck! A little test every time you run loop. Too bad if you aren't using serial events, there is still a test there that has to be done. And is there a warning anywhere, that if you choose to "do your own" loop, that the test won't be done, and maybe it is important?
Compare to the old main.cpp:
#include <WProgram.h>
int main(void)
{
init();
setup();
for (;;)
loop();
return 0;
}
At least you get what you expect: initialize and then loop.
bubulindo:
Having said this, in the first example, no one actually writes the sei() instruction in any of your programs. Just a hunch, but I may be right. Can someone have a look at the IDE internals to look for this?
Init sets interrupts up:
void init()
{
// this needs to be called before setup() or some functions won't
// work there
sei();
...
HardwareSerial.cpp has some pretty dodgy code in it:
struct ring_buffer
{
unsigned char buffer[SERIAL_BUFFER_SIZE];
volatile int head;
volatile int tail;
};
...
inline void store_char(unsigned char c, ring_buffer *buffer)
{
int i = (unsigned int)(buffer->head + 1) % SERIAL_BUFFER_SIZE;
// if we should be storing the received character into the location
// just before the tail (meaning that the head would advance to the
// current location of the tail), we're about to overflow the buffer
// and so we don't write the character or advance the head.
if (i != buffer->tail) {
buffer->buffer[buffer->head] = c;
buffer->head = i;
}
}
They are using 2-byte fields for the ring buffer, but no protection when accessing them (eg. clearing interrupts), so from time to time they will become corrupted. I would not use this for production code.
// if we should be storing the received character into the location
// just before the tail (meaning that the head would advance to the
// current location of the tail), we're about to overflow the buffer
// and so we don't write the character or advance the head.
if (i != buffer->tail) {
4ba: 80 91 6a 01 lds r24, 0x016A // started accessing the buffer
4be: 90 91 6b 01 lds r25, 0x016B // <--- hope no interrupt here
4c2: 28 17 cp r18, r24 // <--- or here
4c4: 39 07 cpc r19, r25 // <--- or here
4c6: 59 f0 breq .+22 ; 0x4de <__vector_18+0x50>
So, sei() is actually called before setup(), so that is not the reason why the string isn't printed.
One thing I have noticed in wiring.c is this:
// timers 1 and 2 are used for phase-correct hardware pwm
// this is better for motors as it ensures an even waveform
// note, however, that fast pwm mode can achieve a frequency of up
// 8 MHz (with a 16 MHz clock) at 50% duty cycle
#if defined(TCCR1B) && defined(CS11) && defined(CS10)
TCCR1B = 0;
// set timer 1 prescale factor to 64
sbi(TCCR1B, CS11);
#if F_CPU >= 8000000L
sbi(TCCR1B, CS10);
#endif
#elif defined(TCCR1) && defined(CS11) && defined(CS10)
sbi(TCCR1, CS11);
#if F_CPU >= 8000000L
sbi(TCCR1, CS10);
#endif
#endif
// put timer 1 in 8-bit phase correct pwm mode
#if defined(TCCR1A) && defined(WGM10)
sbi(TCCR1A, WGM10);
#elif defined(TCCR1)
#warning this needs to be finished
#endif
so there are some settings done beforehand that should be override before setting all up.
My suggestion is to clear all the TCC1 registers and then set them up.
I'd be happy to set the timers up, but I have no idea what is intended with it. So I guess that's a bit hard.
It just keeps running, nothing ever stopped it, but this is not a bad thing or even unusual. But the interrupt should not be occurring every 2.25µs, it should be every every 62.5µs with the original 1000 in OCR1A, or 187.5µs with 3000. bubulindo's suggestion to clear all the registers first (not knowing what init does to them) is a very good one, but it seems to me that's been done. Hmmmm...
I should have noticed that. It still doesn't work though.