ISR inside cpp file - how to clean this up?

Projects Programming
1

Hi!
I'm working on an AVR Butterfly port for the Arduino, and I'm cleaning up a library that was made for the Arduino IDE 0.19 or so. I got the code working but not like I want. Here's a little information:

The code is forked of the Butteruino project, and hasn't been updated in 8 years. I've done some changes to make this work with the latest IDE. The AVR Butterfly has an onboard 32.768 kHz oscillator thats connected to an 8 bit timer. The original timer library lets you display the current time on the Butterfly LCD. The timer class object, RTCTimer, is located inside the header the library (using 'extern'), but I want to move this to the main inn file. The problem is that there is an ISR (interrupt service routine) inside the cpp file which runs the time keeping function and a user defined callback, and is depending on a known object.
How can I move the object of the class out of the function without messing up the ISR?

I've attached the code as a ZIP file, if you prefer that :slight_smile:

Main.ino

#include "timer2_RTC.h"
#include "Butterfly.h"

// Butterfly LCD object
ButterflyLCD lcd;

// I want this object to work properly
//ButterflyRTC RTCTimer;


void setup()
{
  // Buzzer on pin 13
  pinMode(BUZZER, OUTPUT);

  // Initialize the LCD
  lcd.begin();
  
  lcd.print("Butterfly RTC");
  delay(4000);


  // The RTCTimer can  be started with a 'tick' callback. In this
  // case there is no need to check whether the time has changed,
  // RTCTimer will call the routine you specify when the time changes.
  RTCTimer.begin(secTick);
  lcd.showColons(true);
}


void secTick()
{
  lcd.setCursor(0);
  lcd.print(RTCTimer.hour, DEC);

  lcd.setCursor(2);
  lcd.print(RTCTimer.minute, DEC);

  lcd.setCursor(4);
  lcd.print(RTCTimer.second, DEC);
}

void loop()
{
  //The ISR handles everything
}

Header file:

#ifndef timer2_RTC_h
#define timer2_RTC_h

#include <avr/pgmspace.h>
#include <stdint.h>

class ButterflyRTC
{
  public:
    // Constructor
    ButterflyRTC(void);

    // Function callback
    typedef void (*ClockChangeCallback_t) (void);
    ClockChangeCallback_t clockChangeCallback;
        
    // Public methods
    void begin(ClockChangeCallback_t clockChangeCallback = 0);
    void timerTick();
    
    // Public variables
    volatile uint8_t timeChanged;
    volatile uint8_t second;
    volatile uint8_t minute;
    volatile uint8_t hour;
    volatile uint8_t day;
    volatile uint8_t month;
    volatile uint16_t year;
};

// I want to get rid of this!
extern ButterflyRTC RTCTimer;

#endif

CPP file:

#include <avr/io.h>
#include <avr/interrupt.h>

#include "timer2_RTC.h"
#include "Arduino.h"

// Month length lookup table
const char PROGMEM MonthLength[] = {0, 31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31};

// How can we deal with this?
ButterflyRTC RTCTimer = ButterflyRTC();


ButterflyRTC::ButterflyRTC(void)
{
 // begin();
}


ISR(TIMER2_OVF_vect) 
{
  // Enable global interrupts to be able to update the LCD
  sei();

  // execute a "tick"
 // ButterflyRTC RTCTimer;
  RTCTimer.timerTick();
}


void ButterflyRTC::begin(ClockChangeCallback_t userCallback)
{
  // Save callback
  clockChangeCallback = userCallback;

  // Startup date and time.
  timeChanged = 0;
  second		  = 0;
  minute		  = 0;
  hour    		= 0;
  day			    = 0;
  month   		= 0;
  year		    = 0;

  // Disable global interrupts
  cli();

  TIMSK2 &= ~(1 << TOIE2);               // disable OCIE2A and TOIE2
  ASSR = (1 << AS2);                 // select asynchronous operation of Timer2
  TCNT2 = 0;                           // clear TCNT2A
  TCCR2A |= (1 << CS22);                // select precaler: 32.768 kHz / 64 = 1 sec between each overflow
  while ((ASSR & 0x01) | (ASSR & 0x04)); // wait for TCN2UB and TCR2UB to be cleared
  TIFR2 = 0xFF;                          // clear interrupt-flags
  TIMSK2 |= (1 << TOIE2);                // enable Timer2 overflow interrupt

  // Enable global interrupts
  sei();
}

void ButterflyRTC::timerTick(void)
{
  static char LeapMonth;
  second++;

  if (second == 60)
  {
    second = 0;
    minute++;

    if (minute > 59)
    {
      minute = 0;
      hour++;

      if (hour > 23)
      {
        hour = 0;
        day++;

        // If February check for leap year
        if (month == 2)
          if (!(year & 0x0003))
            if (year % 100 == 0)
              if (year % 400 == 0)
                LeapMonth = 1;
              else
                LeapMonth = 0;
            else
              LeapMonth = 1;
          else
            LeapMonth = 0;
        else
          LeapMonth = 0;

        // Check for month length
        if (day > (MonthLength[month] + LeapMonth))
        {
          day = 1;
          month++;

          if (month > 12)
          {
            month = 1;
            year++;
          }
        }
      }
    }
  }

  timeChanged++;
  // If the user has provided a callback, call it now.
  if (clockChangeCallback != 0)
    clockChangeCallback();
}

Timer2RTC.zip (3.2 KB)

2

The timer class object, RTCTimer, is located inside the header the library (using 'extern')

That doesn't make sense. The extern keyword says that the object is defined somewhere external to the compilation unit.

What I see is that the cpp file creates the instance of the class. The extern statement in the header file tells the sketch that the object already exists, so it does not need to create one.

If you want the sketch to create the object. remove the instantiation from the source file, and the extern line from the source file.

3

If you want the sketch to create the object. remove the instantiation from the source file, and the extern line from the source file.

OK, so I've commented out the object in the header file, and the object in the source file.
Then what should the ISR look like?

ISR(TIMER2_OVF_vect) 
{
  // Enable global interrupts to be able to update the LCD
  sei();

  // execute a "tick"
  timerTick();
}

the problem is that timerTick(); actually runs a user callback function:

main file:

RTCTimer.begin(secTick);

void secTick()
{
  lcd.setCursor(0);
  lcd.print(RTCTimer.hour, DEC);

  lcd.setCursor(2);
  lcd.print(RTCTimer.minute, DEC);

  lcd.setCursor(4);
  lcd.print(RTCTimer.second, DEC);
}

Source file

// This is the two last lines of the timerTick function
if (clockChangeCallback != 0)
    clockChangeCallback();
4

By keeping the object in the source file like this:

// Object inside source file
ButterflyRTC RTCTimer2;


ISR(TIMER2_OVF_vect) 
{
  // Enable global interrupts to be able to update the LCD
  sei();

  // execute a "tick"
  RTCTimer2.timerTick();
}

The user callback function is gone. If I use the same name for the object as in the main file (RTCTimer) I get an error saying there are multiple definitions of 'RTCTimer'.
The timerTick function inside the ISR needs to points towards the object created in the main info file

5

You should NOT be enabling interrupts in the interrupt handler.

You should NOT be doing slow things, like writing to the LCD in an interrupt service routine.

You should NOT be posting snippets. You are not charged by the byte for what you upload here.

6

You should NOT be enabling interrupts in the interrupt handler.

You should NOT be doing slow things, like writing to the LCD in an interrupt service routine.

That is true. It's not ideal. However this is not my work, I'm simply trying to get it up and running in IDE 1.6. This is something I'll might fix/change after I've got rid of this issue. It works, but could and should be rewritten.

You should NOT be posting snippets. You are not charged by the byte for what you upload here.

OK, that fine. I posted the snippets to spare you the trouble of reading all the irrelevant code. If you prefer reading the whole code, that's fine! I was just trying to illustrate the problem further (in code) to be more precise, as English isn't my native language