Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Embedded C code review

Tags:

c

I am required to write a function that uses a look up table for ADC values for temperature sensor analog input, and it finds out the temperature given an ADC value by "interpolating" - linear approximation. I have created a function and written some test cases for it, I want to know if there is something which you guys can suggest to improve the code, since this is supposed to be for an embedded uC, probably stm32.

I am posting my code and attaching my C file, it will compile and run.

Please let me know if you have any comments/suggestions for improvement.

I also want to know a bit about the casting from uint32_t to float that I am doing, if it is efficient way to code.

#include <windows.h>
#include <stdio.h>
#include <stdlib.h>
#include <stdint.h>

#define TEMP_ADC_TABLE_SIZE 15

typedef struct
{
 int8_t temp;     
 uint16_t ADC;       
}Temp_ADC_t;

const Temp_ADC_t temp_ADC[TEMP_ADC_TABLE_SIZE] = 
{
    {-40,880}, {-30,750},
    {-20,680}, {-10,595},
    {0,500}, {10,450},
    {20,410}, {30,396},
    {40,390}, {50,386},
    {60,375}, {70,360},
    {80,340}, {90,325},
    {100,310}
};

// This function finds the indices between which the input reading lies.
// It uses an algorithm that doesn't need to loop through all the values in the 
// table but instead it keeps dividing the table in two half until it finds
// the indices between which the value is or the exact index.
//
// index_low, index_high, are set to the indices if a value is between sample
// points, otherwise if there is an exact match then index_mid is set.
// 
// Returns 0 on error, 1 if indices found, 2 if exact index is found.
uint8_t find_indices(uint16_t ADC_reading, 
                    const Temp_ADC_t table[],
                    int8_t dir, 
                    uint16_t* index_low, 
                    uint16_t* index_high, 
                    uint16_t* index_mid, 
                    uint16_t table_size)
{
    uint8_t found = 0;
    uint16_t mid, low, high;
    low = 0;
    high = table_size - 1;

    if((table != NULL) && (table_size > 0) && (index_low != NULL) &&
       (index_mid != NULL) && (index_high != NULL))
    {
        while(found == 0)
        {
            mid = (low + high) / 2;

            if(table[mid].ADC == ADC_reading)
            {   
                // exact match                       
                found = 2;            
            }
            else if(table[mid].ADC < ADC_reading)
            {
                if(table[mid + dir].ADC == ADC_reading)
                {
                    // exact match          
                    found = 2;
                    mid = mid + dir;                             
                }
                else if(table[mid + dir].ADC > ADC_reading)
                {
                    // found the two indices
                    found = 1;
                    low = (dir == 1)? mid : (mid + dir);
                    high = (dir == 1)? (mid + dir) : mid;                            
                }
                else if(table[mid + dir].ADC < ADC_reading)
                {                     
                    low = (dir == 1)? (mid + dir) : low;
                    high = (dir == 1) ? high : (mid + dir);
                }              
            }
            else if(table[mid].ADC > ADC_reading)
            {
                if(table[mid - dir].ADC == ADC_reading)
                {
                    // exact match          
                    found = 2;
                    mid = mid - dir;                             
                }
                else if(table[mid - dir].ADC < ADC_reading)
                {
                    // found the two indices
                    found = 1;
                    low = (dir == 1)? (mid - dir) : mid;
                    high = (dir == 1)? mid : (mid - dir);                            
                }
                else if(table[mid - dir].ADC > ADC_reading)
                {
                    low = (dir == 1)? low : (mid - dir);
                    high = (dir == 1) ? (mid - dir) : high;
                }
            } 
        }        
        *index_low = low;
        *index_high = high;
        *index_mid = mid;        
    }

    return found;  
}

// This function uses the lookup table provided as an input argument to find the
// temperature for a ADC value using linear approximation. 
// 
// Temperature value is set using the temp pointer.  
// 
// Return 0 if an error occured, 1 if an approximate result is calculate, 2
// if the sample value match is found.

uint8_t lookup_temp(uint16_t ADC_reading, const Temp_ADC_t table[], 
                    uint16_t table_size ,int8_t* temp)
{
    uint16_t mid, low, high;
    int8_t dir;
    uint8_t return_code = 1;
    float gradient, offset;

    low = 0;
    high = table_size - 1;

    if((table != NULL) && (temp != NULL) && (table_size > 0))
    {
        // Check if ADC_reading is out of bound and find if values are
        // increasing or decreasing along the table.
        if(table[low].ADC < table[high].ADC)
        {
            if(table[low].ADC > ADC_reading)
            {
                return_code = 0;                                    
            }
            else if(table[high].ADC < ADC_reading)
            {
                return_code = 0;
            }
            dir = 1;
        }    
        else
        {
            if(table[low].ADC < ADC_reading)
            {
                return_code = 0;                                    
            }
            else if(table[high].ADC > ADC_reading)
            {
                return_code = 0;
            }
            dir = -1; 
        }
    }
    else
    {
        return_code = 0;    
    } 

    // determine the temperature by interpolating
    if(return_code > 0)
    {
        return_code = find_indices(ADC_reading, table, dir, &low, &high, &mid, 
                                   table_size);

        if(return_code == 2)
        {
            *temp = table[mid].temp;
        }
        else if(return_code == 1)
        {
            gradient = ((float)(table[high].temp - table[low].temp)) / 
                       ((float)(table[high].ADC - table[low].ADC));
            offset = (float)table[low].temp - gradient * table[low].ADC;
            *temp = (int8_t)(gradient * ADC_reading + offset);
        }
    }  

    return return_code;   
}



int main(int argc, char *argv[])
{
  int8_t temp = 0;
  uint8_t x = 0;  
  uint16_t u = 0;
  uint8_t return_code = 0; 
  uint8_t i;

  //Print Table
  printf("Lookup Table:\n");
  for(i = 0; i < TEMP_ADC_TABLE_SIZE; i++)
  {
      printf("%d,%d\n", temp_ADC[i].temp, temp_ADC[i].ADC);                
  }

  // Test case 1
  printf("Test case 1: Find the temperature for ADC Reading of 317\n");
  printf("Temperature should be 95 Return Code should be 1\n");
  return_code = lookup_temp(317, temp_ADC, TEMP_ADC_TABLE_SIZE, &temp);
  printf("Temperature: %d C\n", temp);
  printf("Return code: %d\n\n", return_code);

  // Test case 2  
  printf("Test case 2: Find the temperature for ADC Reading of 595 (sample value)\n");
  printf("Temperature should be -10, Return Code should be 2\n");
  return_code = lookup_temp(595, temp_ADC, TEMP_ADC_TABLE_SIZE, &temp);
  printf("Temperature: %d C\n", temp);
  printf("Return code: %d\n\n", return_code);

  // Test case 3  
  printf("Test case 3: Find the temperature for ADC Reading of 900 (out of bound - lower)\n");
  printf("Return Code should be 0\n");
  return_code = lookup_temp(900, temp_ADC, TEMP_ADC_TABLE_SIZE, &temp);
  printf("Return code: %d\n\n", return_code);

  // Test case 4 
  printf("Test case 4: Find the temperature for ADC Reading of 300 (out of bound - Upper)\n");
  printf("Return Code should be 0\n");
  return_code = lookup_temp(300, temp_ADC, TEMP_ADC_TABLE_SIZE, &temp);
  printf("Return code: %d\n\n", return_code);

  // Test case 5
  printf("Test case 5: NULL pointer (Table pointer) handling\n");
  printf("Return Code should be 0\n");
  return_code = lookup_temp(595, NULL, TEMP_ADC_TABLE_SIZE, &temp);
  printf("Return code: %d\n\n", return_code);

  // Test case 6
  printf("Test case 6: NULL pointer (temperature result pointer) handling\n");
  printf("Return Code should be 0\n");
  return_code = lookup_temp(595, temp_ADC, TEMP_ADC_TABLE_SIZE, NULL);
  printf("Return code: %d\n", return_code);

  // Test case 7
  printf("Test case 7: Find the temperature for ADC Reading of 620\n");
  printf("Temperature should be -14 Return Code should be 1\n");
  return_code = lookup_temp(630, temp_ADC, TEMP_ADC_TABLE_SIZE, &temp);
  printf("Temperature: %d C\n", temp);
  printf("Return code: %d\n\n", return_code);

  // Test case 8
  printf("Test case 8: Find the temperature for ADC Reading of 880 (First table element test)\n");
  printf("Temperature should be -40 Return Code should be 2\n");
  return_code = lookup_temp(880, temp_ADC, TEMP_ADC_TABLE_SIZE, &temp);
  printf("Temperature: %d C\n", temp);
  printf("Return code: %d\n\n", return_code);

  // Test case 9
  printf("Test case 9: Find the temperature for ADC Reading of 310 (Last table element test)\n");
  printf("Temperature should be 100 Return Code should be 2\n");
  return_code = lookup_temp(310, temp_ADC, TEMP_ADC_TABLE_SIZE, &temp);
  printf("Temperature: %d C\n", temp);
  printf("Return code: %d\n\n", return_code);  

  printf("Press ENTER to continue...\n");  
  getchar();
  return 0;
}
like image 883
alliiieeeennnnn Avatar asked Dec 12 '22 19:12

alliiieeeennnnn


1 Answers

I generally compute the lookup table offline and the runtime code boils down to:

temp = table[dac_value];

Particularly if going embedded, you dont want floating point, often dont need it. Pre-computing the table solves that problem as well.

Pre-computing also solves the problem of having an efficient algorithm, you can be as sloppy and slow as you want, you only have to do this computation rarely. No algorithm is going to be able to compete with the lookup table at runtime. So long as you have room for the look up table it is a win-win. If you dont have say 256 locations in prom for an 8 bit dac for example you might have 128 locations, and you can do a little real-time interpolation:

//TODO add special case for max dac_value and max dac_value-1 or make the table 129 entries deep
if(dac_value&1)
{
  temp=(table[(dac_value>>1)+0]+table[(dac_value>>1)+1])>>1;
}
else
{
   temp=table[dac_value>>1];
}

I often find that the table being fed in can and will change. Yours may be cast in stone, but this same kind of computation comes about with calibrated devices. And you have done the right thing by checking that the data is in the right general direction (decreasing relative to the dac increasing or increasing relative to dac values increasing) and more importantly check for divide by zero. Despite being a hard coded table develop habits with the expectation that it will change to a different hard coded table with the desire to not have to change your interpolation code every time.

I also believe that the raw dac value is the most important value here, the computed temperature can happen at any time. Even if the conversion to degrees of some flavor has been cast in stone, it is a good idea to display or store the raw dac value along with the computed temperature. You can always re-compute the temperature from the dac value, but you cannot always accurately reproduce the raw dac value from the computed value. It depends on what you are building naturally, if this is a thermostat for public use in their homes they dont want to have some hex value on the display. But if this is any kind of test or engineering environment where you are collecting data for later analysis or verification that some product is good or bad, carrying around that dac value can be a good thing. It only takes once or twice for a situation where the engineer that provided you with the table, claims it was the final table then changes it. Now you have to go back to all the logs that used the incorrect table, compute back to the dac value using the prior table and re-compute the temp using the new table and write a new log file. If you had the raw dac value there and everyone was trained to think in terms of dac values and that the temperature was simply a reference, you might not have to repair older log values for each new calibration table. The worst case is having only the temperature in the log file and not being able to determine which cal table was used for that log file, the log file becomes invalid the unit tested becomes a risk item, etc.

like image 120
old_timer Avatar answered Jan 05 '23 08:01

old_timer