Problem: UDP Wraped with If Statement

Hi,

I have a few sensors that I'm trying to send the data from them onto a server using UDP protocol. As soon as I use If Statement ( if(e) ) with my UDP, it stops working. It makes my sensors to stop working as well. Has anyone encountered the same problem?

#include <SPI.h>
#include <Ethernet.h>
#include <EthernetUdp.h>
#include <MuxShield.h>

int IO1DigitalVals[16];

//Initialize the Mux Shield
MuxShield muxShield;

byte arduinoMac[] = { 0xDE, 0xAD, 0xBE, 0xEF, 0xFE, 0xED };
IPAddress arduinoIP(192, 168, 1, 192); // desired IP for Arduino
unsigned int arduinoPort = 8888;      // port of Arduino

IPAddress receiverIP(192, 168, 1, 135); // IP of udp packets receiver
unsigned int receiverPort = 6000;      // port to listen on my PC

EthernetUDP Udp;

void setup() {
  Ethernet.begin(arduinoMac,arduinoIP);
  Udp.begin(arduinoPort);

  muxShield.setMode(1,DIGITAL_IN_PULLUP);
 
  for (int i=0; i<16; i++) {
    IO1DigitalVals[i] = 1;
  }

  Serial.begin(9600);
}

void loop() {
  int e;
  
  e = 0;
  for (int i=0; i < 16; i++) {
    int v1 = muxShield.digitalReadMS(1, i);

    IO1DigitalVals[i] = v1;

    if (v1 == 0) {
      e = 1;
    }
  }

  if(e) {
    Udp.beginPacket(receiverIP, receiverPort);
    Udp.write("Yes"); 
    Udp.endPacket();
  }
}

What do you mean by 'stops working' ?
What do you see if you print the value of e just after the test of the value of v1 inside the for loop, and before the if after the for loop ?

Note that e will always be 1 if the 16th reading of v1 is zero no matter what the previous 15 readings were. Is that what you intended ?

UKHeliBob:
What do you mean by 'stops working' ?
What do you see if you print the value of e just after the test of the value of v1 inside the for loop, and before the if after the for loop ?

Note that e will always be 1 if the 16th reading of v1 is zero no matter what the previous 15 readings were. Is that what you intended ?

The sensors are infrared sensors. It works once and after it goes through the loop it stops working!

Take a look at the following code. It's more readable:

#include <SPI.h>
#include <Ethernet.h>
#include <EthernetUdp.h>
#include <MuxShield.h>

int IO1DigitalValues[16];
int IO2DigitalValues[16];
int IO3DigitalValues[16];

byte dataBuff[16];

//Initialize the Mux Shield
MuxShield muxShield;

byte arduinoMac[] = { 0xDE, 0xAD, 0xBE, 0xEF, 0xFE, 0xED };
IPAddress arduinoIP(192, 168, 1, 192);  // desired IP for Arduino
unsigned int arduinoPort = 8888;        // port of Arduino

IPAddress receiverIP(192, 168, 1, 135); // IP of udp packets receiver
unsigned int receiverPort = 6000;       // port to listen on my PC

EthernetUDP Udp;

void setup() {
  Ethernet.begin(arduinoMac,arduinoIP);
  Udp.begin(arduinoPort);

  muxShield.setMode(1,DIGITAL_IN_PULLUP);  //use DIGITAL_IN in place of DIGITAL_IN_PULLUP if internal pullups are not needed
  muxShield.setMode(2,DIGITAL_IN_PULLUP);
  muxShield.setMode(3,DIGITAL_IN_PULLUP);

  memset(IO1DigitalValues, 0, sizeof(IO1DigitalValues));
  memset(IO2DigitalValues, 0, sizeof(IO2DigitalValues));
  memset(IO3DigitalValues, 0, sizeof(IO3DigitalValues));

  Serial.begin(9600);
}

void loop() {
  int dataChanged = readData();
  if (dataChanged) {
    ProcessData();
  }
}

int readData() {
  int dataChanged = 0;
  for (int i=0; i < 16; i++) {
    int v1 = muxShield.digitalReadMS(1, i);
    int v2 = muxShield.digitalReadMS(2, i);
    int v3 = muxShield.digitalReadMS(3, i);

    if ( (v1 != IO1DigitalValues[i]) || (v2 != IO2DigitalValues[i]) || (v3 != IO3DigitalValues[i]) ) {
     dataChanged = 1;

     IO1DigitalValues[i] = v1;
     IO2DigitalValues[i] = v2;
     IO3DigitalValues[i] = v3;
    }
  }
  return dataChanged;
}

void ProcessData() {
  for(int i=0; i<16; i++) {
    dataBuff[i] = (char) IO1DigitalValues[i];
  }

  Udp.beginPacket(receiverIP, receiverPort);
  Udp.write(dataBuff, sizeof(dataBuff));
  Udp.endPacket();
}

The sensors are infrared sensors. It works once and after it goes through the loop it stops working!

No, it doesn't. It simply doesn't do what you expect. So, clearly your expectations are wrong.

Where are your debugging statements?

PaulS:

The sensors are infrared sensors. It works once and after it goes through the loop it stops working!

No, it doesn't. It simply doesn't do what you expect. So, clearly your expectations are wrong.

I want the Udp.packets to be written only when the status of my sensors change. That's why I'm using an if statement. This is my expectation.

So, you have some expectations. You have some code that does something. What it does isn't exactly clear. What is clear is that what it does isn't in line with your expectations.

So, I'll ask again. Where are your debug statements? Where is your debug output?

PaulS:
So, you have some expectations. You have some code that does something. What it does isn't exactly clear. What is clear is that what it does isn't in line with your expectations.

So, I'll ask again. Where are your debug statements? Where is your debug output?

Here's the code (written a bit differently) but with the debugging statements. This only prints to console once and doesn't send any UDP packets. After printing once to the console it kinda exits the loop (!):

#include <SPI.h>
#include <Ethernet.h>
#include <EthernetUdp.h>
#include <MuxShield.h>

int IO1DigitalVals[16];
int IO2DigitalVals[16];
int IO3DigitalVals[16];

byte valueInBytes[16];

//Initialize the Mux Shield
MuxShield muxShield;

byte arduinoMac[] = { 0xDE, 0xAD, 0xBE, 0xEF, 0xFE, 0xED };
IPAddress arduinoIP(192, 168, 1, 192); // desired IP for Arduino
unsigned int arduinoPort = 8888;      // port of Arduino

IPAddress receiverIP(192, 168, 1, 135); // IP of udp packets receiver
unsigned int receiverPort = 6000;      // port to listen on my PC

EthernetUDP Udp;


void setup() {
  Ethernet.begin(arduinoMac,arduinoIP);
  Udp.begin(arduinoPort);

  muxShield.setMode(1,DIGITAL_IN_PULLUP);  //use DIGITAL_IN in place of DIGITAL_IN_PULLUP if internal pullups are not needed
  muxShield.setMode(2,DIGITAL_IN_PULLUP);
  muxShield.setMode(3,DIGITAL_IN_PULLUP);

  for (int i=0; i<16; i++) {
    IO1DigitalVals[i] = 1;
    IO2DigitalVals[i] = 1;
    IO3DigitalVals[i] = 1;
  }

  Serial.begin(9600);
}

void loop() {
  int evt;
 
  evt = 0;

 
  for (int i=0; i < 16; i++) {
    //Digital read on all 16 inputs on IO1, IO2, and IO3
   
    int v1 = muxShield.digitalReadMS(1, i);
    int v2 = muxShield.digitalReadMS(2, i);
    int v3 = muxShield.digitalReadMS(3, i);
   
    if ((v1 != IO1DigitalVals[i]) || (v2 != IO2DigitalVals[i]) || (v3 != IO3DigitalVals[i])) {
      IO1DigitalVals[i] = v1;
      IO2DigitalVals[i] = v2;
      IO3DigitalVals[i] = v3;
     
      evt = 1;
    }
  }
  if (evt) {  
    for(int i=0;i<16;i++) {
      valueInBytes[i] = (char) IO1DigitalVals[i];
     
      if (IO1DigitalVals[i] == 0) {
        Serial.print("port 1 \n");
      }
      if (IO2DigitalVals[i] == 0) {
        Serial.print("port 2 \n");
      }
      if (IO3DigitalVals[i] == 0) {
        Serial.print("port 3 \n");
      }
    }
    Udp.beginPacket(receiverIP, receiverPort); //start udp packet
    Udp.write(valueInBytes, 16); //write sensor data to udp packet
    Udp.endPacket(); // end packet
  }

}

Here's the code (written a bit differently) but with the debugging statements.

Where?

What are you reading from the mux shield? Unless you KNOW that, you are just guessing that the rest of the code is doing what you want. I gave up on guessing what code does my second week as a coder. I strongly suggest that you do, too.

I agree with PaulS. I also gave up guessing shortly after starting to program. That was in 1983. Note that number is not the correct format for a military 24 hour time. :wink:

Add a Serial.prinln call to this if condition like I did below. Does it display "Changed" on the serial monitor when you think it should have changed?

    if ((v1 != IO1DigitalVals[i]) || (v2 != IO2DigitalVals[i]) || (v3 != IO3DigitalVals[i])) {
      IO1DigitalVals[i] = v1;
      IO2DigitalVals[i] = v2;
      IO3DigitalVals[i] = v3;
      evt = 1;

      // Here is a Serial debug message
      Serial.println("Changed"); 
   }

SurferTim:
I agree with PaulS. I also gave up guessing shortly after starting to program. That was in 1983. Note that number is not the correct format for a military 24 hour time. :wink:

Add a Serial.prinln call to this if condition like I did below. Does it display "Changed" on the serial monitor when you think it should have changed?

    if ((v1 != IO1DigitalVals[i]) || (v2 != IO2DigitalVals[i]) || (v3 != IO3DigitalVals[i])) {

IO1DigitalVals[i] = v1;
     IO2DigitalVals[i] = v2;
     IO3DigitalVals[i] = v3;
     evt = 1;

// Here is a Serial debug message
     Serial.println("Changed");
  }

I tried it. Only loops once. Prints out "Changed" once and after that nothing happens! See the attached screen shot.

Prints out "Changed" once and after that nothing happens!

So now you know that this
if ((v1 != IO1DigitalVals[i]) || (v2 != IO2DigitalVals[i]) || (v3 != IO3DigitalVals[i])) is either only true once or is only executed once. So put in some Serial.prints to show whether it is executed more than once and the values of the variables being tested by the statement. Also put some Serial.prints elsewhere in the code to show what code it is actually executing.

Ok guys, I realized where the problem is. I modified the code, put some debugging statements. The code looks like this now:

#include <SPI.h>
#include <Ethernet.h>
#include <EthernetUdp.h>
#include <MuxShield.h>

int IO1DigitalValues[16];
int IO2DigitalValues[16];
int IO3DigitalValues[16];

byte dataBuff[16];

//Initialize the Mux Shield
MuxShield muxShield;

byte arduinoMac[] = { 0xDE, 0xAD, 0xBE, 0xEF, 0xFE, 0xED };
IPAddress arduinoIP(192, 168, 1, 192);  // desired IP for Arduino
unsigned int arduinoPort = 8888;        // port of Arduino

IPAddress receiverIP(192, 168, 1, 135); // IP of udp packets receiver
unsigned int receiverPort = 6000;       // port to listen on my PC

EthernetUDP Udp;

void setup() {
  Ethernet.begin(arduinoMac,arduinoIP);
  Udp.begin(arduinoPort);

  muxShield.setMode(1,DIGITAL_IN_PULLUP);  //use DIGITAL_IN in place of DIGITAL_IN_PULLUP if internal pullups are not needed
  muxShield.setMode(2,DIGITAL_IN_PULLUP);
  muxShield.setMode(3,DIGITAL_IN_PULLUP);

  memset(IO1DigitalValues, 0, sizeof(IO1DigitalValues));
  memset(IO2DigitalValues, 0, sizeof(IO2DigitalValues));
  memset(IO3DigitalValues, 0, sizeof(IO3DigitalValues));
  
  
  Serial.begin(9600);
}

void loop() {
  int dataChanged = readData();
  if (dataChanged) {
    ProcessData();
  }
 
}

int readData() {
  int dataChanged = 0;
  for (int i=0; i<16; i++) {
    IO1DigitalValues[i] = 1;
    IO2DigitalValues[i] = 1;
    IO3DigitalValues[i] = 1; 
    }
  
  for (int i=0; i < 16; i++) {
    int v1 = muxShield.digitalReadMS(1, i);
    int v2 = muxShield.digitalReadMS(2, i);
    int v3 = muxShield.digitalReadMS(3, i);
    //Serial.println(v1);
    //Serial.println(v2);
    //Serial.println(v3);
    if  ((v1 != IO1DigitalValues[i]) || (v2 != IO2DigitalValues[i]) || (v3 != IO3DigitalValues[i])) {
     
     dataChanged = 1;
 
     IO1DigitalValues[i] = v1;
     IO2DigitalValues[i] = v2;
     IO3DigitalValues[i] = v3;
     Serial.println("Changed");
    }
    
  }
  return dataChanged;
}

void ProcessData() {
  Serial.println("Process");
  for(int i=0; i<16; i++) {
    dataBuff[i] = (char) IO1DigitalValues[i];
  }

  Udp.beginPacket(receiverIP, receiverPort);
  Udp.write(dataBuff, sizeof(dataBuff));
  Udp.endPacket();

}

This gets the sensors data fine, prints out both "Changed" and "Process". The loop works fine only if I comment out the three "UDP" Statements. As soon as I put those three lines in my code the loop stops.

P.s. I am using Node.js to receive the UDP packets (but i don't receive anything). Below is my code for that:

var dgram = require("dgram");
var server = dgram.createSocket("udp4");
var fs = require('fs');

var crlf = new Buffer(2);
crlf[0] = 0xD; //CR - Carriage return character
crlf[1] = 0xA; //LF - Line feed character

server.on("message", function (msg, rinfo) {

  console.log(msg.toString());
  fs.appendFile('mydata.txt', msg.toString() + crlf, encoding='utf8');
  return;

  var value = '';
  for(var k=0; k<msg.length ; k++) {
    var v = msg.readUInt8(k);
    if (v != 1) { console.log(k) ;};
  } 
//	console.log(msg.toString());
  return;
 

 var value = msg.readUInt8(2);
 if (value != 1) {
//every time new data arrives do this:
  console.log("server got: " , msg.readUInt8(2) , " from " + rinfo.address + ":" + rinfo.port); // you can comment this line out
  fs.appendFile('mydata.txt', msg.readUInt8(0) + crlf, encoding='utf8');//write the value to file and add CRLF for line break
}
});

server.on("listening", function () {
  var address = server.address();
  console.log("server listening " + address.address + ":" + address.port);
});

server.bind(6000); //listen to udp traffic on port 6000

The loop works fine only if I comment out the three "UDP" Statements. As soon as I put those three lines in my code the loop stops.

The obvious questions, then, are where and why.
The equally obvious (I hope) question is why you are making no effort to determine the answers to the first questions.

We've been telling you that you need to put debug Serial.print() statements in the code. You've, apparently, identified a block of code that causes the Arduino to stop doing useful work. So, which of those three statements causes the problem?

What sized values does muxShield.digitalReadMS() actually return? The values appear to be 0 or 1, so, why do you need to store those in an int? Why do you need, on every pass through loop to store 48 ones on three arrays? You use the 48 elements to determine if v1, v2, and v3 are not the same as the 1 stored in the array. Why not just compare v1, v2, and v3 to 1, and get rid of the 96 wasted bytes?

Finally, a link to the shield you are using, and the library, would be good. I'm beginning to suspect a pin conflict.

We've been telling you that you need to put debug Serial.print() statements in the code. You've, apparently, identified a block of code that causes the Arduino to stop doing useful work. So, which of those three statements causes the problem?

Ok so I added Serial.print() to each line in the UDP code block. The first two are executed but the Udp.endPacket(); is NOT executed! So I guess that's why the loop doesn't work properly and nothing is sent over.

What sized values does muxShield.digitalReadMS() actually return? The values appear to be 0 or 1, so, why do you need to store those in an int? Why do you need, on every pass through loop to store 48 ones on three arrays? You use the 48 elements to determine if v1, v2, and v3 are not the same as the 1 stored in the array. Why not just compare v1, v2, and v3 to 1, and get rid of the 96 wasted bytes?

the muxShield.digitalReadMS() returns 0's and 1's. It returns 0 when an object is detected by the sensor and returns 1 if there's nothing detected. The reason why I created three different arrays is because I have three different rows on my muxShield which increases the number of my digital inputs. Paul, I'm a beginner programmer so that was the only way that came into my mind. And about the storing them in an "int" it was the only way that I thought was correct, so feel free to suggest any other way you think is better for this purpose.

Finally, a link to the shield you are using, and the library, would be good. I'm beginning to suspect a pin conflict.

My Arduino is an Arduino Uno.

Links for the shields:

Muxshield: Mux Shield | Mayhew Labs

Ethernet Shield: Ethernet shield module w5100 micro sd card slot for mega 2560 geekcreit for arduino - products that work with official arduino boards Sale - Banggood.com sold out-arrival notice-arrival notice

this ethernet shield is a cheaper equivalent to the Arduino ethernet shield,

the muxShield.digitalReadMS() returns 0's and 1's. It returns 0 when an object is detected by the sensor and returns 1 if there's nothing detected. The reason why I created three different arrays is because I have three different rows on my muxShield which increases the number of my digital inputs. Paul, I'm a beginner programmer so that was the only way that came into my mind.

The output may be 0 or 1, but to test that the output is 1, you don't need to compare against an array that you set, on every pass, to 1.

The arrays do not have to be ints. The values that you are storing in the array are byte sized, so a byte array would be more appropriate IF THEY WERE NEEDED.

Ok so I added Serial.print() to each line in the UDP code block. The first two are executed but the Udp.endPacket(); is NOT executed!

Is not executed? Or never returns? Big difference.

The MUX shield has an LED on pin 13. Is that LED lighting up? Pin 13 is one of the SPI pins that the Ethernet shield uses.

Do you have a SD card in the ethernet shield's slot? That would cause this kind of problem.

Is not executed? Or never returns? Big difference.

The MUX shield has an LED on pin 13. Is that LED lighting up? Pin 13 is one of the SPI pins that the Ethernet shield uses.

I added an SD card to the ethernet shield and now it is both executed and I receive some smiley faces (which i don't understand why) on my command prompt (where I'm using node). However, the loop still doesn't work. So it only goes through ONCE. After that my sensors get deactivated. By that I mean, my sensors have two LED's on them. One is red which is the power, and one is green which lights up only when it detects an object. After the loop is executed once, the green LED's never light up again unless I restart the shield or unplug the USB cable and plug it back again.

And my MUX shield only has one LED which is the Power LED.

SurferTim:
Do you have a SD card in the ethernet shield's slot? That would cause this kind of problem.

I bought an SD card and used it with the ethernet shield. Now the Udp.endPacket() is executed and I see some smiley faces on my cmd (where I use node.js). However, the loop still doesnt work. This whole thing is executed once, and returns the smiley faces once. After that my sensors get deactivated. By that I mean, my sensors have two LED's on them. One is red which is the power, and one is green which lights up only when it detects an object. After the loop is executed once, the green LED's never light up again unless I restart the shield or unplug the USB cable.

That MUX shield uses D4, and so does the SD card. That will cause SPI bus collisions between the SD card and the W5100. Remove the SD card.

Gentlemen,

I removed the muxShield (the multiplexer) and wrote a similar code with only 2 sensors and I used digital pins 5 and 6. This works just fine. See the code below.

I think the pin conflict is what's causing the problem. Look at the code below. Can you please suggest a solution?

#include <SPI.h>
#include <Ethernet.h>
#include <EthernetUdp.h>

const int pinNumber1=5;
const int pinNumber2=6;

byte dataBuff[2];

int sensorVal1;
int sensorVal2;

byte arduinoMac[] = { 0xDE, 0xAD, 0xBE, 0xEF, 0xFE, 0xED };
IPAddress arduinoIP(192, 168, 1, 192);  // desired IP for Arduino
unsigned int arduinoPort = 8888;        // port of Arduino

IPAddress receiverIP(192, 168, 1, 135); // IP of udp packets receiver
unsigned int receiverPort = 6000;       // port to listen on my PC

EthernetUDP Udp;

void setup() {
  Ethernet.begin(arduinoMac,arduinoIP);
  Udp.begin(arduinoPort);

  
  pinMode(pinNumber1, INPUT);
  pinMode(pinNumber2, INPUT);
  
  Serial.begin(9600);
}

void loop(){
  
   int dataChanged=readData();
   if(dataChanged){
     ProcessData();
   }
   
}


int readData() {
  int dataChanged=0;
  
      sensorVal1=digitalRead(pinNumber1);
      sensorVal2=digitalRead(pinNumber2);

  
  if (sensorVal1 != 1 || sensorVal2 != 1) {
    dataChanged=1;
    } 
  return dataChanged;
}

void ProcessData() {
  Serial.println("Process");
  
    dataBuff[0] = (char) sensorVal1;
    dataBuff[1] = (char) sensorVal2;

  Udp.beginPacket(receiverIP, receiverPort);
  Serial.println("UPD Recieved");
  Udp.write(dataBuff, sizeof(dataBuff));
  Serial.println("UPD Written");
  Udp.endPacket();
  Serial.println("UDP Ended");
}