Problem converting character array into integer in arduino using atoi function

Question

I'm receiving a character string from a radio signal on my arduino. This character string consists of four character digits, which I want to convert into an integer. The four digit sequence may contain zeros padded in front of the integer, such as '0025' to represent the integer 25.

So, I saved this character string onto a character array of length 5:

char left[5];

The first four characters in this array have the four character digits received in the radio signal. I have a fifth character which I set to the character '\0' to signify a termination character.

The message received is any number ranging from 0 to 1023, but I need to use values between -255 and +255. To convert the character into an integer, I use the atoi function as follows:

int leftpower = (atoi(left)-512)/2;

When I print out the variable "leftpower", it seems to give the correct value. I'm even able to use "leftpower" in an if then statement as follows:

if (myvalue<0){  
  Serial.println("inside the if-then statement");
  digitalWrite(in1,HIGH);
  digitalWrite(in2,LOW);
  //analogWrite(enableA,myvalue);
}

So, as far as I can tell, everything is working fine. I want to use "myvalue" as a command to an analogWrite function call as follows:

//analogWrite(enablePinA,myvalue);

However, something strange happens when I uncomment this line. WHen I uncomment it, the program seems to hang at the if statement, as if it paused at that point and neither crashes nor gives any error. The program no longer prints out "inside the if-then statement" at all, but no other errors seem to be detected. No other compilation or runtime errors.

The strange thing about this is that if I change "leftpower" to a number like 223 as follows:

analogWrite(enablePinA,223);

the program works as I expected it to, producing a motor command. So, I know the problem is not with my wiring. The problem seems to be associated with the variable "leftpower", which was converted from a character array to an integer using the atoi function.

What exactly is going on? What problem is actually occurring and how can I fix it? Any help would be greatly appreciated!

For reference, my code is posted below:

#include <RH_ASK.h> //radiohead library to read RF signals
#include <SPI.h> //just because radiohead library depends on it
RH_ASK driver; //default: transmitter data pin goes to arduino pin 11
int in1=4;
int in2=5;
int in3=6;
int in4=7;

int enableA = 9;
int enableB = 10;

void setup() {
  // put your setup code here, to run once:
  Serial.begin(9600);
  if (!driver.init()){
    Serial.println("receiver init failed");
  } 
  pinMode(enableA,OUTPUT);
  pinMode(enableB,OUTPUT);
  pinMode(in1,OUTPUT);
  pinMode(in2,OUTPUT);
  pinMode(in3,OUTPUT);
  pinMode(in4,OUTPUT);
}
int leftpower,rightpower;
void loop() {
  // put your main code here, to run repeatedly:
  uint8_t buf[8];
  uint8_t buflen = sizeof(buf);
  if (driver.recv(buf, &buflen)){ //non-blocking
    //Convert buf (unsigned integer) to a character array
    char temp[8]="";
    sprintf(temp,"%s",(char*) buf);
    //Four characters for first integer, four for the second integer
    //So, we have to split them into two parts (left and right)
    char left[5]="";
    for (int i=0; i<4; i++){
      left[i]=temp[i];
    }
    //Put a terminating character at the end
    left[4]='\0';
    char right[5]="";
    for (int i=0; i<4; i++){
      right[i]=temp[4+i];
    }
    //Put a terminating character at the end
    right[4]='\0';
    leftpower=(int) (atoi(left)-512)/2;
    rightpower=(int) (atoi(right)-512)/2;
    constrain(leftpower,-255,255);
    constrain(rightpower,-255,255);
    Serial.println(leftpower);
    Serial.println(rightpower);
    if (leftpower<0) {
      //switch H-bridge to forward direction
      Serial.println("inside the if-then statement");
      digitalWrite(in1,HIGH);
      digitalWrite(in2,LOW);
      analogWrite(enableA,255);
      Serial.println("got this far");
    }
    else {
      digitalWrite(in1,LOW);
      digitalWrite(in2,HIGH);
      analogWrite(enableA,255);      
    }
//    if (rightpower<0) {
//      //switch H-bridge to forward direction
//      digitalWrite(in3,LOW);
//      digitalWrite(in4,HIGH);
//      analogWrite(enableA,abs(rightpower));      
//    }
//    else {
//      digitalWrite(in4,LOW);
//      digitalWrite(in3,HIGH);
//      analogWrite(enableA,abs(rightpower));      
//    }

  }
}

Answer 1

You have a few issues here.

if (driver.recv(buf, &buflen)) { ... }

You don't check the value of buflen after calling the recv() method. I didn't read the doc of the RH_ASK library you are using. Are you sure it does preserve message boundaries? I let you check. If it only transports bytes (rather than full messages), then you will have to repeatedly recv() until you have a full message.

Edit: Looked at the doc. The communication is indeed packetized, so you don't need to collect the message chunk by chunk. It is, however, still a good idea to check whether the received size is what you expect:

if (driver.recv(buf, &buflen) && buflen == 8) { ... }

sprintf(temp,"%s",(char*) buf);

This is a serious bug, and most probably the reason your program behaves inconsistently. Here buf is not a NUL-terminated string. The sprintf() function will copy bytes from buf into temp until it finds a NUL. It can potentially overwrite the receiving buffer and start clobbering whatever happens to be past temp in your memory. This is called “memory corruption” and can lead to any kind of unpredictable behaviors.

Note that the whole copying you do here serves no useful purpose. You can work from the initial buffer to start with:

char left[5];
memcpy(left, buf, 4);
left[4] = '\0';
char right[5];
memcpy(right, buf+4, 4);
right[4] = '\0';

constrain(leftpower,-255,255);

You are implicitly discarding the value computed by constrain(). You probably mean

leftpower = constrain(leftpower, -255, 255);