I2C ReceiveEvent doesn't affect global variables

Hi all,

I'm attempting to set up two Arduino Megas on I2C. One is a joystick (master), whose primary task is monitoring switches and pots and sending results upon changes, and the other is the (slave) receiver whose primary task is operating the equipment. The issue at the moment seems to be that data is accurately received inside the "receiveEvent" routine, but global variables outside the Event scope are not affected. I have used "volatile" everywhere the compiler lets me, to no avail.

I have plagiarized the Wire Example (Wire Master Writer Slave Reader) for the code. I understand that "using Serial inside an ISP is not good," but I'm not sure why, so I used it to verify all bytes were successfully transmitted / received, then commented it back out and continued testing before resorting to this post. When used, the Serial.print commands verified that data sent from the joystick is being received properly inside the "receiveEvent" routine.

I'd appreciate any insight moderators / gurus share to offer.

char temp1, temp2;
volatile uint8_t mBuf[6];

typedef struct  wLine {
	volatile uint8_t cmd;	// byte will be 'a' .. 'd'
	volatile uint8_t swVal;
	volatile int vVal;
	volatile int hVal;
};

volatile wLine comm;

....

void setup() {
	Serial.begin(9600);           // start serial for output
	Wire.begin(8); // join i2c bus (address optional for master)
	Wire.onReceive(receiveEvent); // register event
...
	comm.cmd = 'L';
	comm.swVal = mBuf[1] = 0x3F;
	comm.vVal = -1;
	comm.hVal = -1;
	mBuf[0] = mBuf[1] = mBuf[2] = mBuf[3] = mBuf[4] = mBuf[5] = -1;
...
}

void loop() {
	if (comm.swVal == 0x3F) {         // Benign state where no switches are pressed
	}
	else {                                        // <-----------------This section never gets executed,
                                                        //  though I've verified that data is correctly received 
                                                        //  (and changes with switch press) inside the handler

		Serial.println("--------------"); // print the character
		Serial.print("cmd: "); // print the character
//		Serial.print(mBuf[2]); // print the character
		Serial.print(", sw: "); // print the character
		Serial.print(comm.swVal, HEX); // print the character
		Serial.println(",  hVal: "); // print the character
		Serial.println(comm.hVal, HEX); // print the character
		Serial.print(",  vVal: "); // print the character
		Serial.println(comm.vVal, HEX); // print the character
		Serial.println("--------------"); // print the character

...

}

void receiveEvent(int howMany) {
	uint8_t buf[6], index = 0;
	buf[0] = 0; buf[1] = 0; buf[2] = 0; buf[3] = 0; buf[4] = 0; buf[5] = 0;
//		Serial.println("-------------------"); // print the character
	while (0 < Wire.available())  // loop through all but the last
	{
		buf[index] = Wire.read(); // receive byte
		mBuf[index] = buf[index];
//		Serial.print("index: "); // print the character
//		Serial.print(index, HEX); // print the character
//		Serial.print(", byte: "); // print the character
//		Serial.println(buf[index], HEX); // print the character
		index++;
	}
//		Serial.println("-------------------"); // print the character
	mBuf[0] = buf[0]; mBuf[1] = buf[1]; mBuf[2] = buf[2]; 
        mBuf[3] = buf[3]; mBuf[4] = buf[4]; mBuf[5] = buf[5];

	comm.cmd = buf[0];                  // single byte ASCII char for later expansion
//		Serial.print("Event: cmd: "); 
//		Serial.print(comm.cmd, HEX); 
//		Serial.print("buf[0]: "); 
//		Serial.println(buf[0], HEX); 

	comm.swVal = buf[1];               // receive byte (switch settings, norm 0x3F HIGH)
//		Serial.print("sw: ");
//		Serial.print(comm.swVal, HEX); 
//		Serial.print(" buf[1]: "); 
//		Serial.println(buf[1], HEX); 

	comm.hVal = buf[2];                 // 2 byte horizontal DAC
	comm.hVal <<= 8;
	comm.hVal += buf[3];// | buf[3] << 8;
//		Serial.print("hVal: "); 
//		Serial.print(comm.hVal, HEX); 
//		Serial.print("buf[2]: "); 
//		Serial.print(buf[2], HEX); 
//		Serial.print(" "); 
//		Serial.println(buf[3], HEX); 

	comm.vVal = buf[4];                 // 2 byte vertical DAC
	comm.vVal <<= 8;
	comm.vVal += buf[5];
//		Serial.print("  vVal: ");      
//		Serial.println(comm.vVal, HEX); 
//		Serial.println("-------------------");
}

I understand that "using Serial inside an ISP is not good," but I'm not sure why,

When the Interrupt Service Routine (ISR) is entered, interrupts are disabled - so the interrupt that is created when a byte is sent or received is not created.

Please post ALL of your code. What you posted will not compile.

PaulS:
Please post ALL of your code. What you posted will not compile.

As you wish. It's several files on the receiver side, most of which will not fit due to limitation of the web site. The .ino:

#if defined(ARDUINO) && ARDUINO >= 100
 //#include "arduino.h"

#else
#include "WProgram.h"
#endif

#include <Wire.h>

 // const int buttonPin = 26;
 // const int ledPin = 13;
#ifdef RAMPS_H
#include <EEPROM.h>
#else
#include <Servo.h>
#define JOYSTICK_PS3
#endif // RAMPS_H

#include <SoftwareSerial.h>
#include "display.h"
#include "sendData.h"
#include "switches.h"

// #define VERBOSE_ALL
// #define VERBOSE_DEBUG
// #define VERBOSE_ECHO
// #define VERBOSE_NONE

//***************************************************************************

d7Seg seg7;
Servo serv1;  // create servo object to control a servo

unsigned long time;
boolean lastState;

volatile int count;
char temp1, temp2;
volatile uint8_t mBuf[6];

typedef struct  wLine {
	volatile uint8_t cmd;	// byte will be 'a' .. 'd'
	volatile uint8_t swVal;
	volatile int vVal;
	volatile int hVal;
};

volatile wLine comm;

void setup() {
	Serial.begin(9600);           // start serial for output
	Wire.begin(8); // join i2c bus (address optional for master)
	Wire.onReceive(receiveEvent); // register event

	// initialize the digital pin as an output.
	// Pin 13 has an LED connected on most Arduino boards:
	serv1.attach(2);  // attaches the servo on pin 9 to the servo object
  // Serial.begin(9600);
	pinMode(22, OUTPUT);	// 7 segment "a"
	pinMode(23, OUTPUT);	// 7 segment "b"
	pinMode(24, OUTPUT);	// 7 segment "c"
	pinMode(25, OUTPUT);	// 7 segment "d"
	pinMode(26, OUTPUT);	// 7 segment "e"
	pinMode(27, OUTPUT);	// 7 segment "f"
	pinMode(28, OUTPUT);	// 7 segment "g"
//	pinMode(11, OUTPUT);	// 7 segment "P"
  //  sw1.init(sw1Pin);
  //  sw2.init(sw2Pin);
	count = -13;
	lastState = 0;
	seg7.init(22, 23, 24, 25, 26, 27, 28);
	comm.cmd = 'L';
	comm.swVal = mBuf[1] = 0x3F;
	comm.vVal = -1;
	comm.hVal = -1;
	mBuf[0] = mBuf[1] = mBuf[2] = mBuf[3] = mBuf[4] = mBuf[5] = -1;

#ifdef JOYSTICK_PS3
#endif // JOYSTICK_PS3
}

void loop() {
	if (comm.swVal == 0x3F) {
	}
	else {
		Serial.println("--------------"); // print the character
		Serial.print("cmd: "); // print the character
//		Serial.print(mBuf[2]); // print the character
		Serial.print(", sw: "); // print the character
		Serial.print(comm.swVal, HEX); // print the character
		Serial.println(",  hVal: "); // print the character
		Serial.println(comm.hVal, HEX); // print the character
		Serial.print(",  vVal: "); // print the character
		Serial.println(comm.vVal, HEX); // print the character
		Serial.println("--------------"); // print the character
		mBuf[0] = mBuf[1] = mBuf[2] = mBuf[3] = mBuf[4] = mBuf[5] = -1;


		Serial.print("swVal == ");
		Serial.println(comm.swVal);
		if (!(comm.swVal & 0x01)) {	// Down Button
			Serial.println("0x01 ");
			temp1 = 0;
		}
		if (!(comm.swVal & 0x02)) {	// Right Button
			Serial.println("0x02 ");
			temp1 = 12;
		}
		if (!(comm.swVal & 0x04)) {	// Up Button
			Serial.println("0x04 ");
			temp1 = 10;
		}
		if (!(comm.swVal & 0x08)) {	// Left Button
			Serial.println("0x08 ");
			temp1 = 11;
		}
		if (!(comm.swVal & 0x10)) {	// Select Button
			Serial.println("0x10 ");
			temp1 = 1;
		}
		if (!(comm.swVal & 0x20)) {	// Left Joystick Pushbutton
			Serial.println("0x20 ");
			temp1 = 2;
		}
	}
	//	seg7.draw(temp1);
	//	Serial.print("temp1=");
	//	Serial.println(temp1);

	switch (temp1) {
	case 10:	// UP button
		Serial.println("UP Button pushed.");
		count = 170;
		break;
	case 0:	// DOWN button
		Serial.println("DOWN Button pushed.");
		count = 0;
		break;
	case 12:	// RIGHT button
		Serial.println("RIGHT Button pushed.");
		count -= 5;
		break;
	case 11:	// LEFT button
		Serial.println("LEFT Button pushed.");
		count += 5;
		break;
	default:
		break;
	}
	if (count < 0) {
		count = 0;
	}
	else if (count > 170) {
		count = 170;
	}
	serv1.write(count);   // sets the servo position in degrees

	comm.cmd = 'L';
	comm.swVal = mBuf[1] = 0x3F;
	comm.vVal = -1;
	comm.hVal = -1;
	temp1 = -1;
//	}
//	delay(100);
//	comm[0].val = 63;
}

void altLoop(void) {
	delay(10);
}

// function that executes whenever data is received from Joystick Reader
// this function is registered as an event, see setup()
void receiveEvent(int howMany) {
	uint8_t buf[6], index = 0;
	buf[0] = 0; buf[1] = 0; buf[2] = 0; buf[3] = 0; buf[4] = 0; buf[5] = 0;
//		Serial.println("-------------------"); // print the character
	while (0 < Wire.available())  // loop through all but the last
	{
		buf[index] = Wire.read(); // receive byte
		mBuf[index] = buf[index];
//		Serial.print("index: "); // print the character
//		Serial.print(index, HEX); // print the character
//		Serial.print(", byte: "); // print the character
//		Serial.println(buf[index], HEX); // print the character
		index++;
	}
//		Serial.println("-------------------"); // print the character
	mBuf[0] = buf[0]; mBuf[1] = buf[1]; mBuf[2] = buf[2]; mBuf[3] = buf[3]; mBuf[4] = buf[4]; mBuf[5] = buf[5];
	comm.cmd = buf[0]; // receive byte as a character
//		Serial.print("Event: cmd: "); // print the character
//		Serial.print(comm.cmd, HEX); // print the character
//		Serial.print("buf[0]: "); // print the character
//		Serial.println(buf[0], HEX); // print the character

	comm.swVal = buf[1];    // receive byte (switch settings)
//		Serial.print("sw: "); // print the character
//		Serial.print(comm.swVal, HEX); // print the character
//		Serial.print(" buf[1]: "); // print the character
//		Serial.println(buf[1], HEX); // print the character

	comm.hVal = buf[2];// | buf[3] << 8;    // receive integer
	comm.hVal <<= 8;
	comm.hVal += buf[3];// | buf[3] << 8;
//		Serial.print("hVal: "); // print the character
//		Serial.print(comm.hVal, HEX); // print the character
//		Serial.print("buf[2]: "); // print the character
//		Serial.print(buf[2], HEX); // print the character
//		Serial.print(" "); // print the character
//		Serial.println(buf[3], HEX); // print the character

	comm.vVal = buf[4];// | buf[5] << 8;    // receive integer
	comm.vVal <<= 8;
	comm.vVal += buf[5];// | buf[3] << 8;
//		Serial.print("  vVal: "); // print the character
//		Serial.println(comm.vVal, HEX); // print the character
//		Serial.println("-------------------"); // print the character
}

The display.h file (primarily for a 128x64 used elsewhere):

// display.h

#ifdef RAMPS_H
	#include <U8glib.h>
#include <EEPROM.h>
#include "Control.h"
#endif // RAMPS_H

#include "arduino.h"
#include <SoftwareSerial.h>
//#else
//#include "WProgram.h"
//#endif
#include <String.h>
//#include "motors.h"

// display options
#define LED_OFF 0;
#define LED_ON 1;

#define LED_OFF_LOW		0;	// To turn an LED off, Write the pin LOW   ***** Uncomment only ONE of these two lines
// #define LED_OFF_HIGH	1;	// To turn an LED off, Write the pin HIGH

#define DRAW_INFO	0;	// Draw the Info screen
#define DRAW_MENU	1;	// Draw the Menu screen

#ifdef RAMPS_H
extern uint8_t menuItem, temp1, temp2, temp3, temp4;
extern U8GLIB_ST7920_128X64_4X u8g;
extern dScreen disp;
extern String getValue(String data, char separator, int index);
extern struct Motor mX, mY, mZ, mE, mF;
#endif // RAMPS_H


#ifndef _DISPLAY_h
#define _DISPLAY_h

#ifdef RAMPS_H
typedef struct mLine {
	long loc;
	long spd;
};

typedef struct dScreen {
	mLine x;
	mLine y;
	mLine z;
	mLine e;
	mLine f;
	String lCmd;
};

void displayInit(void);
void beep(uint8_t mSec);
int8_t readEncoder(void);
void pUnits(Motor* motor, int x, int y);
void drawMain(void);
void drawMotorMenu(void);

#endif // RAMPS_H

class Led
{
private:
	uint8_t  pinNumber;
	boolean	state;

public:
	void init(uint8_t led);
	uint8_t pin(void);
	void set(void);
	void clear(void);

	Led(void);
};

class d7Seg
{
private:
	Led a, b, c, d, e, f, g, P;
//	uint8_t  pin1, pin2, pin3, pin4, pin5, pin6, pin7, pinDot;
	uint8_t	state;

public:
	void init (uint8_t pin1, uint8_t pin2, uint8_t pin3, uint8_t pin4, uint8_t pin5, uint8_t pin6, uint8_t pin7);
	void blank(void);
	void draw(uint8_t val);

	d7Seg(void);
};

#endif

The corresponding display.cpp, senddata.h and .cpp (not used at the moment) and the switches.h and .cpp would not fit due to size limitations on this web site. I haven't yet figured out sites like git and bintray, but I suppose that's for another forum.

CrossRoads:
When the Interrupt Service Routine (ISR) is entered, interrupts are disabled - so the interrupt that is created when a byte is sent or received is not created.

@CrossRoads, I suspected as much, and was frankly surprised that VisualStudio / Visual Micro worked in this mode (at all) to display received data using Serial from within receiveEvent. However, having commented out all calls to Serial inside that routine, I'm still at a loss as to why it shouldn't be used outside the ISR, or why global volatile variables aren't ( ( accessible from within the ISR) / (affected outside the ISR by changes from within it) )?

From my olden days when I actually wrote code for a living, this "smells" like a scope issue. But I just can't seem to put my finger on why.

Thanks for the kind and clarifying reply.

		buf[index] = Wire.read(); // receive byte
		mBuf[index] = buf[index];

Why do you need two copies of the data?

	mBuf[0] = buf[0]; mBuf[1] = buf[1]; mBuf[2] = buf[2]; mBuf[3] = buf[3]; mBuf[4] = buf[4]; mBuf[5] = buf[5];

Haven't you already assigned values to mBuf[n] for n equal 0 to 5?

PaulS:

		buf[index] = Wire.read(); // receive byte
	mBuf[index] = buf[index];


Why do you need two copies of the data?

Haven't you already assigned values to mBuf[n] for n equal 0 to 5?

To answer your question, I'm sure you've deduced by now that one was a global array and one was local. The intent was to try to verify and solve why local variables were being received but global variables, be they byte, char, int or array, were not updating.

I've since solved the mystery--it was a logic error, not a scope or a library issue. Thanks for your help.

Why so many code. I think all that is needed is this:

void receiveEvent(int howMany) {
  if (sizeof(comm) == howMany) {   // it is the right amount of bytes of the struct ?
    Wire.readBytes( (byte *) &comm, howMany);
  }
}

You have to be careful with all the reading and writing to 'comm'.
When using 'comm' in the loop(), an element that is a byte can be used, but when an integer is used (vVal, hVal), then the interrupts must be temporarely disabled, because the receiveEvent() could happen in between reading or writing the two bytes of the integer.