Error with my library...

I wrote a three wire unidirectional serial communication interface to communicate between two AVRs.
I am getting this error:

/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.cpp: In member function 'int SimpleSerial::begin(byte, byte, byte, boolean)':
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.cpp:18: error: argument of type 'void (SimpleSerial::)()' does not match 'void (*)()'

Here is the sketch:

#include <SimpleSerial.h>


void setup() {
  simple.begin(0, 1, 3, MASTER);
  delay(100);
  simple.send(50);
  delay(100);
  simple.send(100);
  delay(100);
  simple.end();
}

void loop() {
}

The .h:

#ifndef SimpleSerial_h
#define SimpleSerial_h

#include "WProgram.h"

//bite set macros, if we need them
#ifndef cbi
#define cbi(sfr, bit) (_SFR_BYTE(sfr) &= ~_BV(bit))
#endif
#ifndef sbi
#define sbi(sfr, bit) (_SFR_BYTE(sfr) |= _BV(bit))
#endif 
#ifndef gbi 
/* I made this one up. Returns supplied bit of byte. Can be used for 
 * digital read, such as gbi(PINB, 6)
 */
#define gbi(sfr, bit) (!!(_SFR_BYTE(sfr) & _BV(bit)))
#endif

//DEFINES
#define MASTER 1
#define SLAVE 0

#ifndef INTPIN
#define INTPIN 0 //slave clock line, set on pin 2 default, change to 1 for pin 3
#endif

class SimpleSerial
{
	public:
		int begin(byte clkpin, byte datapin, byte busypin, boolean mode);
		void end();
		
		int send(byte data);
		boolean available();
		byte read();
	
	private:
		void receive();
		byte clkpin, datapin, busypin;
		boolean mode;
		byte rxdata, temp;
		byte bits;
		boolean buffer;
};

extern SimpleSerial simple;

#endif

and the .cpp:

#include "WProgram.h"
#include "SimpleSerial.h"


int SimpleSerial::begin(byte clkpin, byte datapin, byte busypin, boolean mode) {
	bits = 0;
	buffer = false;
  if(mode) { //MASTER
    pinMode(clkpin, OUTPUT);
    pinMode(datapin, OUTPUT);
    pinMode(busypin, OUTPUT);
    return 0;
  }
  else { //SLAVE, clkpin must be on an ext. int pin
    pinMode(clkpin, INPUT);
    pinMode(datapin, INPUT);
    pinMode(busypin, INPUT);
    attachInterrupt(INTPIN, receive, RISING);
    temp = 0;
    rxdata = 0;
    return 0;
  }
  return 1;
}

void SimpleSerial::end() {
  if(mode) { //only master can do this
    digitalWrite(busypin, LOW);
    digitalWrite(clkpin, LOW);
    digitalWrite(datapin, LOW);
    busypin = 0; //set everything to 0
    clkpin = 0;
    datapin = 0;
  } 
  else { //if we are a slave, disable interrupts.
    detachInterrupt(INTPIN);
  }
}

//Serial Master commands
int SimpleSerial::send(byte data) {
  if(mode) { //make sure we are a master
    digitalWrite(busypin, HIGH);
    delay(1);
    shiftOut(datapin, clkpin, MSBFIRST, data); //save code by using builtins
    digitalWrite(busypin, LOW); //tell slave we are done.
    return 0;
  }
  return 1; //we are a slave, can't send data
}

//Serial Slave commands
void SimpleSerial::receive() { //ISR triggered by clock rise, private function
  buffer = false;
  rxdata = 0;
  temp |= (digitalRead(datapin) << bits);
  if(bits < 7) { //increment 'bits' only if we are under 7 bits
    bits++;
  } 
  else { //on the eight bit, reset values for next byte
    bits = 0;
    rxdata = temp;
    temp = 0;
    buffer = true;
  }

}

boolean SimpleSerial::available() {
  return buffer;
}

byte SimpleSerial::read() {
  return rxdata;
}

…sorry for having so much!

The problem, obviously, is the attachInterrupt() function in the .cpp file (line 18). The function wants “void (*userFunc)(void)” as its second argument, obviously my receive() function is of the type SimpleSerial. How can I fix it? I basically need the receive function to be called when the clock line rises.

Thanks a lot!

baum

The problem, obviously, is the attachInterrupt() function in the .cpp file (line 18). The function wants "void (*userFunc)(void)" as its second argument, obviously my receive() function is of the type SimpleSerial. How can I fix it? I basically need the receive function to be called when the clock line rises.

Yes, the problem is with the attachInterrupt() call. The attachInterrupt function defines which function to call when an interrupt occurs.

Suppose you have:

SimpleSerial tom;
SimpleSerial dick;
SimpleSerial harry;

Now, the interrupt is triggered. Should tom.receive(), dick.receive(), or harry.receive() be called?

The interrupt handler can not decide which one should be called. So, you must, by declaring receive() to be static, so that multiple instances of the class share just one instance of the function. Then, [u]you/u must, somehow, know which instance of the class is to receive the data that is read from the serial port.

But this:

static void SimpleSerial::receive() {

causes this:

/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.cpp: In member function 'int SimpleSerial::begin(byte, byte, byte, boolean)':
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.cpp:18: error: argument of type 'void (SimpleSerial::)()' does not match 'void (*)()'
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.cpp: At global scope:
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.cpp:53: error: cannot declare member function 'void SimpleSerial::receive()' to have static linkage

baum

The declaration of the function occurs in the header file. That's where the static keyword needs to go, not the source file.

OK.... I put this:

        static void receive();

Inside the private part of the class. Now I get:

/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.cpp:53: error: cannot declare member function 'static void SimpleSerial::receive()' to have static linkage
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.h: In static member function 'static void SimpleSerial::receive()':
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.h:44: error: invalid use of member 'SimpleSerial::buffer' in static member function
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.cpp:54: error: from this location
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.h:42: error: invalid use of member 'SimpleSerial::rxdata' in static member function
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.cpp:55: error: from this location
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.h:42: error: invalid use of member 'SimpleSerial::temp' in static member function
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.cpp:56: error: from this location
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.h:40: error: invalid use of member 'SimpleSerial::datapin' in static member function
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.cpp:56: error: from this location
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.h:43: error: invalid use of member 'SimpleSerial::bits' in static member function
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.cpp:56: error: from this location
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.h:43: error: invalid use of member 'SimpleSerial::bits' in static member function
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.cpp:57: error: from this location
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.h:43: error: invalid use of member 'SimpleSerial::bits' in static member function
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.cpp:58: error: from this location
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.h:43: error: invalid use of member 'SimpleSerial::bits' in static member function
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.cpp:61: error: from this location
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.h:42: error: invalid use of member 'SimpleSerial::rxdata' in static member function
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.cpp:62: error: from this location
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.h:42: error: invalid use of member 'SimpleSerial::temp' in static member function
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.cpp:62: error: from this location
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.h:42: error: invalid use of member 'SimpleSerial::temp' in static member function
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.cpp:63: error: from this location
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.h:44: error: invalid use of member 'SimpleSerial::buffer' in static member function
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.cpp:64: error: from this location

Correct me if I'm wrong, but those errors look like they are saying that I can't use the class's variables in a static function. So how can I make it that the receive function be static, but let it use the class's functions and variables? Basically, I am wondering if it is possible to have multiple connections open which each have their own variables but all share the receive function.

baum

Inside the private part of the class.

And which class would that be?

In the SimpleSerial class in the .h file:

#ifndef SimpleSerial_h
#define SimpleSerial_h

#include "WProgram.h"

//bite set macros, if we need them
#ifndef cbi
#define cbi(sfr, bit) (_SFR_BYTE(sfr) &= ~_BV(bit))
#endif
#ifndef sbi
#define sbi(sfr, bit) (_SFR_BYTE(sfr) |= _BV(bit))
#endif 
#ifndef gbi 
/* I made this one up. Returns supplied bit of byte. Can be used for 
 * digital read, such as gbi(PINB, 6)
 */
#define gbi(sfr, bit) (!!(_SFR_BYTE(sfr) & _BV(bit)))
#endif

//DEFINES
#define MASTER 1
#define SLAVE 0

#ifndef INTPIN
#define INTPIN 0 //slave clock line, set on pin 2 default, change to 1 for pin 3
#endif

class SimpleSerial
{
    public:
        int begin(byte clkpin, byte datapin, byte busypin, boolean mode);
        void end();

        int send(byte data);
        boolean available();
        byte read();

    private:
        static void receive(); //here
        byte clkpin, datapin, busypin;
        boolean mode;
        byte rxdata, temp;
        byte bits;
        boolean buffer;
};

extern SimpleSerial simple;

#endif

Still not working... maybe is "static" not the right keyword? Should I use extern or something else? If I make the function regular, though, (not inside the class) then I can't access the variables and the user can call the function (bad).

baum

Correct me if I'm wrong, but those errors look like they are saying that I can't use the class's variables in a static function.

That is exactly what they are telling you. As I mentioned earlier, if you have multiple instances of the class, which instance's data are you trying to affect?

Still not working... maybe is "static" not the right keyword?

It is. You need to make all the fields you want to write to/read from static, too.

Should I use extern or something else?

It won't help.

If I make the function regular, though, (not inside the class) then I can't access the variables and the user can call the function (bad).

The first part is true. I don't see how a user would accomplish anything by directly calling an interrupt handler, though.

After making all the concerned variables static, I get the following error:

simpleserial_test_1_master.cpp.o: In function `setup':
simpleserial_test_1_master.cpp:8: undefined reference to `simple'
simpleserial_test_1_master.cpp:8: undefined reference to `simple'
SimpleSerial/SimpleSerial.cpp.o: In function `SimpleSerial::receive()':
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.cpp:54: undefined reference to `SimpleSerial::buffer'
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.cpp:55: undefined reference to `SimpleSerial::rxdata'
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.cpp:56: undefined reference to `SimpleSerial::temp'
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.cpp:56: undefined reference to `SimpleSerial::datapin'
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.cpp:56: undefined reference to `SimpleSerial::bits'
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.cpp:56: undefined reference to `SimpleSerial::temp'
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.cpp:58: undefined reference to `SimpleSerial::bits'
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.cpp:61: undefined reference to `SimpleSerial::bits'
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.cpp:62: undefined reference to `SimpleSerial::rxdata'
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.cpp:63: undefined reference to `SimpleSerial::temp'
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.cpp:64: undefined reference to `SimpleSerial::buffer'
SimpleSerial/SimpleSerial.cpp.o: In function `SimpleSerial::send(unsigned char)':
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.cpp:42: undefined reference to `SimpleSerial::mode'
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.cpp:43: undefined reference to `SimpleSerial::busypin'
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.cpp:45: undefined reference to `SimpleSerial::datapin'
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.cpp:45: undefined reference to `SimpleSerial::clkpin'
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.cpp:46: undefined reference to `SimpleSerial::busypin'
SimpleSerial/SimpleSerial.cpp.o: In function `SimpleSerial::end()':
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.cpp:27: undefined reference to `SimpleSerial::mode'
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.cpp:28: undefined reference to `SimpleSerial::busypin'
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.cpp:29: undefined reference to `SimpleSerial::clkpin'
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.cpp:30: undefined reference to `SimpleSerial::datapin'
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.cpp:31: undefined reference to `SimpleSerial::busypin'
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.cpp:32: undefined reference to `SimpleSerial::clkpin'
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.cpp:33: undefined reference to `SimpleSerial::datapin'
SimpleSerial/SimpleSerial.cpp.o: In function `SimpleSerial::begin(unsigned char, unsigned char, unsigned char, unsigned char)':
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.cpp:6: undefined reference to `SimpleSerial::bits'
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.cpp:7: undefined reference to `SimpleSerial::buffer'
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.cpp:19: undefined reference to `SimpleSerial::temp'
/Users/carlbaum/Documents/Arduino/libraries/SimpleSerial/SimpleSerial.cpp:20: undefined reference to `SimpleSerial::rxdata'

All I have done is this:

class SimpleSerial
{
    public:
        int begin(byte clkpin, byte datapin, byte busypin, boolean mode);
        void end();

        int send(byte data);
        boolean available();
        byte read();

    private:
        static void receive();
        static byte clkpin, datapin, busypin;
        static boolean mode;
        static byte rxdata, temp;
        static byte bits;
        static boolean buffer;
};

So now NONE of the functions, including receive() can find my variables!!!! What's going on?

What changes have you made to the sketch and source file?

The receive() method should see the other static variables, although you may need to use the scope resolution operator (i.e. explicitly refer to SimpleSerial::buffer, etc.).

Okay… I changed the function in question to this:

void SimpleSerial::receive() { //ISR triggered by clock rise, private function
  SimpleSerial::buffer = false;
  SimpleSerial::rxdata = 0;
  SimpleSerial::temp |= (digitalRead(SimpleSerial::datapin) << SimpleSerial::bits);
  if(SimpleSerial::bits < 7) { //increment 'bits' only if we are under 7 bits
    SimpleSerial::bits++;
  } 
  else { //on the eight bit, reset values for next byte
    SimpleSerial::bits = 0;
    SimpleSerial::rxdata = temp;
    SimpleSerial::temp = 0;
    SimpleSerial::buffer = true;
  }

}

But I still get the same error as before. (invalid use of member…)

baum

Do you ever plan on having more than one SimpleSerial declared?. If not, why not just make receive a public function and then create a wrapper that simply calls simple.receive() and then set that function as your ISR?

This compiles, no idea if it works. This is no ideal solution as you won’t be able to have more than one instance but I don’t see why it wouldn’t work.

The sketch:

#include "SimpleSerial.h"

SimpleSerial simple;

void setup() {
  simple.begin(0, 1, 3, MASTER);
  delay(100);
  simple.send(50);
  delay(100);
  simple.send(100);
  delay(100);
  simple.end();
}

void loop() {
}

The header:

#ifndef SimpleSerial_h
#define SimpleSerial_h

#include "WProgram.h"

//bite set macros, if we need them
#ifndef cbi
#define cbi(sfr, bit) (_SFR_BYTE(sfr) &= ~_BV(bit))
#endif
#ifndef sbi
#define sbi(sfr, bit) (_SFR_BYTE(sfr) |= _BV(bit))
#endif 
#ifndef gbi 
/* I made this one up. Returns supplied bit of byte. Can be used for 
 * digital read, such as gbi(PINB, 6)
 */
#define gbi(sfr, bit) (!!(_SFR_BYTE(sfr) & _BV(bit)))
#endif

//DEFINES
#define MASTER 1
#define SLAVE 0

#ifndef INTPIN
#define INTPIN 0 //slave clock line, set on pin 2 default, change to 1 for pin 3
#endif

class SimpleSerial
{
	public:
		int begin(byte clkpin, byte datapin, byte busypin, boolean mode);
		void end();
		
		int send(byte data);
		boolean available();
		byte read();
                void receive();
	
	private:
		byte clkpin, datapin, busypin;
		boolean mode;
		byte rxdata, temp;
		byte bits;
		boolean buffer;
};

void receiveWrapper();

extern SimpleSerial simple;

#endif

The source file:

#include "WProgram.h"
#include "SimpleSerial.h"


int SimpleSerial::begin(byte clkpin, byte datapin, byte busypin, boolean mode) {
	bits = 0;
	buffer = false;
  if(mode) { //MASTER
    pinMode(clkpin, OUTPUT);
    pinMode(datapin, OUTPUT);
    pinMode(busypin, OUTPUT);
    return 0;
  }
  else { //SLAVE, clkpin must be on an ext. int pin
    pinMode(clkpin, INPUT);
    pinMode(datapin, INPUT);
    pinMode(busypin, INPUT);
    attachInterrupt(INTPIN, receiveWrapper, RISING);
    temp = 0;
    rxdata = 0;
    return 0;
  }
  return 1;
}

void SimpleSerial::end() {
  if(mode) { //only master can do this
    digitalWrite(busypin, LOW);
    digitalWrite(clkpin, LOW);
    digitalWrite(datapin, LOW);
    busypin = 0; //set everything to 0
    clkpin = 0;
    datapin = 0;
  } 
  else { //if we are a slave, disable interrupts.
    detachInterrupt(INTPIN);
  }
}

//Serial Master commands
int SimpleSerial::send(byte data) {
  if(mode) { //make sure we are a master
    digitalWrite(busypin, HIGH);
    delay(1);
    shiftOut(datapin, clkpin, MSBFIRST, data); //save code by using builtins
    digitalWrite(busypin, LOW); //tell slave we are done.
    return 0;
  }
  return 1; //we are a slave, can't send data
}

//Serial Slave commands
void SimpleSerial::receive() { //ISR triggered by clock rise, private function
  buffer = false;
  rxdata = 0;
  temp |= (digitalRead(datapin) << bits);
  if(bits < 7) { //increment 'bits' only if we are under 7 bits
    bits++;
  } 
  else { //on the eight bit, reset values for next byte
    bits = 0;
    rxdata = temp;
    temp = 0;
    buffer = true;
  }

}

boolean SimpleSerial::available() {
  return buffer;
}

byte SimpleSerial::read() {
  return rxdata;
}

void receiveWrapper() {
  simple.receive();
}

Yes, that compiles... but now that limits me to one instance. Is there a way in which I can specify which instance of the class which the interrupt should trigger?

baum

The receiveWrapper method would need to know about all instances of the class, so that it could invoke the correct receive method. This is possible, if you provide a constructor. You'd also need a setCurrent() method, to set the current instance from the list.

but when doing begin(), is there a way I can set the interrupt to a certain function, i.e. interrupt pin 0 would execute receive0(), pin 1 would be receive1(), etc.

baum

What would you do for the 3rd instance?

There are only 2 interrupt pins on the arduino. But if I shared clock lines, maybe then I could specify the class to use for the dataline...

finally!

Binary sketch size: 1644 bytes (of a 2048 byte maximum)

It's not perfect, but whatever.