Extending the ISR of the TinyWire library.

I am currently working on a small project on an attiny85 that requires me to use one of the external pin change interrupts. I had that set up all working. I also need the tiny to talk to another device using i2c, so I wanted to implement that using the TinyWire library. However, I sadly had to find out that this library already uses the ISR for the PCINT vector and there's no mean for me to somehow edit that. So figured I could edit the library a bit and add a handler function to sneak in my code. I've never done something like that but here's my effort based heavily on this:

The library itself is structured as follows:

  • TinyWire.cpp
  • TinyWire.h
  • twi.cpp - containing lots of smart stuff which i mostly don't understand as well as my desired ISR
  • twi.h

First thing I did was adding a static function pointer to the twi.h file

//twi.h

static void (*__PCINT0Hook)(void);

The ISR (included in twi.cpp) was by one line at the bottom to look like this:

//twi.cpp

ISR( PCINT0_vect )
{
  if((PIN_USI & (1 << PIN_USI_SDA)) && (PIN_USI & ( 1 << PIN_USI_SCL ))){
    // stop condition occured
    _delay_us(7);
    if((PIN_USI & (1 << PIN_USI_SDA)) && (PIN_USI & ( 1 << PIN_USI_SCL ))){
      twi_bus_busy = false;
      if(currently_receiving){ // If we are receiving bytes from a master, call user callback
        if(rxByteNum>0) twi_onSlaveReceive(rxByteNum);  // check, if anything is in the rx buffer, because it is possible,
                                                        // that the communication was interrupted by a stop condition
        currently_receiving = false;
      }
      USISR |= 1<<USIPF; // resetting stop condition flag
    }
  }
  // --------- NEW STUFF -----------
  __PCINT0Hook();
}

Now I just added a public function to the TinyTwi class that sets the function pointer to my desire.

//TinyWire.h
void PCINT0Hook(void (*function)(void));

 --------------------------------------

//TinyWire.cpp
void TinyTwi::PCINT0Hook(void (*function)(void))
{
  __PCINT0Hook = function;
}

Now all I need to do in order to get my code executed in the ISR, is to wrap it in a function in my main program and pass it to the handler.

//main.ino

void MySneakyISR()
{
  //do something
}

TinyWire.PCINT0Hook(demoISRaction);

Or so I thought... Turns out I'm not the genius I thought I was and I can't seem to get this to work. It's also really hard trying to figure out what's wrong, as my way of debugging was primarily flashing leds and sending information over i2c (which is not working atm).

I was wondering if any of you could help me get on track again? Thanks for all the ideas in advance.

The issue you are having is likely with how you are defining __PCINT0Hook: you have defined it as static and in a header file which means that it will be redefined for each compilation unit. ie. twi.h and twi.cpp get compiled and have storage for __PCINT0Hook, then TinyWire.h and TinyWire.cpp get compiled and have storage for __PCINT0Hook (but not the same storage as the twi.* compilation unit).

Some things to note:

  • Don't define variables in header files; you should declare them there if necessary using the extern keyword and then define them in one cpp file.
  • static in the context you have used it means that the variable is local/private to the compilation unit that is being compiled.
  • Avoid using double underscores in variable names as they are reserved in C++. If you really want to use an underscore, then a single one at the end of the variable should be OK.

So, a simple fix for your case is to define the variable in twi.cpp

void (*PCINT0Hook_)(void) = 0;

Then declare it in twi.h

extern void (*PCINT0Hook_)(void);

And it may be a good idea to check that it has indeed been set before using it

ISR( PCINT0_vect )
{
...
      USISR |= 1<<USIPF; // resetting stop condition flag
    }
  }
  // --------- NEW STUFF -----------
  if (PCINT0Hook_) PCINT0Hook_();
}

Wow, thanks a lot that seemed to have solved it. I still had a few questions regarding your answer though.

Why is that a bad habit? Because I run the risk of defining the variables in multiple places then?

So it is only visible within the file, or with every file that includes my h file? I always thought static just means it never goes out of scope.

By being reserved do you mean that by convention it is used in C++ or is there a syntax that uses it?
Thank you again for your great help!

Jocobes:
Why is that a bad habit? Because I run the risk of defining the variables in multiple places then?

Yes, as you just found out.

Jocobes:
So it is only visible within the file, or with every file that includes my h file? I always thought static just means it never goes out of scope.

static has multiple meanings depending on the context. Life's complicated.

Jocobes:
By being reserved do you mean that by convention it is used in C++ or is there a syntax that uses it?

It is reserved by the C++ specification for internal uses. For example, double underscores might be used as part of a compiler implementation's name mangling.