嵌入式C代码审查

我需要编写一个函数，该函数使用ADC值的查找表作为温度传感器模拟输入，并通过“插值” - 线性逼近找出给定ADC值的温度。我已经创建了一个函数并为它编写了一些测试用例，我想知道是否有什么东西可以用来改进代码，因为这应该是嵌入式uC，可能是stm32。嵌入式C代码审查

我张贴我的代码并附上我的C文件，它会编译并运行。

如果您有任何关于改进的意见/建议，请让我知道。

我也想知道一些关于从uint32_t投射到我正在做的浮动，如果它是有效的编码方式。

#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; 
}

来源

2010-09-04 alliiieeeennnnn

有很多你可以改进。首先，最好的整数数据类型取决于机器（字的大小）。我不知道你的int8_t和uint16_t是如何声明的。

而且，不是性能而是可读性，我通常不使用 “级联” 如果，像

if condition 
{ 
    if another_condition 
    { 
     if third condition 
     {

而是：

if not condition 
    return false; 

// Here the condition IS true, thus no reason to indent

关注的还有一点：

low = (dir == 1)? mid : (mid + dir); 
high = (dir == 1)? (mid + dir) : mid;

你做了dir == 1两次，最好用ifs：

int sum = mid+dir; 
if dir == 1 
{ 
low = mid; 
high = sum; 
} 
else 
{ 
low=sum; 
high=mid; 
}

但还有更多要说的。例如，您可以使用更快的搜索算法。

来源

2010-09-04 10:30:44 vulkanino

'uint16_t'等在C99的'stdint.h'中定义。这是很好的做法。 – 2010-09-06 05:18:39

为什么你的平分搜索必须处理升序和降序表？该表无论如何都是硬编码的，因此您可以离线准备表，并将复杂性减半。您通常也不需要进行一半的比较 - 只需在高和低相邻或相等时停止。

在诸如小程序中，对非空表和其他输入进行非常少的检查，并且默默地报告没有匹配 - 返回和错误代码，或者确保一个地方函数被称为它不是用无效指针调用（没有理由认为无效指针为NULL，仅仅因为NULL在某些系统上可能是无效指针）。

如果没有额外的复杂性，搜索将成为类似：

enum Match { MATCH_ERROR, MATCH_EXACT, MATCH_INTERPOLATE, MATCH_UNDERFLOW, MATCH_OVERFLOW }; 

enum Match find_indices (uint16_t ADC_reading, 
        const Temp_ADC_t table[], 
        uint16_t* index_low, 
        uint16_t* index_high) 
{ 
    uint16_t low = *index_low; 
    uint16_t high = *index_high; 

    if (low >= high) return MATCH_ERROR; 
    if (ADC_reading < table [ low ].ADC) return MATCH_UNDERFLOW; 
    if (ADC_reading > table [ high ].ADC) return MATCH_OVERFLOW; 

    while (low < high - 1) 
    { 
     uint16_t mid = (low + high)/2; 
     uint16_t val = table [ mid ].ADC; 

     if (ADC_reading > val) 
     { 
      low = mid; 
      continue; 
     } 

     if (ADC_reading < val) 
     { 
      high = mid; 
      continue; 
     } 

     low = high = mid; 
     break; 
    } 

    *index_low = low; 
    *index_high = high; 

    if (low == high) 
     return MATCH_EXACT; 
    else 
     return MATCH_INTERPOLATE; 
}

由于该表已预先准备将上升，和搜索返回一个有意义的枚举，而不是一个整数代码，你不”吨需要那么多的lookup_temp：

enum Match lookup_temp (uint16_t ADC_reading, const Temp_ADC_t table[], 
       uint16_t table_size, int8_t* temp) 
{ 
    uint16_t low = 0; 
    uint16_t high = table_size - 1; 

    enum Match match = find_indices (ADC_reading, table, &low, &high); 

    switch (match) { 
     case MATCH_INTERPOLATE: 
      { 
       float gradient = ((float)(table[high].temp - table[low].temp))/
           ((float)(table[high].ADC - table[low].ADC)); 
       float offset = (float)table[low].temp - gradient * table[low].ADC; 

       *temp = (int8_t)(gradient * ADC_reading + offset); 

       break; 
      } 

     case MATCH_EXACT: 
      *temp = table[low].temp; 
      break; 
    } 

    return match; 
}

鉴于该梯度计算所有条款都是16位整数的，你可以在32位分割之前执行插值，只要你计算出分子的所有条款：

*temp = temp_low + uint16_t ((uint32_t (ADC_reading - adc_low) * uint32_t ( temp_high - temp_low))/uint32_t (adc_high - adc_low));

来源

2010-09-04 11:43:05

我大致计算查找表离线和运行时代码归结为：

 
temp = table[dac_value];

特别是如果外出时嵌入的，你不想浮点，往往不需要它。预先计算表格也解决了这个问题。

预计算也解决了高效算法的问题，你可以像任意想象的那样随意而慢，你只需要很少做这种计算。没有一种算法能够在运行时与查找表竞争。只要你有查找表的空间，这是一个双赢。如果你没有发言权舞会的256个单元的8位DAC，例如，你可能有128个位置，你可以做一个小的实时插补：

 
//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]; 
}

我经常发现，该表中可以被送入并会改变。你的结果可能会被抛在脑后，但这种计算方式与校准设备有关。并且通过检查数据是否处于正确的总体方向（相对于dac增加或增加，相对于dac值增加而减少）并且更重要地检查除以零来做出正确的事情。尽管是一个硬编码表的开发习惯，期望它会变成不同的硬编码表，希望不必每次都改变你的插值代码。

我也相信原始的dac值是这里最重要的值，计算的温度可以随时发生。即使转换成某种味道的程度已经成功了，最好显示或存储原始dac值以及计算出的温度。您始终可以通过dac值重新计算温度，但不能始终准确地从计算值中重现原始dac值。这取决于你自己建造什么，如果这是一个在家中公共使用的恒温器，他们不希望在显示器上有一些十六进制值。但是，如果这是任何一种测试或工程环境，您正在收集数据以供日后分析或验证某些产品的好坏，那么携带dac值可能是件好事。对于提供表格的工程师来说，这只需要一次或两次，并声称它是最终表格，然后对其进行更改。现在，您必须返回所有使用不正确表的日志，使用先前的表计算回dac值，并使用新表重新计算临时值，并写入新的日志文件。如果您有原始的dac值，并且每个人都受过培训以考虑dac值，并且温度只是一个参考值，则您可能不必为每个新校准表修复较早的log值。最糟糕的情况是只有日志文件中的温度，并且无法确定该日志文件使用哪个校准表，日志文件变得无效，被测试的单元成为风险项目等。

来源

2010-09-04 15:36:42

这很好你包含了测试框架，但是你的测试框架缺乏严谨性和滥用干涉（Do not Repeat Yourself）原则。

static const struct test_case 
{ 
    int inval; /* Test reading */ 
    int rcode; /* Expected return code */ 
    int rtemp; /* Expected temperature */ 
} test[] = 
{ 
    { 317, 1, 95 }, 
    { 595, 1, -10 }, 
    { 900, 0, 0 }, // Out of bound - lower 
    { 300, 0, 0 }, // Out of bound - upper 
    { 620, 1, -14 }, 
    { 880, 2, -40 }, // First table element 
    { 310, 2, 100 }, // Last table element 
};

现在，您可以编写测试代码，用于在功能单一的测试：

static int test_one(int testnum, const struct test_case *test) 
{ 
    int result = 0; 
    int temp; 
    int code = lookup_temp(test->inval, temp_ADC, TEMP_ADC_TABLE_SIZE, &temp); 

    if (temp == test->rtemp && code == test->rcode) 
     printf("PASS %d: reading %d, code %d, temperature %d\n", 
       testnum, test->inval, code, temp); 
    else 
    { 
     printf("FAIL %d: reading %d, code (got %d, wanted %d), " 
       "temperature (got %d, wanted %d)\n", 
       testnum, test->inval, code, test->rcode, temp, test->rtemp); 
     result = 1; 
    } 
}

然后主程序可以有一个循环驱动的测试功能：

#define DIM(x) (sizeof(x)/sizeof(*(x))) 

int failures = 0; 
int i; 

for (i = 0; i < DIM(test); i++) 
    failures += test_one(i + 1, &test[i]); 

if (failures != 0) 
    printf("!! FAIL !! (%d of %d tests failed)\n", failures, (int)DIM(test)); 
else 
    printf("== PASS == (%d tests passed)\n", (int)DIM(test));

现在，如果有任何测试出现问题，就很难原谅没有发现问题。用你的原始代码，有人可能会忽略这个错误。

很明显，如果您还想要关于测试的评论，您可以将const char *tag添加到阵列并提供并打印这些标签。如果你真的想变得很花哨，你甚至可以通过在数组中包含适当的初始化指针来编码空指针测试（包括这些测试的荣誉） - 你可以为'table is null'添加一对位标志并且'温度指针为空'以及函数中的条件代码。

来源

2010-09-04 16:03:07

嵌入式C代码审查

回答

相关问题