Using interrupts with i2c - Problems

Hello Folks.

I am using a DS1307 RTC connected to my Arduino via i2c. I want to use the square wave output to drive the interrupt on the arduino (pin 2 or 3) so that I only have to poll the chip once a second.
So I connected a 4.7k resistor as a pullup on pin 7 of the DS1307 and connected an LED to it (via associated resistor) to check the output and make sure it works. It does.
I have now connected pin 7 of the DS1307 to pin 2 (interrupt 0) on the arduino.

The problem comes when attempting to get data from the DS1307 from within the interrupt method. This is the code I am using:

//add the wire library, this allows us to connect to the device via I2C
#include <Wire.h>
//add the device library
#include <DS1307.h>

DS1307 clk;

void setup()
{
  Serial.begin(9600);
  clk.begin();
  clk.enableOutput(); //enable the square wave output
  clk.setOutputRate(0); //set the output to 1Hz
  attachInterrupt(0, printclock, FALLING);
  Serial.print("initalised");
}
  
void loop()
{

}

void printclock()
{
  Serial.print(clk.getDate());
  Serial.print("/");
  Serial.print(clk.getMonth());
  Serial.print("/20");
  Serial.println(clk.getYear());
  Serial.println(clk.getTime());
  Serial.println(clk.getDay());
}

When I try to run it the arduino just hangs at setup() and you never see “Initalised”. However, if I comment out all the lines in printclock() and replace it with Serial.print(“Badgers”); it works as expected. Therefore I deduce that the problem lies in retrieving the data from the clock.
I have written my own library to operate the clock:

There is the .h

/*
 *  DS1307.h
 *  This library is desinged to operate the DS1307 Real Time Clock via I2C
 *  
 *
 *  Created by Christopher Parish on 23/01/2010.
 *
 */

#include <WProgram.h>
#include <inttypes.h>


//address
const uint8_t ADDR = B01101000;


// Control Bytes
const uint8_t OUTCON = B10000000;
const uint8_t SQWE       = B00010000;

//mode
const uint8_t MODE2412 = B01000000;

//registeres
const uint8_t SECREG   = 0x00;
const uint8_t MINREG   = 0x01;
const uint8_t HOURREG  = 0x02;
const uint8_t DAYREG   = 0x03;
const uint8_t DATEREG  = 0x04;
const uint8_t MONTHREG = 0x05;
const uint8_t YEARREG  = 0x06;
const uint8_t CTRLREG  = 0x07;

// Square Wave frequence control~~~~~~~~~~~
// 0 = 1Hz
// 1 = 4.096kHz
// 2 = 8.192kHz
// 3 = 32.768kHz

class DS1307 
{
public:
      DS1307();
      //setters
      void setSeconds(int seconds);
      void setMinutes(int minutes);
      void setHours(int hours);
      void setDay(int day);
      void setDate(int date);
      void setMonth(int month);
      void setYear(int year);
      void enableOutput();
      void disableOutput();
      void setDefaultOutputLow();
      void setDefaultOutputHigh();
      void setOutputRate(int rate);
      void setTime(int hours, int minutes, int seconds);
      void setDate(int day, int month, int year);
      void set24Mode();
      void set12Mode();
      void setTimeFormat(char* tf);
      
      //getters
      int getSeconds();
      int getMinutes();
      int getHours();
      int getDay();
      int getDate();
      int getMonth();
      int getYear();
      char* getTime();
      void begin();
private:
;
      void sendData(uint8_t reg, uint8_t data);
      int getData(uint8_t reg);
      uint8_t decToBCD(uint8_t number);
      uint8_t BCDToDec(uint8_t number);
      uint8_t _mode;
      char* _timeformat;

};

and here is the .cpp

/*
 *  DS1307.cpp
 *  This library is desinged to operate the DS1307 Real Time Clock via I2C
 *  
 *
 *  Created by Christopher Parish on 23/01/2010.
 *
 */

#include "DS1307.h"
#include <Wire.h>
#include <inttypes.h>
#include <stdio.h>


DS1307::DS1307()
{
      Wire.begin();
}

void DS1307::setSeconds(int seconds)
{
      sendData(SECREG, decToBCD(seconds));
}

void DS1307::setMinutes(int minutes)
{
      sendData(MINREG, decToBCD(minutes));
}

void DS1307::setHours(int hours)
{
      if (hours > 12 && _mode != 0) {
            sendData(HOURREG, decToBCD(hours - 12) | B01100000);
      } else if (_mode != 0) {
            sendData(HOURREG, decToBCD(hours) | B01000000);
      } else {
            sendData(HOURREG, decToBCD(hours));
      }
}

void DS1307::setDay(int day)
{
      sendData(DAYREG, decToBCD(day));
}
                   
void DS1307:: setDate(int date)
{
      sendData(DATEREG, decToBCD(date));
}

void DS1307::setMonth(int month)
{
      sendData(MONTHREG, decToBCD(month));
}

void DS1307::setYear(int year)
{
      sendData(YEARREG, decToBCD(year));
}

void DS1307::setDefaultOutputLow()
{
      int ctrl = getData(CTRLREG);
      sendData(CTRLREG, ctrl & ~OUTCON);
}

void DS1307::setDefaultOutputHigh()
{
      int ctrl = getData(CTRLREG);
      sendData(CTRLREG, ctrl | OUTCON);
}

void DS1307::enableOutput()
{
      int ctrl = getData(CTRLREG);
      sendData(CTRLREG, ctrl | SQWE);
}

void DS1307::disableOutput()
{
      int ctrl;
      ctrl = getData(CTRLREG);
      sendData(CTRLREG, ctrl & ~SQWE);
}

void DS1307::setOutputRate(int rate)
{
      if (rate >= 0 && rate <=3) {
            sendData(CTRLREG, rate | SQWE);
      }
}

void DS1307::setTime(int hours, int minutes, int seconds)
{
      setHours(hours);
      setMinutes(minutes);
      setSeconds(seconds);
}

void DS1307::setDate(int day, int month, int year)
{
      setDay(day);
      setMonth(month);
      setYear(year);
}

void DS1307::set24Mode()
{
      if (_mode == 1)
      {
            //first we need to retrive the current hour value
            Wire.beginTransmission(ADDR);
            Wire.send(HOURREG);
            Wire.endTransmission();
            Wire.requestFrom((int)ADDR, 4);
            int hours;
            hours = Wire.receive();
            if (hours & B01100000) { // is it pm and 12 hour mode is enabled
                  hours = BCDToDec(hours & ~B01100000); //retreive the current hours reading and remove the 12 hour mode flag
                  hours = hours + 12; //add 12 to the hours to make it 24 hour
                  hours = decToBCD(hours); //convert it back to BCD
                  sendData(HOURREG, hours);
            } else if (hours & B01000000) {
                  sendData(HOURREG, hours & ~B01000000);
            }
            _mode = 0;
      }
}

void DS1307::set12Mode()
{
      if (_mode == 0)
      {
            int hours;
            //first we need to retrive the current hour value
            hours = getData(HOURREG);
            hours = (int)BCDToDec(hours);
            Serial.println(hours);
            if (hours > 12) {
                  hours = hours - 12;
                  hours = decToBCD(hours);
                  sendData(HOURREG, (hours | B01100000));            
            } else {
                  sendData(HOURREG, (hours | B01000000));
            }
            _mode = 1;
      }
}

void DS1307::setTimeFormat(char* tf)
{
      _timeformat = tf;
}


int DS1307::getSeconds()
{
      return (BCDToDec(getData(SECREG)));
}

int DS1307::getMinutes()
{
      return (BCDToDec(getData(MINREG)));
}

int DS1307::getHours()
{
      int hours = getData(HOURREG);
      if (_mode != 0)
      {
            hours &= ~B01100000;
      }
      return (BCDToDec(hours));
}

int DS1307::getDay()
{
      return (BCDToDec(getData(DAYREG)));
}

int DS1307::getDate()
{
      return (BCDToDec(getData(DATEREG)));
}

int DS1307::getMonth()
{
      return (BCDToDec(getData(MONTHREG)));
}

int DS1307::getYear()
{
      return (BCDToDec(getData(YEARREG)));
}

char* DS1307::getTime()
{
      static char timestamp[9];
      int hours, minutes, seconds;
      hours = getHours();
      minutes = getMinutes();
      seconds = getSeconds();
      sprintf(timestamp, _timeformat , hours, minutes, seconds);
      return (timestamp);
}


void DS1307::begin()
{
      int hours;
      hours = getData(HOURREG);
      if (hours & 0x40)
      {
            _mode = 1;
      } else {
            _mode = 0;
      }
      _timeformat = "%02d:%02d:%02d";
}

void DS1307::sendData(uint8_t reg, uint8_t data)
{
      Wire.beginTransmission(ADDR);
      Wire.send(reg);
      Wire.send(data);
      Wire.endTransmission();
}

int DS1307::getData(uint8_t reg)
{
      Wire.beginTransmission(ADDR);
      Wire.send(reg);
      Wire.endTransmission();
      Wire.requestFrom((int)ADDR, 1);
      return (Wire.receive());
}


uint8_t DS1307::decToBCD(uint8_t number)
{
      return (((number / 10)<<4) | (number%10));
}

uint8_t DS1307::BCDToDec(uint8_t number)
{
      uint8_t msb, lsb;
      msb = number >> 4;
      lsb = number & B00001111;
      msb = (msb * 10) + lsb;
      //return (((number >> 4) * 10) + (number & B00001111));
      return (msb);
}

Any ideas folks?

Therefore I deduce that the problem lies in retrieving the data from the clock.

I think you are doing too much in the ISR. Another interrupt fires before the first one completes, so it gets interrupted.

I would try calling the clk functions, but only Serial.print'ing a ".". See if that prints.

I am not sure i2c will work from within an interrupt handler.

If not, the arduino millis timer can tell you within a millisecond of how much time has elapsed since you last got a reading from the RTC and you can safely use that to tell you when its time to refresh your clock.

I think you are doing too much in the ISR. Another interrupt fires before the first one completes, so it gets interrupted.

I don't think this is the case, as the interrupt only fires once a second, and it takes far less than a second to execute this call (it takes around 1/40th of a second)

I would try calling the clk functions, but only Serial.print'ing a ".". See if that prints.

If I add a clk function before the Serial.print then I never see the Serial.print output. However if I put the clk function after the Serial.print then I see exactly one print output.

I am not sure i2c will work from within an interrupt handler

It is looking like this may indeed be the case. Which is a bit of a swine, as I have a couple of other projects in which I was hoping to use the interrupt to notify of changes to items connected via i2c.

It is looking like this may indeed be the case. Which is a bit of a swine, as I have a couple of other projects in which I was hoping to use the interrupt to notify of changes to items connected via i2c.

You can always create a global variable such as needToReadI2CData. Set it to false initially. In the ISR, set it to true. In loop, test the value. Read the I2C data if true, and reset to false.