Confusing results when reading I2C data from one Arduino to another!

Using Arduino Programming Questions
1

Hello good people of Arduino Land.

This question relates to another question about the same project.
It is in regards to a water drop controller which I am building for high speed photography, water drop collision projects.

The previous post goes into more detail about what I am hoping to achieve, and can be found with the following link:

https://forum.arduino.cc/index.php?topic=679995.0

I shall be using four Arduinos in total:

  • One Nano as master
  • Three ATMega328P-PU chips as slaves to operate the valves - one valve/chip

The Nano is the input for the timing data, and sends this to the relevant 328 chip via I2C.
It will also control the camera and flash - triggering the flash after the drop sequence has completed on the slaves.

So far, I have been reasonably successful, in that I have my timing data input correctly and displayed on a single 20x4 LCD, and I have been able to send the appropriate data to one of the 328 chips.

This is where the problems start.

The slave seems to receive the correct data, but the 'on/off' cycles of the valves (I'm using LEDs as stand ins) is not working correctly.

I want the valve to open and close three times, with a pause between each drop - which can set on the master.

However, I am getting five 'pulses'....not three.
And consequently the sequence is taking too long, so the flash goes out of sync.

I have stared and stared at the IDE, and I cannot for the life of me work out why it's doing that!

The slave sketch is very simple....just a series of digitalWrites to set the valve pins HIGH or LOW

Here is the master sketch:
(It's in two parts due to the 9000 character limit)

//Six pot, three valve drop controller
//MASTER


#include <digitalIOPerformance.h>
#include <Wire.h>
#include <LiquidCrystal_I2C.h>
LiquidCrystal_I2C lcd(0x27, 20, 4);

byte customChar1[8] = {
  0b10100,
  0b01000,
  0b00000,
  0b00100,
  0b01100,
  0b00100,
  0b00100,
  0b00100
};

byte customChar2[8] = {
  0b10100,
  0b01000,
  0b00000,
  0b01110,
  0b00010,
  0b00100,
  0b01000,
  0b01110
};

byte customChar3[8] = {
  0b10100,
  0b01000,
  0b00000,
  0b01110,
  0b00010,
  0b00110,
  0b00010,
  0b01110
};

byte customChar4[8] = {
  0b11100,
  0b10000,
  0b11000,
  0b10001,
  0b10010,
  0b00100,
  0b01000,
  0b10000
};

byte customChar5[8] = {
  0b00100,
  0b01000,
  0b10000,
  0b00111,
  0b00101,
  0b00111,
  0b00100,
  0b00100
};

#define button1 2
#define button2 3
#define button3 4
#define pot1 A0
#define pot2 A1
#define pot3 A2
#define pot4 A3
#define pot5 A6
#define pot6 A7

#define startButton 5
#define flash 6
#define camera 7
#define trigger 9

int pot1Array[3];
int pot2Array[3];
int pot3Array[3];
int pot4Array[3];
int pot5Array[3];
int pot6Array[3];

int button1state = HIGH;
int button2state = HIGH;
int button3state = HIGH;
int startButtonState = HIGH;

void setup() {

  Serial.begin(9600);
  Wire.begin();

  pinMode(button1, INPUT_PULLUP);
  pinMode(button2, INPUT_PULLUP);
  pinMode(button3, INPUT_PULLUP);
  pinMode(startButton, INPUT_PULLUP);
  pinMode(flash, OUTPUT);
  pinMode(camera, OUTPUT);
  pinMode(trigger, OUTPUT);

  digitalWrite(flash, LOW);
  digitalWrite(camera, LOW);
  digitalWrite(trigger, LOW);

  lcd.init();
  lcd.backlight();
  lcd.begin(20, 4);
  lcd.clear();

  lcd.createChar(0, customChar1);
  lcd.createChar(1, customChar2);
  lcd.createChar(2, customChar3);
  lcd.createChar(3, customChar4);
  lcd.createChar(4, customChar5);

  lcd.setCursor(2, 0);
  lcd.write((uint8_t)3);
  lcd.setCursor(3, 0);
  lcd.write((uint8_t)4);
  lcd.setCursor(4, 0);
  lcd.print(" D1 P1 D2 P2 D3 ");
  lcd.setCursor(0, 1);
  lcd.write((uint8_t)0);
  lcd.setCursor(0, 2);
  lcd.write((uint8_t)1);
  lcd.setCursor(0, 3);
  lcd.write((uint8_t)2);
}

void loop() {

  int pot1Val = map(analogRead(pot1), 0, 1023, 0, 99);
  int pot2Val = map(analogRead(pot2), 0, 1023, 0, 99);
  int pot3Val = map(analogRead(pot3), 0, 1023, 0, 99);
  int pot4Val = map(analogRead(pot4), 0, 1023, 0, 99);
  int pot5Val = map(analogRead(pot5), 0, 1023, 0, 99);
  int pot6Val = map(analogRead(pot6), 0, 1023, 0, 99);

  int valve1Total = (pot2Array[0] + pot3Array[0] + pot4Array[0] + pot5Array[0] + pot6Array[0]);
  int valve2Total = (pot1Array[1] + pot2Array[1] + pot3Array[1] + pot4Array[1] + pot5Array[1] + pot6Array[1]);
  int valve3Total = (pot1Array[2] + pot2Array[2] + pot3Array[2] + pot4Array[2] + pot5Array[2] + pot6Array[2]);

  int totalTime = max(valve1Total, max(valve2Total, valve3Total));

  startButtonState = digitalRead(startButton);
  if (startButtonState == LOW) {

    Wire.beginTransmission(8);
    Wire.write(pot2Array[0]);
    Wire.write(pot3Array[0]);
    Wire.write(pot4Array[0]);
    Wire.write(pot5Array[0]);
    Wire.write(pot6Array[0]);
    Wire.endTransmission();

    Wire.beginTransmission(9);
    Wire.write(pot1Array[1]);
    Wire.write(pot2Array[1]);
    Wire.write(pot3Array[1]);
    Wire.write(pot4Array[1]);
    Wire.write(pot5Array[1]);
    Wire.write(pot6Array[1]);
    Wire.endTransmission();

    Wire.beginTransmission(10);
    Wire.write(pot1Array[2]);
    Wire.write(pot2Array[2]);
    Wire.write(pot3Array[2]);
    Wire.write(pot4Array[2]);
    Wire.write(pot5Array[2]);
    Wire.write(pot6Array[2]);
    Wire.endTransmission();

    digitalWrite(camera, HIGH);
    delay(500);
    digitalWrite(trigger, LOW);
    delay(totalTime + pot1Array[0]);
    digitalWrite(flash, HIGH);
    digitalWrite(trigger, HIGH);
    delay(50);
    digitalWrite(flash, LOW);
    delay(500);
    digitalWrite(camera, LOW);

  }
  else {
    digitalWrite(flash, LOW);
    digitalWrite(camera, LOW);
    digitalWrite(trigger, HIGH);
  }

  //F/P

  lcd.setCursor(2, 1);
  lcd.print("  ");
  lcd.setCursor(2, 1);
  lcd.print(pot1Array[0]);
  //delay(10);

  lcd.setCursor(2, 2);
  lcd.print("  ");
  lcd.setCursor(2, 2);
  lcd.print(pot1Array[1]);
  //delay(10);

  lcd.setCursor(2, 3);
  lcd.print("  ");
  lcd.setCursor(2, 3);
  lcd.print(pot1Array[2]);
  //delay(10);

  //D1

  lcd.setCursor(5, 1);
  lcd.print("  ");
  lcd.setCursor(5, 1);
  lcd.print(pot2Array[0]);
  //delay(10);

  lcd.setCursor(5, 2);
  lcd.print("  ");
  lcd.setCursor(5, 2);
  lcd.print(pot2Array[1]);
  //delay(10);

  lcd.setCursor(5, 3);
  lcd.print("  ");
  lcd.setCursor(5, 3);
  lcd.print(pot2Array[2]);
  //delay(10);

  //P1

  lcd.setCursor(8, 1);
  lcd.print("  ");
  lcd.setCursor(8, 1);
  lcd.print(pot3Array[0]);
  //delay(10);

  lcd.setCursor(8, 2);
  lcd.print("  ");
  lcd.setCursor(8, 2);
  lcd.print(pot3Array[1]);
  //delay(10);

  lcd.setCursor(8, 3);
  lcd.print("  ");
  lcd.setCursor(8, 3);
  lcd.print(pot3Array[2]);
  //delay(10);

  //D2

  lcd.setCursor(11, 1);
  lcd.print("  ");
  lcd.setCursor(11, 1);
  lcd.print(pot4Array[0]);
  //delay(10);

  lcd.setCursor(11, 2);
  lcd.print("  ");
  lcd.setCursor(11, 2);
  lcd.print(pot4Array[1]);
  //delay(10);

  lcd.setCursor(11, 3);
  lcd.print("  ");
  lcd.setCursor(11, 3);
  lcd.print(pot4Array[2]);
  //delay(10);

  //P2

  lcd.setCursor(14, 1);
  lcd.print("  ");
  lcd.setCursor(14, 1);
  lcd.print(pot5Array[0]);
  //delay(10);

  lcd.setCursor(14, 2);
  lcd.print("  ");
  lcd.setCursor(14, 2);
  lcd.print(pot5Array[1]);
  //delay(10);

  lcd.setCursor(14, 3);
  lcd.print("  ");
  lcd.setCursor(14, 3);
  lcd.print(pot5Array[2]);
  //delay(10);

  //D3

  lcd.setCursor(17, 1);
  lcd.print("  ");
  lcd.setCursor(17, 1);
  lcd.print(pot6Array[0]);
  //delay(10);

  lcd.setCursor(17, 2);
  lcd.print("  ");
  lcd.setCursor(17, 2);
  lcd.print(pot6Array[1]);
  //delay(10);

  lcd.setCursor(17, 3);
  lcd.print("  ");
  lcd.setCursor(17, 3);
  lcd.print(pot6Array[2]);
  //delay(10);
2

....and now the remaining bit....

  /////////button1/////////////////////////

  button1state = digitalRead(button1);
  if (button1state == LOW) {

    digitalWrite(button1, LOW);
    digitalWrite(button2, HIGH);
    digitalWrite(button3, HIGH);

    lcd.setCursor(19, 1);
    lcd.print("<");
    lcd.setCursor(19, 2);
    lcd.print(" ");
    lcd.setCursor(19, 3);
    lcd.print(" ");
    delay(10);

    pot1Array[0] = pot1Val;
    pot2Array[0] = pot2Val;
    pot3Array[0] = pot3Val;
    pot4Array[0] = pot4Val;
    pot5Array[0] = pot5Val;
    pot6Array[0] = pot6Val;
  }

  /////////button2/////////////////////////

  button2state = digitalRead(button2);
  if (button2state == LOW) {

    digitalWrite(button1, HIGH);
    digitalWrite(button2, LOW);
    digitalWrite(button3, HIGH);

    lcd.setCursor(19, 1);
    lcd.print(" ");
    lcd.setCursor(19, 2);
    lcd.print("<");
    lcd.setCursor(19, 3);
    lcd.print(" ");
    delay(10);

    pot1Array[1] = pot1Val;
    pot2Array[1] = pot2Val;
    pot3Array[1] = pot3Val;
    pot4Array[1] = pot4Val;
    pot5Array[1] = pot5Val;
    pot6Array[1] = pot6Val;
  }

  /////////button3/////////////////////////

  button3state = digitalRead(button3);
  if (button3state == LOW) {

    digitalWrite(button1, HIGH);
    digitalWrite(button2, HIGH);
    digitalWrite(button3, LOW);

    lcd.setCursor(19, 1);
    lcd.print(" ");
    lcd.setCursor(19, 2);
    lcd.print(" ");
    lcd.setCursor(19, 3);
    lcd.print("<");
    delay(10);

    pot1Array[2] = pot1Val;
    pot2Array[2] = pot2Val;
    pot3Array[2] = pot3Val;
    pot4Array[2] = pot4Val;
    pot5Array[2] = pot5Val;
    pot6Array[2] = pot6Val;
  }
}

And now the slave sketch:

//Six pot, three valve drop controller
//Slave #1
//Valve 1
//Address - 8

#include <digitalIOPerformance.h>
#include <Wire.h>

#define trigger 9
#define valve1 2

int triggerState = HIGH;
int valveTimes[5];

void setup() {

  Serial.begin(9600);
  Wire.begin(8);

  pinMode(trigger, INPUT_PULLUP);
  pinMode(valve1, OUTPUT);
  digitalWrite(valve1, LOW);
  Wire.onReceive(receiveEvent);
}

void loop() {



  triggerState = digitalRead(trigger);
  if (triggerState == LOW) {

    digitalWrite(valve1, HIGH);
    delay(valveTimes[0]);
    digitalWrite(valve1, LOW);
    delay(valveTimes[1]);
    digitalWrite(valve1, HIGH);
    delay(valveTimes[2]);
    digitalWrite(valve1, LOW);
    delay(valveTimes[3]);
    digitalWrite(valve1, HIGH);
    delay(valveTimes[4]);
    digitalWrite(valve1, LOW);
  }
  else {
    digitalWrite(valve1, LOW);
  }
}

void receiveEvent(int howMany) {
  for (howMany; howMany > 0; howMany--) {
    valveTimes[howMany - 1] = Wire.read();
    Serial.print(valveTimes[howMany - 1]);
    if (howMany != 1) {
      Serial.print(",");
    }
    else {
      Serial.println();
    }
  }
}
3

I have prepared some images of the LCD, serial monitor and a scope trace of the valve pin on the slave, they are as follows:

First, the LCD screen showing the timing data as entered on the master via potentiometers.
(I'm testing with only one valve/LED, and the first value - labelled F/P - is the flash delay which I have left at zero)

This translates to:

  • D1 = 1st drop size (valve open for 99 milliseconds)
  • P1 = 1st pause between 1st and 2nd drops (in milliseconds)
  • D2 = 2nd drop size (as 1st drop size)
  • P2 = 2nd pause between 2nd and 3rd drops (milliseconds)
  • D3 = 3rd drop size

Here is a screen capture of the serial monitor for the slave Arduino:

So it appears like the data is being sent successfully.

And finally a scope image of the ONs and OFFs of the valve pin on the slave Arduino:

This is what is causing me much stress!

Why are there five pulses?
And why is the middle one twice as long as the others?

The four shorter pulses do at least seem to be the correct length, but I don't know why there are five and not three as there should be!

I'm still quite the novice regarding Arduino and certainly I'm not knowlegable with the I2C protocol, so please forgive my naive coding style!

I'm sure it's glaringly obvious what the problem is, but I'm just not seeing it....please could some kind person nudge me in the right direction!

Many thanks!

4

Hello.

I don't think you are getting 5 pulses, I think you are getting 6 with the beginning of the 4th being right after the end of the 3rd.

In your slave code what happens at the end of loop? Right after:

    delay(valveTimes[4]);
    digitalWrite(valve1, LOW);

How long does triggerState stay low for?

You need to detect that triggerState has become low, not that it is low.

As an aside, I can't understand why you have so many Arduino's doing what 1 would do perfectly well.

5

Thanks for the reply Perry.

I don't think you are getting 5 pulses, I think you are getting 6 with the beginning of the 4th being right after the end of the 3rd.

I believe you're onto something there....that does make sense.
So, the whole drop sequence is cycling twice with effectively no delay between them?

That's certainly something I can work with, although I'm not quite sure I understand how to differentiate between 'becomes LOW' and 'is LOW'.
Please could you elaborate.

I see the 'else' statement immediately after the section you're referring to....should I simply set 'triggerState' to LOW here also?

As an aside, I can't understand why you have so many Arduino's doing what 1 would do perfectly well.

The answer to that is twofold:

  • 1, I have indeed tried to implement all the valve operations on a single Arduino, but I managed to tie myself up in many technical knots, owing to my lack of knowledge.
    So I surmised that I could perhaps make it simpler...'sketch-wise' to work with the master/slave system.

  • 2, There would be times when I am using the device, that I would like to dispense drops at (as close to) exactly the same time as possible.
    I realise that this could be achieved with a single Arduino too, but I refer you to point 1 above....I simply don't know enough yet!

Thanks for taking the time to respond Perry, most appreciated.

6

I2C communication is a bit complicated. So I recommend you to use SPI or UART.

7

So, the whole drop sequence is cycling twice with effectively no delay between them?

Correct.

I need you to think about this question, it is fundamental to your problem and to how any computer program works:

In your slave code what happens at the end of loop?

I see the 'else' statement immediately after the section you're referring to....should I simply set 'triggerState' to LOW here also?

First think about what happens at the end of loop and come back with an answer, then consider what the consequences would be if you did as you suggest. Try it, see what happens. The else is not really needed, although having it isn't causing any problem.

I would say that doing this properly on one Arduino is no more difficult than learning to use I2C, well at least you know how to use I2C now, that will be useful to you.

There are several things you need to learn, I am trying to lead you to them in the order you need them.

[Edit]
This question:

How long does triggerState stay low for?

Should be: How long does trigger stay low? I have confused trigger and triggerState, but the question is basically the same.

8

Something is not correct in the following ISR routine. Remove the print() methods in the loop() function as interrupt remains disabled in ISR; but, print() methods works on interrupt. Set a flag in ISR and then use that flag in the loop() function to carry out print activities.

void receiveEvent(int howMany) {
  for (howMany; howMany > 0; howMany--) {
    valveTimes[howMany - 1] = Wire.read();
    Serial.print(valveTimes[howMany - 1]);
    if (howMany != 1) {
      Serial.print(",");
    }
    else {
      Serial.println();
    }
  }
}
9

ArnavPawarAA:
I2C communication is a bit complicated. So I recommend you to use SPI or UART.

Thanks for your input, you're probably right, although I have invested time in learning I2C now, and I think I'm getting to grips with it slowly.
I fear it may be too late for me to turn back and begin learning other methods, so for better or worse I'm stuck with I2C!
For my very simple requirements, it seems to be quite straightforward, plus I really like that I only need two wires!

GolamMostafa:
Something is not correct in the following ISR routine. Remove the print() methods in the loop() function as interrupt remains disabled in ISR; but, print() methods works on interrupt. Set a flag in ISR and then use that flag in the loop() function to carry out print activities.

Thanks for taking the time to respond.
I have to admit to a great lack of knowledge here, because I'm not quite sure what you mean.
I think I understand that Serial.print() statements are ill advised within ISRs, but I am unfamiliar with the concept of 'setting' flags etc.

I will comment out the Serial.print()() statements, since they were only in there to monitor and verify that the values were being transmitted successfully.
But other than that, the void receiveEvent() function seems to work well.

PerryBebbington:
First think about what happens at the end of loop and come back with an answer....

Thank you Perry, this really made me take a step back and I saw things clearly!
Indeed I saw that it was looping twice because the master loop() was holding the trigger LOW for too long, so it just looped again....which is entirely expected.
It was in fact holding the trigger pin LOW for the entire duration of the drop sequence, when I really only needed a brief pulse.

So I modified the master code to set the trigger pin LOW for just 50 milliseconds and then setting it HIGH again, and now it works perfectly!!
I now see three beautiful pulses on my scope!!

PerryBebbington:
How long does trigger stay low?

This was an incredibly helpful hint, thanks!

PerryBebbington:
I would say that doing this properly on one Arduino is no more difficult than learning to use I2C, well at least you know how to use I2C now, that will be useful to you.

I would really appreciate any insight into how I would achieve this, since I very nearly went insane trying to do it!

I managed to get a rough structure, but it was allowing for independent control of three separate valves with full timing control for three drops each that really proved impossible for me.

I have built two drop controllers previously: one which dispensed two timed drops from a single valve, and another that dispensed one drop each from two separate valves.
Both were challenging, and I know that my codes were very simplistic and naive, but they did work.

I feared that I may have bitten off more that I could chew when I set myself the challenge of building a controller that could operate three fully independent valves, each with fully controllable drop sequences, but I thought I could circumvent my shortcomings (knowledge-wise) by breaking it down into manageable chunks viz-a-viz the I2C maste/slave system.

I would dearly love to learn about how to perform the same processes on a single Arduino....maybe one day I will, but learning is difficult if you're an old geezer like me!! :smiley:

10

OK, good.

This:

int triggerState = HIGH;

Should be:

bool triggerState = HIGH;

A bool (boolean*) can have only 2 states, which is all you need, true or false, high or low, 1 or 0.
These are used as flags, so

I am unfamiliar with the concept of 'setting' flags etc.

You need a bool that you can use as a flag; in the ISR you set it to 1, then outside the ISR you use if() to test if it is 1, if it is 1 then you do whatever and set it back to 0.

You need a flag to determine whether trigger has become low or is low, You need to check trigger to see if it low or high, but also you need to save the result in a flag. Next time you check it you check what its current state is and what its previous state was, and only do something if the state has changed. Don't forget to save the new current state in the flag for next time.

Take a step back from your project and follow these tutorials:
Using millis for timing
Demonstration for several things at the same time
Also, in the IDE under examples / digital / blink without delay.

You must stop using delay.

When you understand the above think how to apply them to your project.

Have fun :slight_smile:

*You will hear in common useage something like this: "They only have a binary choice", meaning a choice between 2 things. This is wrong, binary is a number base. What they actually mean is "They only have a Boolean choice".

11

Thanks again for your patient guidance Perry.

It's clear that I have an immense learning curve ahead of me, and I do really appreciate the helpful hints you have given me with regard to my current project.

Perry:
You must stop using delay.

You are quite right.

It always seems strange to me that so many areas of education, begin with the 'wrong' way to do things (if only to serve as a confidence boost), and then further learning involves 'un-learning' those methods and learning 'better' ones!

Still, I shall heed your sound advice, and postpone my project, and force myself to fully grasp the millis() concept - I am already vaguely familiar with it, but as with all novices, it seems very abstract to me at the moment.
As does the concept of interrupt routines, but again, I shall buckle down and learn....I have just had a couple of decent beginner books delivered (Author - Simon Monk) so I will get busy reading them.

Those links you posted are a very good resource for beginners also, so I thank you again for posting them.

Cheers!