0

I have this function:

void changeColor(char control[16])
{
  char _red[3], _green[3], _blue[3], _brightness[3], _white[3];
  int red, green, blue, brightness, white, offset;

  offset = 1;
  for (int i = 0; i < 3; i++) {
    _red[i] = control[i + offset];
  }
  offset = 4;
  for (int i = 0; i < sizeof(green); i++) {
    _green[i] = control[i + offset];
  }
  // strncpy(_green,control+offset,3); // same result
  offset = 7;
  for (int i = 0; i < sizeof(_blue); i++) {
    _blue[i] = control[i + offset];
  }
  offset = 10;
  for (int i = 0; i < sizeof(_white); i++) {
    _white[i] = control[i + offset];
  }
  offset = 13;
  for (int i = 0; i < sizeof(_brightness); i++) {
    _brightness[i] = control[i + offset];
  }

  red = atoi(_red);
  green = atoi(_green);
  //sscanf(_green, "%d", &green); //same result
  blue = atoi(_blue);
  white = atoi(_white);
  brightness = atoi(_brightness);

  Serial.print(" control ");
  Serial.println(control);
  Serial.print(" _red ");
  Serial.println(_red);
  Serial.print(" _green ");
  Serial.println(_green);
  Serial.print(" _blue ");
  Serial.println(_blue);
  Serial.print(" _white ");
  Serial.println(_white);
  Serial.print(" _brightness ");
  Serial.println(_brightness);
  Serial.print(" red ");
  Serial.println(red);
  Serial.print(" green ");
  Serial.println(green);
  Serial.print(" blue ");
  Serial.println(blue);
  Serial.print(" white ");
  Serial.println(white);
  Serial.print(" brightness ");
  Serial.println(brightness);
  //... do something with ints
}

And by calling from the setup:

 char _control[16]={'6','2','5','5','0','0','0','0','0','0','0','0','0','0','5','0'};
 //6255000000000050

 void setup() {
   Serial.begin(9600);
   // ... do something
   while (!Serial) {
     ; // wait for serial port to connect. Needed for native USB
   }
  changeColor(_control);
}

I get from console this output:

   control 6255000000000050
   _red 255ÿ
   _green 000255ÿ
   _blue 000000255ÿ
   _white 000050000000255ÿ
   _brightness 050000000255ÿ
   red 255
   green 255
   blue 255
   white 29951
   brightness 29951

Basically, the char array are filled with more elements than the actual dimension (3), but I don't get error or warnings from the compiler

I also tried with strncpy, strcpy, strncat, and by using the pointer in the for loops

for(int i=0;i<sizeof(_green);i++){
  _green[i]=control+i*sizeof(char)+offset*sizeof(char);
} // also without the sizeof()

And I got the same result in the console (with nl&cr), any hints?

EDIT:

Added NULL char

  ...
  char _red[4], _green[4], _blue[4], _brightness[4], _white[4];
  int red, green, blue, brightness, white, offset;
  _red[4]='\0';
  _green[4]='\0';
  _blue[4]='\0';
  _white[4]='\0';
  _brightness[4]='\0';
  offset=1;
  for(int i=0;i<3;i++){
    _red[i]=control[i+ offset];
  }
  // same as above

Output:

control 6255000000000050
_red 255
_green 100255
_blue 000100255
_white 0000050
_brightness 050
red 255
green 100
blue 0
white 50
brightness 50
  • Where's the NULL termination of your strings? – Majenko Jun 22 '17 at 15:04
  • @Majenko, I added the null char(edited the question), but I get messed up output – Ghesio Jun 22 '17 at 15:15
  • 3
    In C you start counting from 0, not 1. _red[4] is actually _green[0]. – Majenko Jun 22 '17 at 15:29
  • 1
    @Majenko Understood, adding the null character with good index (been coding for too long today!) fixed the issue. Thank you sir. – Ghesio Jun 22 '17 at 15:52
0

In this piece of code you have a strange mix of upper bounds:

  for(int i=0;i<3;i++){
    _red[i]=control[i+offset];
  }
  offset=4;
  for(int i=0;i<sizeof(green);i++){
    _green[i]=control[i+offset];
  }
 // strncpy(_green,control+offset,3); // same result
 offset=7;
 for(int i=0;i<sizeof(_blue);i++){
   _blue[i]=control[i+offset];
 }
 offset=10;
 for(int i=0;i<sizeof(_white);i++){
   _white[i]=control[i+offset];
 }
 offset=13;
 for(int i=0;i<sizeof(_brightness);i++){
   _brightness[i]=control[i+offset];
 }

The first looks correct at 3. The second loop has an upper bound of sizeof(int) which is 2. The third loop seems to be sizeof(char) which is 1.

Unfortunately non of those values match up with the results you are getting. What I would try is refactoring whats inside you function to this.

int Convert (char* buffer, char const* control, const int& offset, char const* name)
{
    int value = 0;
    for (index = offset; index < offset; ++index)
    {
        buffer[index-offset] = control[index];
    }
    buffer[4] = '\0';
    atoi(buffer);
    Serial.print(name);
    Serial.print(" = ");
    Serial.print(buffer);
    Serial.print("  Value = ");
    Serial.println(value);
    return value;
}

And then call it to populate your values:

void changeColor(char control[16])
{
    char buffer[4]; // You can reuse this buffer, unless you wanted to have separate ones.
    const int red    = Convert (buffer, control, 1, "red");
    const int green  = Convert (buffer, control, 4, "green");
    const int blue   = Convert (buffer, control, 7, "blue");
    const int white  = Convert (buffer, control, 4, "white");
    const int bright = Convert (buffer, control, 4, "bright");
}
  • 1
    sizeof(green) is a typo. This is a much more good looking and working code. Thanks – Ghesio Jul 1 '17 at 13:32
  • re: sizeof(green) I thought so, but just in case it wasn't I thought I'd say. – Code Gorilla Jul 3 '17 at 7:25

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.