Search code examples
csegmentation-faultserial-portscanfadc

Is there a more reasonable way for reading adc values from serial?


I have written a program to read adc values from an atmega2560 in two modes, a single sample mode that prints a sample for each channel selected by user, and a bulk mode that works by filling a buffer of samples for each channel selected and printing it out on a txt when all these buffers are full.

I use an header 0x55aa to announce data is coming, if I receive it, I fill the struct with the informations I need: sequence number, channel, number of samples. I then use a for loop to fill the buffers I am interested in (it will do only one cycle in continous mode, as number of samples in struct will be one)

int n_read = fscanf(f, "%d", & key); //we read 4 bytes and see if it equals the key
    
    if (key == 0x55aa) {
      n_read = fscanf(f, "%hhu", & structure.seq); //if it does we fill the struct
      n_read = fscanf(f, "%hhu", & structure.channel);
      n_read = fscanf(f, "%hhu", & structure.num_samples);
      num_samples_buffer[structure.channel - 1] = structure.num_samples;
      
      for (int k = 0; k < structure.num_samples; k++) { //we fill the received channel's buffer with the data in bulk (if mode is single it's executed once)
        switch (structure.channel) {
        case 1:
          curr_buff = buffer1;
          break;
        case 2:
          curr_buff = buffer2;
          break;
        case 3:
          curr_buff = buffer3;
          break;
        case 4:
          curr_buff = buffer4;
          break;
        case 5:
          curr_buff = buffer5;
          break;
        case 6:
          curr_buff = buffer6;
          break;
        case 7:
          curr_buff = buffer7;
          break;
        case 8:
          curr_buff = buffer8;
          break;
        }
        n_read = fscanf(f, "%hhu", & curr_buff[k]);

If mode is continous I call the function write_single, it puts the value read after the for loop inside a temporary buffer with size 8, one place for each channel; I use it to hold one sample for each channel selected by the user, when every channel we are interested in has its sample, I print the row in the output file.

void write_single(FILE * ftxt, uint8_t sample, uint8_t channel) { 

  temp_buffer[structure.channel - 1] = sample;
  counter++;
  if (counter == num_channels) {
    for (int k = 0; k < 8; k++) {
      if (channel_buffer[k] != 0) {
        printf("-%d-\n",k);
        res = fprintf(ftxt, "%.1f", ((((float)((unsigned char) temp_buffer[k])) / 255) * 5));
        if (res < 0) {
          printf("error");
        }
        res = fprintf(ftxt, " ");
        if (res < 0) {
          printf("error");
        }
      }
    }
    res = fprintf(ftxt, "\n");
    if (res < 0) {
      printf("error");
    }
    counter = 0;
  }
}

If we are in bulk mode and we received all the samples indicated by the initial struct for all the channels indicated by the user, we call the function write_bulk. It is used to cycle on every buffer the user selected and for BUFFER_SIZE elements for each buffer.

void write_bulk(void) { 
  for (int j = 0; j < BUFFER_SIZE; j++) {
    for (int k = 0; k < 8; k++) {
      if (channel_buffer[k] != 0) {
        if (j >= num_samples_buffer[k]) {
          break;
        }
        switch (k) {
        case 0:
          curr_buff_bulk = buffer1;
          break;
        case 1:
          curr_buff_bulk = buffer2;
          break;
        case 2:
          curr_buff_bulk = buffer3;
          break;
        case 3:
          curr_buff_bulk = buffer4;
          break;
        case 4:
          curr_buff_bulk = buffer5;
          break;
        case 5:
          curr_buff_bulk = buffer6;
          break;
        case 6:
          curr_buff_bulk = buffer7;
          break;
        case 7:
          curr_buff_bulk = buffer8;
          break;
        }
        res = fprintf(ftxt, "%.1f", ((((float)((unsigned char) curr_buff_bulk[j])) / 255) * 5));
        res = fprintf(ftxt, " ");
      }
    }
    res = fprintf(ftxt, "\n");
  }
}

The values sent are an int for the header (called key in the code), uint8_t for each of the other data.

I set the serial in blocking mode with:

void serial_set_blocking(int fd, int should_block) {
  struct termios tty;
  memset (&tty, 0, sizeof tty);
  if (tcgetattr (fd, &tty) != 0) {
      printf ("error %d from tggetattr", errno);
      return;
  }

  tty.c_cc[VMIN]  = should_block ? 1 : 0;
  tty.c_cc[VTIME] = 5;            // 0.5 seconds read timeout

  if (tcsetattr (fd, TCSANOW, &tty) != 0)
    printf ("error %d setting term attributes", errno);
}

Now, it works for a brief period and then go in segmentation fault, I could not manage to use gdb (I'm ona virtual machine, could be the cause?) as it doesn't open the serial if I run the code with it, but I printed some values and used valgrind and the problem looks like sometimes it is very error prone, it takes the adc value when it should read the channel and viceversa, or it reads the channel when it should read the sequence number, I lose some sequence numbers.. etc i thought the serial in blocking mode would make it easy, I read the key, after which I read the values one by one, the values should be in order right? What about scanf and fprintf, is it ok to use them?

EDIT: I'm adding the definition of the buffers and the struct, even if they will be modified following @Craig Estey's suggestion

typedef struct bulk {
  uint8_t seq;
  uint8_t channel;
  uint8_t num_samples;
}
bulk;
bulk structure;
uint8_t channel_buffer[8] = {0}; //used to keep track of what channel the user is interested in
uint8_t num_samples_buffer[8] = {0}; //used to limit the accesses to buffer on the number of samples we actually received
uint8_t num_channels = 0;

uint8_t buffer1[BUFFER_SIZE];
uint8_t buffer2[BUFFER_SIZE];
uint8_t buffer3[BUFFER_SIZE];
uint8_t buffer4[BUFFER_SIZE];
uint8_t buffer5[BUFFER_SIZE];
uint8_t buffer6[BUFFER_SIZE];
uint8_t buffer7[BUFFER_SIZE];
uint8_t buffer8[BUFFER_SIZE];

uint8_t temp_buffer[8] = {0}; //buffer to write the rows in continous mode

I'm sending data from atmega2560 through serial port using this: http://mysinski.wieik.pk.edu.pl/MPUandMCU/Using%20printf%20with%20an%20AVR.pdf

using it that way:


printf("%d\n", key); //32 bit
printf("%hhu\n", structure.seq); //8 bit
printf("%hhu\n", structure.channel); //8 bit
printf("%hhu\n", structure.num_samples); //8 bit
printf("%hhu\n", ADCH); //this is the sampled value, 8 bit

Then in my client I read the key and the struct values in a while(1) loop:

while (1) {
    int n_read = fscanf(f, "%d", & key); //we read 4 bytes and see if it equals the key
    if (n_read < 0) {
      perror("error");
    }
    if (key == 0x55aa) {

      n_read = fscanf(f, "%hhu", & structure.seq); //if it does we fill the struct
      if (n_read < 0) {
        perror("error");
      }
      n_read = fscanf(f, "%hhu", & structure.channel);
      if (n_read < 0) {
        perror("error");
      }
      n_read = fscanf(f, "%hhu", & structure.num_samples);
      if (n_read < 0) {
        perror("error");
      }

      .
      .
      .
      //other code
      .
      .
      .

      n_read = fscanf(f, "%hhu", & curr_buff[k]);
      if (n_read < 0) {
        printf("error");
      }

      .
      .
      .
      //other code
      .
      .
      .

      }
      key = 0; //we clear the key
    }
  }

Here is an example of the error I get in continous mode:

.
.
.
seq: 37
channel: 3
num_samp: 1
seq: 38
channel: 4
num_samp: 1
seq: 39
channel: 5
num_samp: 1
seq: 40
channel: 46 <-----------------error
num_samp: 5

Solution

  • The printf function in server was converting each uint8_t in char values, so if I read 255 I was sending 3 bytes instead of 1, this ended up saturating the baud rate. I replaced printf with a low level UDR0=data to send binary values as suggested by @Craig Estey, and I replaced the fscanf with read.