-1

I have succeeded in creating a Timer Class with an interrupt handler in C++ but struggling to understand why my ISR can only update if the variable is globally scoped and will not update a fully encapsulated member variable. I would like to achieve as shown below where the ISR handler updates the member variable. I'm missing something so would appreciate some help/feedback.

This is what I tried. It compiles but will not update the member variable.

extern "C" void TIMER0_COMPA_vect(void) __attribute__((interrupt));
#define F_CPU 20000000UL

class Timer0 {
    volatile uint16_t counter;
    public:
        static Timer0* pTimer0;
        void handle_interruptTIMER0(void);
        void TIMER0_init(void);
        unsigned int getCounter(void);
        Timer0(const unsigned long);
        ~Timer0();   
};

// Define
Timer0* Timer0::pTimer0;

ISR(TIMER0_COMPA_vect)  {
    Timer0::pTimer0->handle_interruptTIMER0();
}

void Timer0::handle_interruptTIMER0(void)
{   
    PORTD ^= (1 << 7);  // Debug bit PORTD Pin 7 OP_CLK_PIN
    pTimer0->counter++;
}

I would like to be able to update a fully encapsulated variable in the ISR Handler

Phil Davies
  • 31
  • 4
  • What happens when you do it? – user253751 Nov 18 '22 at 16:19
  • Don't see anything in the posted code that would be a problem for this. `getCounter()` (implementation not shown) needs to be properly written to deal with the concurrency issue in reading counter. Note that volatile will not take care of it and is probably unnecessary here. Using getCounter() as sole access to counter in your non-ISR code is a good move. – Avi Berger Nov 18 '22 at 17:09
  • 1
    You aren't showing your instantiation of a Timer0 object with an adequate lifetime guarantee or the initialization of pTimer0 to point to it. Both are required before you allow any timer 0 interrupts to get through. Hopefully, you have dealt with that. – Avi Berger Nov 18 '22 at 17:13
  • Side notes: In the interest of keeping ISRs short: Your timer0 object is effectively going to be a singleton. Probably should be coded as one. If your build tools allow you to make your ISR a static member or friend of Timer0, you could make counter a static member as well and may be able to eliminate handle_interruptTIMER0(). If you keep handle_interruptTIMER0(), there is no reason for it to go through pTimer0 to get to counter. – Avi Berger Nov 18 '22 at 17:29
  • Timer::get counter() is implemented as a simple return counter. The instantiation is basic Timer0 timer; what else should I be thinking about? Thanks – Phil Davies Nov 18 '22 at 17:42
  • 2
    That isn't adequate for Timer::get counter() unless your hardware guarantees it. Since counter is a uint16_t and I believe that you are using an 8 bit micro, I am almost certain it doesn't. You could read 1 byte of counter, interrupt and change it, read the other byte. A usual implementation would be: `uint16_t get counter() { uint16_t value; platformSpecificDisableInterrupts(); value = counter; platformSpecificEnableInterrupts(); return value; }` – Avi Berger Nov 18 '22 at 17:59
  • Just to make sure: your `Timer0 timer;` is either a global or directly in main() - specifically it is not a local in a function that is returning, so going out of scope. – Avi Berger Nov 18 '22 at 18:05

1 Answers1

1

Like @AviBerger hinted at, you likely are forgetting to initialize pTimer0 to point to a Timer0 object. Since the AVR only has one Timer0, it doesn't make too much sense to use a pointer. You can get rid of the pointer and just declare a global instance with a static storage duration by writing this after the class definition:

Timer0 timer0;
David Grayson
  • 84,103
  • 24
  • 152
  • 189