Timer 1 Output Compare

I am struggling to see the error of my ways. I guess there’s every chance I’m not showing the bit that produces the gremlin, but does this make sense to anyone?

I have set up prescale for timer 1 so count increments every 500nsec, & TCR1A, COM1A to 3, with TIMSK1 bit 1 (OCIE1A) set to enable interrupts, & Port D9 set for output. Here’s my interrupt servicing routine code:-

///////////////////////////////////////////////////////////////////////////////    
void Servo1A::OCR1A_ISR()
{    
    //if (digitalRead(9) != LOW)             // commented out

    if ((TCCR1A && (0x01<<6)) != 0)
    {
        OCR1A += 4000; //m_uIncrement;
        //Serial.print("/");
    }
    else
    {
        OCR1A += 6000; //(m_uPeriod - m_uIncrement); // Set on counter = base
        //Serial.print("\\");
    }
    TCCR1A = TCCR1A ^ (0x01<<6);
} 
///////////////////////////////////////////////////////////////////////////////

The way I read this, I should be cueing an interrupt 2msec hence when counter delivers a falling edge on D9. I should also be cuing an interrupt 3msec hence when counter delivers a rising edge. At each interrupt I swap the phase change to be produced when compare matures.

I have tried flexing the 4000 value by linking the delay to a twiddle pot.

So I should be making a square wave with total period of 5msec, 2 msec off, 3msec on. That’s what I think.

What I actually get is a total period of 10 msec, mark time is 3 msec, as expected, but space time is 7 msec! But there’s a glitch at 5msec into the space time. All of this is rock solid repeatable.

I can’t explain the glitch. I am sure that I haven’t put in any conflicting code to interfere with the resources that I am using.

I think I have understood how Timer 1 works. Has anyone seen the problem? Can anyone tell me what I’m doing wrong? The code fragment seems so tiny that I’m struggling to spot what can possibly be wrong. Sorry, my 'scope isn’t up to showing glitch time, but it always seems to be there at precisely the same place.

See attached 'scope photo.

Please show our complete setup for Timer1. What mode are you in? Have you cleared the presets?

It's very unusual to be changing the COM1A1/COM1A0 settings.

I should be cueing an interrupt 2msec hence when counter delivers a falling edge on D9. I should also be cuing an interrupt 3msec hence when counter delivers a rising edge. At each interrupt I swap the phase change to be produced when compare matures.

If you are trying to get square wave with total period of 5msec, 2 msec off, 3msec on this does not sound like the correct approach.

Please explain more about what you are trying to achieve.

Please use proper code tags when posting code, and keep in mind that printing from within an interrupt is a bad idea.

cattledog:
Please show our complete setup for Timer1. What mode are you in? Have you cleared the presets?

I have prepared complete test program to disclose the essence of problem. Please see below.

cattledog:
It’s very unusual to be changing the COM1A1/COM1A0 settings.

I accept what you say, but had no way of knowing that. Nevertheless, I don’t see an unusual approach as an indictment. If my approach was wrong, that’s another matter. Actually I did think I was doing something wrong & other forum readers might spot a mistake which was glaringly obvious to them, but lost on me.

cattledog:
If you are trying to get square wave with total period of 5msec, 2 msec off, 3msec on this does not sound like the correct approach.

I was trying to prove the principle of precision control of an output waveform using Timer 1, Output Capture. Essentially, to satisfy myself that I had understood the documentation provided. Having got the architecture understood, I can fill in the business logic subsequently.

cattledog:
Please explain more about what you are trying to achieve.

I am somewhat preplexed by your request. I thought I had explained that in my original post, and can’t really see what I can add to provide the illumination you seek.

jremington:
Please use proper code tags when posting code.

Please accept my humble apologies. I did try to find out how to include the tags, but all I saw was “use code /code to identify code fragments”, or something like that. I tried code, I tried in xml style, I didn’t know where to go from there. Hopefully, I’ve got it sorted out now.

jremington:
Keep in mind that printing from within an interrupt is a bad idea.

I agree. The print statements were commented out; so they didn’t interfere.

Here’s my test program:-

// Output Compare 1A Test Program 5-Feb-2018

const unsigned c_uMarkTime  = 4000;  // 2msec
const unsigned c_uSpaceTime = 6000;  // 3msec
const int c_nOneSecond = 30;
const int c_nLED = 13;
const int c_nWaveform = 9;

unsigned g_uMarkTime;
unsigned g_uSpaceTime;
unsigned g_uOldCount;
int      g_nCountDown;

///////////////////////////////////////////////////////////////////////////////    

ISR(TIMER1_COMPA_vect)
{
    OCR1A_ISR();
}

///////////////////////////////////////////////////////////////////////////////    

void OCR1A_ISR()
{
    if ((TCCR1A & (0x01 << 6)) != 0)  // If D9 Rising Edge 
    {
        OCR1A += g_uMarkTime;
        bitClear(TCCR1A, 6);          // Change to clear D9 on match
    }
    else 
    {
        OCR1A += g_uSpaceTime;
        bitSet(TCCR1A, 6);            // Change to set D9 on match
    }
}

///////////////////////////////////////////////////////////////////////////////    

void setup() 
{
    Presets();
    pinMode(c_nWaveform, OUTPUT);  // Make Port D9 output for output compare control
    TCCR1A = 0x03 << 6;            //  Set D9 on compare match
    TCCR1B = 0x02;                 //  Setup Prescale to 0.5usec per tick (CPU clock / 8)
    OCR1A = TCNT1 + g_uSpaceTime ;
    bitSet(TIMSK1, 1);             // Enable Output Compare 1A Interrupt
}

///////////////////////////////////////////////////////////////////////////////    

void loop() 
{
    unsigned uNewCount = TCNT1;
    if (uNewCount < g_uOldCount)  // Every 32.768 msec
    {
       Background(uNewCount);
    }
    g_uOldCount = uNewCount;
}

///////////////////////////////////////////////////////////////////////////////    

void Background(unsigned uNewCount)
{
    digitalWrite(c_nLED, !digitalRead(c_nLED));
    Serial.print(digitalRead(c_nLED));
    
    if (--g_nCountDown <= 0) 
    {
        g_nCountDown = c_nOneSecond;
        Serial.print("  ");
        Serial.print(g_uOldCount, HEX);
        Serial.print("  ");
        Serial.print(uNewCount, HEX);
        Serial.println();
    }
}

///////////////////////////////////////////////////////////////////////////////    

void Presets()
{
    Serial.begin(230400);
    Serial.println("Output Compare 1A Test Program");
    Serial.println();
    
    g_nCountDown = c_nOneSecond;
    g_uMarkTime  = c_uMarkTime;
    g_uSpaceTime = c_uSpaceTime;

    pinMode(c_nLED, OUTPUT);       // On board LED is D13
}

///////////////////////////////////////////////////////////////////////////////    
/* Ends */

Fundamentally, this hasn’t done anything different from the code fragmant I originally posed. Whilst I concede this might be an unusual way of changing the COM1A1/COM1A0 settings, I don’t understand why that’s so.

However - bottom line is that the test program is delivering my required 5msec period, 40% duty cycle that I was after. So, I do appreciate the attention given by other contributors, but have to accept that the problem must have been somewhere else in the application. My quest is to run that to earth - but thats another story. To those contributors who took the time and trouble to try to help me, I apologise for wasting your time.

Could the timer WGM have been different than you expected it to be? Could you have set it up with |= or &=, unaware that arduino sets it up differently than the default shown in datasheet to make analogWrite() work?

Why not use the WGM that lets you specify TOP - then you could set TOP and OCR1A and the prescalar, to give you the desired duty cycle using the hardware output compare unit, without using any interrupts?

DrAzzy:
Could the timer WGM have been different than you expected it to be?

When I submitted original post, I was sure that wasn't the case, because I had set it up myself. However, I had not checked it by reading it back. If something that's not part of my code was fiddling with it, I hadn't realised. I will check by reading it back with a mod to the original program. This might help me to run the problem to earth, but I haven't been working on it since my last post. I will get back to that shortly. Thanks for the thought.

DrAzzy:
Could you have set it up with |= or &=, unaware that arduino sets it up differently than the default shown in datasheet to make analogWrite() work?

Ah! Yes, I was unaware that analogWrite flexes WGM, and I have been using analogWrite. Sounds like a clash of resources. I have been writing to analog channel 0, and I have also been making use of timer 1 overflow interrupt & input capture interrupt.

You have given me a very good lead. That may well be my problem. I think a few strategically placed debug statements will enable me to check if something else it playing with bits in the timer1 control registers, and if so, I will be able to figure that out by delving into the support library documents. I must admit, my initial reading of analogWrite didn't bring that to my attention. When I write code, I do tend to document the resources that my code depends upon. I have come to realise that this needs to be defined down to the level of individual bits in a hardware register, so I have managed to have a clear understanding that one class might "own", say, bits 6&7 of a register whilst another "owns", say, bits 4*5 of the same register. I haven't adopted the same rigour with imported code.

Very useful input. Thank you.

DrAzzy:
Why not use the WGM that lets you specify TOP - then you could set TOP and OCR1A and the prescalar, to give you the desired duty cycle using the hardware output compare unit, without using any interrupts?

I want to develop the output waveform into something more complex. I do need to make decisions about the next edge once the current one has happened. I was looking for a steer with what I had done wrong. I believe you have given me that steer. Maybe I can make use of TOP, but, for now, I am happy with my counter that ticks up to 65535 & then wraps back to zero, and keep to that. Then I can manage my D9 output waveform by setting up the next phase change, and the one after that once the next has happened. All my phase changes are at least one msec & no more than 30msec. Seems pretty well suited to Arduino capabilities.

rtdgreg:
I have been writing to analog channel 0, and I have also been making use of timer 1 overflow interrupt & input capture interrupt.

Sorry - I have been reading from ADC0 & ADC1 - that’s PC0 & PC1. I have been writing PWM using analogWrite to PD5 & PD6. Hopefully, the analogWrite would be a problem if I were trying to use PB1 for analogWrite, but that’s not the case.