ISRs and scope

I’m trying to create an ISR but I was hoping to have it modify private class variables. I can’t really think of how to do that (doesn’t seem like the right way to do things anyhow) so I moved the variables to be global. Now I get "multiple definitions error"s on the variables I moved outside of the class. they all say they are first defined at cpp:5

Any suggestions on how to get this working and the best implementation? Thanks a bunch! Ya’ll are awesome

rx.h:

#ifndef rx_h
#define rx_h


unsigned int * timings;
unsigned long micros_last;
unsigned char current_pin;
unsigned char status;
void isr();
unsigned char num_channels;
	unsigned char* pin_assignments;

class RX
{
public:
	RX();
	void init(unsigned char *pin_assignments_in, unsigned char num_channels_in);
	
	// char_num starts at 1, not 0
unsigned char get_pin_assignment(unsigned char chan_num);

	// Between 0 and 255
	unsigned char get_position(unsigned char chan_num);
private:
	unsigned char valid_value(unsigned int value);



};



#endif

rx.cpp:

#include <Arduino.h>

#include "rx.h"

RX::RX(){}
	
void RX::init(unsigned char *pin_assignments_in, unsigned char num_channels_in)
{
	num_channels = num_channels_in;
	timings = new unsigned int[num_channels];
	pin_assignments = new unsigned char[num_channels];
	micros_last = 0;
	current_pin = 0;
	status = RISING;

	for (int i = 0; i< num_channels; i++)
	{
		timings[i] = 0;
		pin_assignments[i] = pin_assignments_in[i];
	}

	attachInterrupt(pin_assignments[0], isr, status);
}

unsigned char RX::get_pin_assignment(unsigned char chan_num)
{
	return pin_assignments[chan_num-1];
}

unsigned char RX::get_position(unsigned char chan_num)
{
	if (valid_value(timings[chan_num]))
	{
		return ((timings[chan_num]-1000)*255)/1000;
	}
	else return 0;
}

unsigned char RX::valid_value(unsigned int value)
{
	if (value < 1000)
		return 0;
	if (value > 2000)
		return 0;
	return 1;
}


void isr()
{
	unsigned long micros_now = micros();
	if (status == RISING)
	{
		micros_last = micros_now;
		status = LOW;
	}
	else
	{
		timings[current_pin] = micros_now - micros_last;
		micros_last = micros_now;
		status = RISING;
		current_pin = (current_pin+1)%num_channels;
	}

	attachInterrupt(pin_assignments[current_pin], isr, status);
}

Well, you do have multiple definitions of those variables, because you will get one each time you include the .h file. One method would be to make them extern, and have a (non-extern) instance once, for example in rx.cpp.

Or you could make them class static variables like this:

rx.cpp:

#include <Arduino.h>

#include "rx.h"

RX::RX(){
}

unsigned char RX::status;
unsigned long RX::micros_last;
unsigned int * RX::timings;
unsigned char RX::current_pin;
unsigned char RX::num_channels;
unsigned char* RX::pin_assignments;

void RX::init(unsigned char *pin_assignments_in, unsigned char num_channels_in)
{
  num_channels = num_channels_in;
  timings = new unsigned int[num_channels];
  pin_assignments = new unsigned char[num_channels];
  micros_last = 0;
  current_pin = 0;
  status = RISING;

  for (int i = 0; i< num_channels; i++)
  {
    timings[i] = 0;
    pin_assignments[i] = pin_assignments_in[i];
  }

  attachInterrupt(pin_assignments[0], isr, status);
}

unsigned char RX::get_pin_assignment(unsigned char chan_num)
{
  return pin_assignments[chan_num-1];
}

unsigned char RX::get_position(unsigned char chan_num)
{
  if (valid_value(timings[chan_num]))
  {
    return ((timings[chan_num]-1000)*255)/1000;
  }
  else return 0;
}

unsigned char RX::valid_value(unsigned int value)
{
  if (value < 1000)
    return 0;
  if (value > 2000)
    return 0;
  return 1;
}


void isr()
{
  unsigned long micros_now = micros();
  if (RX::status == RISING)
  {
    RX::micros_last = micros_now;
    RX::status = LOW;
  }
  else
  {
    RX::timings[RX::current_pin] = micros_now - RX::micros_last;
    RX::micros_last = micros_now;
    RX::status = RISING;
    RX::current_pin = (RX::current_pin+1) % RX::num_channels;
  }

  attachInterrupt(RX::pin_assignments[RX::current_pin], isr, RX::status);
}

rx.h:

#ifndef rx_h
#define rx_h



void isr();

class RX
{

public:

  static unsigned char status;
  static unsigned long micros_last;
  static unsigned int * timings;
  static unsigned char current_pin;
  static unsigned char num_channels;
  static unsigned char* pin_assignments;


  RX();
  void init(unsigned char *pin_assignments_in, unsigned char num_channels_in);

  // char_num starts at 1, not 0
  unsigned char get_pin_assignment(unsigned char chan_num);

  // Between 0 and 255
  unsigned char get_position(unsigned char chan_num);
private:
  unsigned char valid_value(unsigned int value);



};

#endif

I moved the variables into the class as static variables, and then instantiated them in rx.cpp.

Thanks!

bobthemagicmoose:
I’m trying to create an ISR but I was hoping to have it modify private class variables.

Now that I think of it, if you make isr() a static function all those variables can be private.

rx.cpp:

#include <Arduino.h>

#include "rx.h"

RX::RX(){ }

unsigned char RX::status;
unsigned long RX::micros_last;
unsigned int * RX::timings;
unsigned char RX::current_pin;
unsigned char RX::num_channels;
unsigned char* RX::pin_assignments;

void RX::init(unsigned char *pin_assignments_in, unsigned char num_channels_in)
{
  num_channels = num_channels_in;
  timings = new unsigned int[num_channels];
  pin_assignments = new unsigned char[num_channels];
  micros_last = 0;
  current_pin = 0;
  status = RISING;

  for (int i = 0; i< num_channels; i++)
  {
    timings[i] = 0;
    pin_assignments[i] = pin_assignments_in[i];
  }

  attachInterrupt(pin_assignments[0], isr, status);
}

unsigned char RX::get_pin_assignment(unsigned char chan_num)
{
  return pin_assignments[chan_num-1];
}

unsigned char RX::get_position(unsigned char chan_num)
{
  if (valid_value(timings[chan_num]))
  {
    return ((timings[chan_num]-1000)*255)/1000;
  }
  else return 0;
}

unsigned char RX::valid_value(unsigned int value)
{
  if (value < 1000)
    return 0;
  if (value > 2000)
    return 0;
  return 1;
}


void RX::isr()
{
  unsigned long micros_now = micros();
  if (status == RISING)
  {
    micros_last = micros_now;
    status = LOW;
  }
  else
  {
    timings[current_pin] = micros_now - micros_last;
    micros_last = micros_now;
    status = RISING;
    current_pin = (current_pin+1) % num_channels;
  }

  attachInterrupt(pin_assignments[current_pin], isr, status);
}

rx.h:

#ifndef rx_h
#define rx_h

class RX
{

public:

  RX();
  void init(unsigned char *pin_assignments_in, unsigned char num_channels_in);

  // char_num starts at 1, not 0
  unsigned char get_pin_assignment(unsigned char chan_num);

  // Between 0 and 255
  unsigned char get_position(unsigned char chan_num);
  
private:
  unsigned char valid_value(unsigned int value);

  static void isr();
  
  static unsigned char status;
  static unsigned long micros_last;
  static unsigned int * timings;
  static unsigned char current_pin;
  static unsigned char num_channels;
  static unsigned char* pin_assignments;

};

#endif