1
\$\begingroup\$

I am working on an embedded systems project, specifically a line follower car. It has an emitter, a left detector and a right detector. I am working on an algorithm to find the maximum white value, so when the detector gets down to that value it will trigger white, and a minimum black value that will trigger black when it gets up to that value.

Right now, the detector reads values in the 70-80 range for white but could go as low as 40 and 400-500 for black but could go over 800. It is not required to write this algorithm, but I am trying to be efficient.

The microcontroller has two switches that we can use. My plan is to press switch 1 to begin calibrating the sensors with this algorithm I am writing. My idea is to place the car on a white surface and press the switch. After moving the car all around the white surface for 10 seconds, the code will store the maximum white value it encountered.

Then I'll move the car to calibrate black in the same manner: move it around the black line for 10 seconds and have the code store the minimum black value it encountered. After it has its maximum white value and minimum black value, I want it to display it to the screen.

I don't expect help with the display process, as that is beyond the scope of this question. I just need some feedback on my min/max process and if there is a more efficient way to go about it then I am all ears. I have posted the code that I wrote a couple of days ago.

// Function Used to Calibrate Detectors
void calibration(void){
  // These Variables are Declared as unsigned ints
  // My Ideas Is if It is Below 400 => It Will Be Trying to Detect White
  // And if It is Above 400 => It Will Be Trying to Detect Black
  minWhite_L = 400;
  minWhite_R = 400; 
  maxBlack_L = 400;
  maxBlack_R = 400;
  
  // Minimum White Value
  // ADC_Left_Detect Variable Holds the Value Read By the Left Detector
  // ADC_Right_Detect Variable Holds the Value Read By the Right Detector
  if((ADC_Left_Detect < 400) && (ADC_Right_Detect < 400)){
    if(ADC_Left_Detect < minWhite_L) { minWhite_L = ADC_Left_Detect; }
    if(ADC_Right_Detect < minWhite_R) { minWhite_R = ADC_Right_Detect; }
  }
  // Maximum Black Value
  if((ADC_Left_Detect >= 400) && (ADC_Right_Detect >= 400)){
    if(ADC_Left_Detect > maxBlack_L) { maxBlack_L = ADC_Left_Detect; }
    if(ADC_Right_Detect > maxBlack_R) { maxBlack_R = ADC_Right_Detect; }
  }
  // The Idea Was that First I Would Need the Absolute Minima and Maxima
  if(time_change){ // This is the Code to Wait for 10 Seconds for My Car
    time_change = 0;
    if(delay_start++ >= (10 * SEC_MULTIPLIER)){
      delay_start = 0;
      // Send Found Values to Another Function to Determine the Maximum White
      // And the Minimum Black Values => Which is What I am Really Looking For
      maxW_minB(minWhite_L,minWhite_R,maxBlack_L,maxBlack_R); 
    }
  }
}

void maxW_minB(unsigned int mwL,unsigned int mwR,unsigned int MBL,unsigned int MBR){
  int x; // Temporary Local Variable to Hold the Left White Value
  int y; // Temporary Local Variable to Hold the Right White Value
  
  // Maximum White Value
  if((ADC_Left_Detect < 400) && (ADC_Right_Detect < 400)){
    if(ADC_Left_Detect > mwL) { maxWhite_L = ADC_Left_Detect; }
    if(ADC_Right_Detect > mwR) { maxWhite_R = ADC_Right_Detect; }
  }
  // Minimum Black Value
  if((ADC_Left_Detect >= 400) && (ADC_Right_Detect >= 400)){
    if(ADC_Left_Detect < MBL) { minBlack_L = ADC_Left_Detect; }
    if(ADC_Right_Detect < MBR) { minBlack_R = ADC_Right_Detect; }
  }
  
  if(time_change){ // This is the Code to Wait for 10 Seconds for My Car
    time_change = 0;
    if(delay_start++ >= (10 * SEC_MULTIPLIER)){
      delay_start = 0;
      // Just One Value for the Maximum White
      // 'HextoBCD' Takes an int Argument, so I typecast
      // 'HextoBCD' Converts an int to a Hex Value to Display on the LCD
      if(maxWhite_L > maxWhite_R) { x = (int)maxWhite_L; HEXtoBCD(x); }
      else{ y = (int)maxWhite_R; HEXtoBCD(y); }
    }
  }
  
  if(P6IN & IR_LED) { strcpy(display_line[0], "Emit: ON  "); }
  else { strcpy(display_line[0], "Emit: OFF "); }
  strcpy(display_line[1], "MaxW:     "); // Display Max White Value
  strcpy(display_line[2], "MinB:     "); // Display Min Black Value
  strcpy(display_line[3], "          ");
  adc_line(1,6); // This Will Start Printing My Value on the First Row, Sixth Column
  adc_line(2,6); // This Will Start Printing My Value on the Second Row, Sixth Column
  display_changed = TRUE;
  
  if(time_change){ // The Idea is to Display the End Result for 10 Seconds
    time_change = 0;
    if(delay_start++ >= (10 * SEC_MULTIPLIER)){
      delay_start = 0;
      event = NONE;
    }
  }
}
\$\endgroup\$
1
  • \$\begingroup\$ There are quite a lot of definitions missing from this code; it would be helpful to see where ADC_Left_Detect etc are declared, as well as SEC_MULTIPLIER, HEXtoBCD etc. - or restructure the code so that all these external identifiers are separate from the algorithm function, so it can be tested in isolation. \$\endgroup\$ Mar 14, 2022 at 7:54

1 Answer 1

2
\$\begingroup\$

Avoid hardcoded assumptions

You are already hardcoding the assumption that anything below 400 is white, and anything above it is black. Ideally, the calibration procedure does not depend on that assumption. If indeed you first put the car onto a white surface and press the button to calibrate what white is, you just need a function to get the maximum value over a period of ten seconds. Then you have another function that calibrates black by getting the minimum value over the same period. Finally, you can do something like calculating the midpoint between the maximum white value and the minimum black value, and use that as the crossover point for deciding whether you are on a white or a black surface.

Consider recording a histogram

If you want to have a one-shot calibration routine, where the car will drive over both white and black regions, and you don't know in advance where the crossover point will be, then consider just recording the measured values into a histogram. For example, if you have a 10-bit ADC and you have enough RAM to store 2 * 1024 integers, then you could write something like:

unsigned int histogram_L[1024] = {0};
unsigned int histogram_R[1024] = {0};

for (int i = 0; i < 10 * SEC_MULTIPLIER; i++) {
    histogram_L[ADC_Left_Detect]++;
    histogram_R[ADC_Right_Detect]++;
    /* delay here */
}

The advantage of this is that you don't have to do any calculations while you are taking the data. You can process the histogram afterwards. For example, you could use \$k\$-means clustering with \$k = 2\$, or look at some other ways to find the peaks in a bimodal histogram.

If you don't have enough RAM, then you could reduce the resolution of your histogram (for example, divide all measured values by 8 to reduce RAM usage by 8) at the cost of a bit less precision in the measured peaks, but if the expected difference between black and white is about 400, that should not be a problem.

Reduce code duplication

There is a lot of duplication in your code; you want to find both the minimum and maximum of each of white and black, and you have both left and right sensors. Try to find some ways to avoid having to repeat yourself. For example, create a struct that holds the state for a single sensor, and write a function that updates the state of one sensor, using a pointer to the struct and the value of the sensor as parameters:

struct MinMax {
    unsigned int min;
    unsigned int max;
};

struct SensorRange {
    struct MinMax white;
    struct MinMax black;
};

static const unsigned int crossOver = 400;

static void updateMinMax(struct MinMax *minMax, unsigned int value) {
    if (value < minMax->min) minMax->min = value;
    if (value > minMax->max) minMax->max = value;
}

static void updateRange(struct SensorRange *range, unsigned int value) {
    updateMinMax(value < crossOver ? &range->white : &range->black, value);
}

void calibrate() {
    struct SensorRange leftRange = {{1023, 0}, {1023, 0}};
    struct SensorRange rightRange = {{1023, 0}, {1023, 0}};
    ...
    updateRange(&leftRange, ADC_Left_Detect);    
    updateRange(&rightRange, ADC_Right_Detect);
    ...
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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