Can't find the bug.

So, I’m building a project where I have an Arduino Ethernet being used as master and 2 328P being used as slaves.

When the Arduino Ethernet sends through the serial the sequence “00001”, it’s a request for the slave number 1, same thing for the number 2.

The slaves are basically counting pulses, and when they get a request they should use the serial port to send the counter value, which will be send to a site named Cosm, who is going to draw a graphic, based on the data received. The system is working correctly, but SOMETIMES, with no apparent reason I get a bug, which I will try to explain at my best.

Bug explanation:

Let’s say the slave 1 is with the counter on “05893”, when requested, that’s what it will be send to the master, but SOMETIMES, what we can see on Cosm is “58933”, and I can’t see how that’s a software bug, because there is no pattern. Can’t see how it’s hardware too, since there is no sense.

Master code:

#include <SPI.h>
#include <Ethernet.h>
#include <StopWatch.h>
#include <stdio.h>
#include <string.h>
#define APIKEY         "0mHaYddddIze8rsoLssddfsfDw3VWZ2TZ3-SbKwsddfs1QXpXNdsfsfd2TsjW9tVfgT0g" // replace your Cosm api key here
#define FEEDID         72433 // replace your feed ID
#define USERAGENT      "Temperatura" // user agent is the project name
byte mac[] = {0xDE, 0xAD, 0xBE, 0xEF, 0xFE, 0xED};
IPAddress ip(192,168,1,20);
EthernetClient client;

unsigned long lastConnectionTime = 0;
boolean lastConnected = false;
const unsigned long postingInterval = 10*1000;
char receiver[5];
char buffer;
int counter = 1;

boolean wassend = false, setupcompleted = false;
int i;
unsigned long tempo;
StopWatch sw_secs;


void setup(){
  pinMode(6,OUTPUT);
  pinMode(5,OUTPUT);
  pinMode(8,OUTPUT);
   Serial.begin(9600);
   delay(2000);
   if (Ethernet.begin(mac) == 0){
     Ethernet.begin(mac,ip);
   }else{
     digitalWrite(5,HIGH);//green
   }
   delay(2000);
   Serial.print('s');
   delay(200);
}

void loop(){  
    if(wassend == false){
      if(counter==3) counter=1;
      Serial.print("0000");
      Serial.println(counter);
      wassend = true;
      sw_secs.reset();
      sw_secs.start();
    }
    
      if(Serial.available() > 0){
        sw_secs.stop();
        sw_secs.reset();
        buffer = Serial.read();
        receiver[i] = buffer;
        i++;
    
    }
    if(receiver[4] > 0){
      wassend = false;
      String dataString = "Hidro";
      dataString = dataString+(counter);
      dataString = dataString+',';
      dataString += receiver;
      counter++;
       sendData(dataString);
       delay(1000);
       sendData(dataString);
      for(int x=0;x<5;x++) receiver[x] = 0;
      i=0;
    }
    if(sw_secs.elapsed() > 5000){
      //IPAddress server(212,1,208,179);
      if (client.connect("www.mysite.com", 80)){
        client.print("GET /?status=Failure&strbuildingnumber=");
        client.print(FEEDID);
        client.println(" HTTP/1.0");
        client.println();
     }else{
       digitalWrite(6,HIGH);//red
       delay(100);
       digitalWrite(6,LOW);//red     
     }
     sw_secs.reset();
     counter++;
     if(counter==3) counter=1;
      wassend=0;
    }
}
void sendData(String thisData) {
   char server[] = "api.cosm.com";
   // if there's a successful connection:
   if (client.connect(server, 80)) {
     //Serial.println("client connected");/////////////////////////////////
     // send the HTTP PUT request:
     client.print("PUT /v2/feeds/");
     client.print(FEEDID);
     client.println(".csv HTTP/1.1");
     client.println("Host: api.cosm.com");
     client.print("X-ApiKey: ");
     client.println(APIKEY);
     client.print("User-Agent: ");
     client.println(USERAGENT);
     client.print("Content-Length: ");
     client.println(thisData.length()-1);

     // last pieces of the HTTP PUT request:
     client.println("Content-Type: text/csv");
     client.println("Connection: close");
     client.println();

     // here's the actual content of the PUT request:
     client.println(thisData);
     //Serial.println("Enviado");//////////////////////////////////////
     digitalWrite(8,HIGH);
     delay(100);
     digitalWrite(8,LOW);
   } 
   else {
     // if you couldn't make a connection:
     //Serial.println("Conexao nao feita.");////////////////////////////
     client.stop();
   }
   // note the time that the connection was made or attempted:
   lastConnectionTime = millis();
}

Slave code:

#include <stdio.h>
#include <string.h>
#define pin2 0 //MCLR
#define pin3 1 //SERIAL
#define pin4 2 //SERIAL
#define pin5 3 //PWM
#define pin6 4
#define pin11 5 //PWM
#define pin12 6 //PWM
#define pin13 7
#define pin14 8
#define pin15 9 //PWM
#define pin16 10 //PWM
#define pin17 11 //PWM
#define pin18 12
#define pin19 13
#define pin23 A0 //analog
#define pin24 A1 //analog
#define pin25 A2 //analog
#define pin26 A3 //analog
#define pin27 A4 //analog
#define pin28 A5 //analog
#define TRUE 1
#define FALSE 0
char receiver[] = "00000";
char address[] = "00001";
char counter[] = "00000";
char buffer;
int i=1,pulsos = 0;
boolean flag = false;
void setup(){ 
  pinMode(pin19, INPUT);
  Serial.begin(9600);
  attachInterrupt(0,incrementa, RISING);
}

void loop(){
   if(Serial.available() > 0){
   buffer = Serial.read();
   receiver[i] = buffer;
   i++;
   if(receiver[4] > 0){
     i = 0;
     if(strcmp(receiver,address) == FALSE){
       Serial.print(counter);
       flag = true;
     }
     for(int x=0;x<5;x++){
       receiver[x] = 0;
     }     
   }
 }
}
void incrementa(){
       if(++pulsos == 10){
       pulsos = 0;
       counter[4]++;
       if(counter[4] == 58){
         counter[4]='0';
         counter[3]++;
         if(counter[3] == 58){
           counter[3]='0';
           counter[2]++;
           if(counter[2] == 58){
             counter[2]='0';
             counter[1]++;
             if(counter[1] == 58){
               counter[1]='0';
               counter[0]++;
             }
           }
         }
       }
     }
 }

My initial reaction is to rewrite without using the String class. Due to a bug in free() (used by String) the memory allocated may become corrupted.

WIll take a look at that and reply back, thank you.

Just the part changed on the code:

    if(receiver[4] > 0){
      wassend = false;
      //String dataString = "Hidro";
      //dataString = dataString+(counter);
      //dataString = dataString+',';
      //dataString += receiver;
      
      char dataString[12] = "Hidro";
      dataString[5] = counter + 48;
      dataString[6] = ',';
      for(int y = 7;y < 12; y ++) dataString[y] = receiver[y - 7];
      counter++;
       sendData(dataString);
       delay(1000);
       sendData(dataString);
      for(int x=0;x<5;x++) receiver[x] = 0;
      i=0;
    }

Do you mean like this?

All of it, I mean. eg.

void sendData(String thisData) {

And in what you did change, you need a null terminator.

      dataString[5] = counter + 48;

counter is an array of chars. It makes no sense to add 48 to it. It makes no sense to try to store (incorrectly) the array of 5 chars into one element of dataString.

counter, by itself, is an address. Adding 48 to an address does not do what you think it does. Storing an int (the address is an int) in a char element does not make sense.

None of this applies if you have a local variable named counter, too, of a different type. But, of course, that would be dumb, so you wouldn't do that. Would you?

     if(strcmp(receiver,address) == FALSE){

strcmp() returns +1, 0, or -1, depending on the lexical position of the two strings. It does NOT return FALSE, or false.

On the master 'counter' is an integer.

strcmp return 0, if the two strings are the same. Since I'm using '#define FALSE 0' there is no problem

Since I'm using '#define FALSE 0' there is no problem

Really?

     if(strcmp(receiver,address) == FALSE)
{
   // The strings are the same
}

Way too confusing for me.

    if(strcmp(receiver,address) == FALSE)

Your code looks like the comparison is false. The code underneath is executed if it compares equal. I agree with PaulS here. This is a recipe for confusion. You may as well say:

    if(strcmp(receiver,address) == LOW)

or:

    if(strcmp(receiver,address) == INPUT)

or:

    if(strcmp(receiver,address) == LSBFIRST)

They are all defined as 0 too. It's just silly to do that, sorry.