Why is integer-to-string not working in this sketch?

Question

I am using a Pro Mini to read analog values and to send them via serial.

Goal of the sketch is to stream these values to Serial as fast as possible, to obtain a steady stream of at least 1000 readings per second. For this reason I reduced the ADC prescaler and I increased serial speed.

I tested sending binary values with Serial.write and it works (it's the fastest), I tested Serial.print (it's the slowest), and I tested an optimised conversion integer-to-string (found on StackOverflow) in association with Serial.write.

This optimised conversion is faster than Serial.print (it would allow me to send the full strings within the given timeframe), but it gives me wrong values. I cannot understand why it happens, the code looks fine to my not-so-expert eye.

A correct stream as sent by Serial.print would be (spaces added for clarity):

3075 \t 2800 \t 4132 \n

"Wrong values" means that the last value gets repeated for all the three columns:

4132 \t 4132 \t 4132 \n

What is the issue in my code regarding the int2str conversion?

Since I cannot get one measurement taken and sent every 500 us, I would like to use the optimised routine to get plaintext readings at 1000 us intervals.

The source code follows. I cut out the function slimADC because it was tested as working, while I left all the 3 methods for sending data via serial: binary, optimised, Serial.print.

#include <digitalWriteFast.h>

volatile unsigned long previousMicros, currentMicros;

#define SAMPLES 8
volatile unsigned int valuesX;
volatile unsigned int valuesY;
volatile unsigned int valuesZ;

char* x;
char* y;
char* z;

void setup() {
// set pins, ADC prescaler and initialise ADC
  pinModeFast(A0, INPUT);
  pinModeFast(A1, INPUT);
  pinModeFast(A2, INPUT);
  ADCSRA &= ~(bit (ADPS0) | bit (ADPS1) | bit (ADPS2)); // clear bits
  ADCSRA |= bit (ADPS0) | bit (ADPS2);  // 32: 32 us per ADC
  slimADC(0);
  slimADC(0);

// initialise Serial
  Serial.begin(250000);
  Serial.write("\n");
  Serial.write("\n");
  previousMicros = 0;
}

void loop() {
  currentMicros = micros();
  if ((currentMicros - previousMicros) < 1000) { return; }
  previousMicros = currentMicros;

// ADC readings, repeated SAMPLES times
  valuesX = 0;
  valuesY = 0;
  valuesZ = 0;
  for (uint8_t i = 1; i < SAMPLES; i++) {  // 8*3*32=768 us
    valuesX += slimADC(0);
    valuesY += slimADC(1);
    valuesZ += slimADC(2);
  }

// SERIAL TRANSMISSION

// fastest
//  Serial.write(valuesX / 256);
//  Serial.write(valuesX % 256);
//  Serial.write("\t");
//  Serial.write(valuesY / 256);
//  Serial.write(valuesY % 256);
//  Serial.write("\t");
//  Serial.write(valuesZ / 256);
//  Serial.write(valuesZ % 256);
//  Serial.write("\n");

// good
  x = int2str( valuesX );
  y = int2str( valuesY );
  z = int2str( valuesZ );
  Serial.write(x);
  Serial.write("\t");
  Serial.write(y);
  Serial.write("\t");
  Serial.write(z);
  Serial.write("\n");

// slowest
//  Serial.print(valuesX);
//  Serial.print("\t");
//  Serial.print(valuesY);
//  Serial.print("\t");
//  Serial.print(valuesZ);
//  Serial.print("\n");
}

static int slimADC(uint8_t pin) { ... }

char _int2str[7];
char* int2str( register int i ) {
  register unsigned char L = 1;
  register char c;
  register boolean m = false;
  register char b;  // lower-byte of i
  // negative
  if ( i < 0 ) {
    _int2str[ 0 ] = '-';
    i = -i;
  }
  else L = 0;
  // ten-thousands
  if( i > 9999 ) {
    c = i < 20000 ? 1
      : i < 30000 ? 2
      : 3;
    _int2str[ L++ ] = c + 48;
    i -= c * 10000;
    m = true;
  }
  // thousands
  if( i > 999 ) {
    c = i < 5000
      ? ( i < 3000
          ? ( i < 2000 ? 1 : 2 )
          :   i < 4000 ? 3 : 4
        )
      : i < 8000
        ? ( i < 6000
            ? 5
            : i < 7000 ? 6 : 7
          )
        : i < 9000 ? 8 : 9;
    _int2str[ L++ ] = c + 48;
    i -= c * 1000;
    m = true;
  }
  else if( m ) _int2str[ L++ ] = '0';
  // hundreds
  if( i > 99 ) {
    c = i < 500
      ? ( i < 300
          ? ( i < 200 ? 1 : 2 )
          :   i < 400 ? 3 : 4
        )
      : i < 800
        ? ( i < 600
            ? 5
            : i < 700 ? 6 : 7
          )
        : i < 900 ? 8 : 9;
    _int2str[ L++ ] = c + 48;
    i -= c * 100;
    m = true;
  }
  else if( m ) _int2str[ L++ ] = '0';
  // decades (check on lower byte to optimize code)
  b = char( i );
  if( b > 9 ) {
    c = b < 50
      ? ( b < 30
          ? ( b < 20 ? 1 : 2 )
          :   b < 40 ? 3 : 4
        )
      : b < 80
        ? ( i < 60
            ? 5
            : i < 70 ? 6 : 7
          )
        : i < 90 ? 8 : 9;
    _int2str[ L++ ] = c + 48;
    b -= c * 10;
    m = true;
  }
  else if( m ) _int2str[ L++ ] = '0';
  // last digit
  _int2str[ L++ ] = b + 48;
  // null terminator
  _int2str[ L ] = 0;  
  return _int2str;
}

Appendix

The bare ADC conversions take about 768 us.

With binary data I am sending via serial 3x uint16_t plus one end of line and two tabs, meaning 9 characters per reading. At 250 kbps 8N1 the minimum time required is 324 us. Using RealTerm (for Win) I can see that I get a stram of 9k chars/s, as expected.

With Serial.print I am sending 4x string[4] (readings are always between 1000 and 9999), plus tabs and newline, meaning 15 bytes per reading. RealTerm however gives me an incoming stream of 11-12k chars/s, therefore Serial.print is too slow.

The optimised int-to-string function (even without modifying it to avoid checking for negative and for >9999, not needed in my case) still sends 15 bytes per reading, and RealTerm also shows me an actual stream of 15k chars/s, as expected.

Answer 1

it took me a while and i'm still not 100% sure if I got you right. I think you have two questions:

1) how to speed up the entire execution?

the serial routines will be your biggest bottleneck. I would suggest that a) you speed up the baud rate; and b) you try to use a tx interrupt here.

2) why isn't the int2str() routine working?

because int2str() stores the converted string in a buffer (_int2str[]), which is subsequently destroyed by conversions. so for it to work, you should do something like this:

  serial.write(int2str(x)); serial.write("\t");
  serial.write(int2str(y)); serial.write("\t");
  serial.write(int2str(z)); serial.write("\n");

BTW, int2str() can be written a lot better than it is now.

edit: to add an alternate int2str() implementaton.

//positive and negative numbers
//int is 16-bit type
char *int2str2(int i) {
    static char _int2str[7];
    unsigned char tmp, str_len=0;       //converted string starts at _int2str[0]

    //process the sign
    if (i&0x8000/*<0*/) {
      _int2str[str_len++]='-';          //insert leading '-'
      i=-i;                             //make the number positive
    };

    //convert the next position (10,000)
    tmp = 0;                            //reset tmp
    while (i >= 10000) {
      i-=10000;                         //decrement i
      tmp+=1;                           //increment counter
    }
    _int2str[str_len++] = tmp + '0';    //form the msb

    //convert the next position (1,000)
    tmp = 0;                            //reset tmp
    while (i >= 1000) {
      i-=1000;                          //decrement i
      tmp+=1;                           //increment counter
    }
    _int2str[str_len++] = tmp + '0';    //form the msb

    //convert the next position (100)
    tmp = 0;                            //reset tmp
    while (i >= 100) {
      i-=100;                           //decrement i
      tmp+=1;                           //increment counter
    }
    _int2str[str_len++] = tmp + '0';    //form the msb

    //convert the next position (10)
    tmp = 0;                            //reset tmp
    while (i >= 10) {
      i-=10;                            //decrement i
      tmp+=1;                           //increment counter
    }
    _int2str[str_len++] = tmp + '0';    //form the msb

    //convert the lsb
    //tmp = 0;                          //reset tmp
    //while (i >= 100000ul) {
    //  i-=100000ul;                        //decrement i
    //  tmp+=1;                         //increment counter
    //}
    tmp = i;
    _int2str[str_len++] = tmp + '0';    //form the msb

    return _int2str;
}

here is what I would use. I benchmark against yours, converting numbers from 0 to 32767. total cycle count for yours is 10.5 million and 7.0 million for mine. Now, the actual performance will depend on a lot of factors, like your chips, compilers (I used IAR AVR), and optimization settings, to name a few.

Note: the routine was written as a generic one. if you are only converting 10-bit numbers (from a 10-bit adc for example), it can be greatly simplified, and execution time greatly shortened: <160K cumulative cycles converting numbers from 0 to 1023.