Help Refine Code

Using Arduino Programming Questions
1

Gentlemen,

This is a small portion of code that I need to improve the efficiency with. See Below and note that every bit of code is too large to post ...

static const uint32_t USAband[13][3] = {{180000, 200000, 350000},
                                                        {350000, 400000, 533050},
                                                        {533050, 540350, 700000},
                                                        {700000, 730000, 1010000},
                                                        {1010000, 1015000, 1400000},
                                                        {1400000, 1435000, 1806800},
                                                        {1806800, 1816800, 2100000},
                                                        {2100000, 2145000, 2489000},
                                                        {2489000, 2499000, 2696500},
                                                        {2696500, 2740500, 2800000},
                                                        {2800000, 2970000, 5000000},
                                                        {5000000, 5400000, 5400000},
                                                        {0, 0, 0}};
static inline void bandplan() {
  goTo(0x10);
  static const char *band_print[] = {"160M",
                                                 " 80M",
                                                 " 60M",
                                                 " 40M",
                                                 " 30M",
                                                 " 20M",
                                                 " 17M",
                                                 " 15M",
                                                 " 12M",
                                                 " 11M",
                                                 " 10M", 
                                                 "  6M",
                                                 "    "};
  bool stopped = false;
  for(uint8_t band_search=0; !stopped; ++band_search) {
  if(freqMEM[0] > (USAband[band_search]  [0]-1) && 
     freqMEM[0] < (USAband[band_search][1]+1) 
     || band_search == 12) {stopped = true;
                                       Serial3.write(band_print[band_search]);}}
}

How it works... the global variable freqMEM[0] contains a uint32_t value AKA the frequency and the function bandplan() looks to see what the frequency falls within and prints what band that it belongs in. Note, the multi dimensional array USAband[13][3] holds the boundary numbers.

Is there a way to get around the for loop and the nested if statement? I feel that this is very inefficient and could be improved upon but would like an opinion or for some one to show be a better way to do this.

FYI, I am using a Teensy 3.1 with the Arduino IDE.

Thanks in advance
Bryan

2

How about this PROGMEM version?

const uint32_t USAband[] PROGMEM = {
  180000UL, 350000UL, 533050UL, 700000UL, 1010000UL, 1400000UL,
  1806800UL, 2100000UL, 2489000UL, 2696500UL, 2800000UL, 5000000UL, 5400000UL
};
const char band_print[] PROGMEM = "    160M 80M 60M 40M 30M 20M 17M 15M 12M 11M 10M  6M    ";

void bandplan(uint32_t value) {
  uint8_t index = 0;
  for (size_t tIdx = 0; tIdx < sizeof(USAband) / sizeof(USAband[0]); tIdx++) {
    if (value <= pgm_read_dword(&USAband[tIdx])) {
      break;
    } else {
      index += 4;
    }
  }
  Serial.write((char)pgm_read_byte(&band_print[index]));
  Serial.write((char)pgm_read_byte(&band_print[index + 1]));
  Serial.write((char)pgm_read_byte(&band_print[index + 2]));
  Serial.write((char)pgm_read_byte(&band_print[index + 3]));
}

void testit(uint32_t value) {
  Serial.print(F("Val "));
  Serial.print(value);
  Serial.print(F(" -> '"));
  bandplan(value);
  Serial.println(F("'"));
}
void setup() {
  Serial.begin(115200);
  testit(100000UL);
  testit(180500UL);
  testit(350500UL);
  testit(533550UL);
  testit(700500UL);
  testit(1015000UL);
  testit(1405000UL);
  testit(1856800UL);
  testit(2105000UL);
  testit(2489500UL);
  testit(2696550UL);
  testit(2805000UL);
  testit(5005000UL);
  testit(5405000UL);
}
void loop() {}

Val 100000 -> '    '
Val 180500 -> '160M'
Val 350500 -> ' 80M'
Val 533550 -> ' 60M'
Val 700500 -> ' 40M'
Val 1015000 -> ' 30M'
Val 1405000 -> ' 20M'
Val 1856800 -> ' 17M'
Val 2105000 -> ' 15M'
Val 2489500 -> ' 12M'
Val 2696550 -> ' 11M'
Val 2805000 -> ' 10M'
Val 5005000 -> '  6M'
Val 5405000 -> '    '
3

I feel that this is very inefficient and could be improved

Why do you feel that way? There is nothing inherently inefficient about a for loop or an if statement. Any other construct is simply going to hide the fact that it iterates over the array and determines if a value is in the range defined by that element of the array.

Whandall's code simply changes where the data is stored. It uses the same "iterate over an array" process.

The only real difference, which you could incorporate in your code, is that it stops iterating over the array when it finds the right element.

4

Apart from the big memory footprint difference there is a functional difference.

Der Sketch verwendet 2.744 Bytes (8%) des Programmspeicherplatzes. Das Maximum sind 30.720 Bytes.
Globale Variablen verwenden 450 Bytes (21%) des dynamischen Speichers, 1.598 Bytes für lokale Variablen verbleiben. Das Maximum sind 2.048 Bytes.

Der Sketch verwendet 2.574 Bytes (8%) des Programmspeicherplatzes. Das Maximum sind 30.720 Bytes.
Globale Variablen verwenden 200 Bytes (9%) des dynamischen Speichers, 1.848 Bytes für lokale Variablen verbleiben. Das Maximum sind 2.048 Bytes.

Same test data as before presented to the original code gives

Val 100000 -> '    '
Val 180500 -> '160M'
Val 350500 -> ' 80M'
Val 533550 -> ' 60M'
Val 700500 -> ' 40M'
Val 1015000 -> ' 30M'
Val 1405000 -> ' 20M'
Val 1856800 -> '    '
Val 2105000 -> ' 15M'
Val 2489500 -> ' 12M'
Val 2696550 -> ' 11M'
Val 2805000 -> ' 10M'
Val 5005000 -> '  6M'
Val 5405000 -> '    '

The used code (I had to comment out the goTo(x10) )

uint32_t freqMEM[] = { 2145000, };

static const uint32_t USAband[13][3] = {{180000, 200000, 350000},
  {350000, 400000, 533050},
  {533050, 540350, 700000},
  {700000, 730000, 1010000},
  {1010000, 1015000, 1400000},
  {1400000, 1435000, 1806800},
  {1806800, 1816800, 2100000},
  {2100000, 2145000, 2489000},
  {2489000, 2499000, 2696500},
  {2696500, 2740500, 2800000},
  {2800000, 2970000, 5000000},
  {5000000, 5400000, 5400000},
  {0, 0, 0}
};

static inline void bandplan() {
  //  goTo(0x10);
  static const char *band_print[] = {"160M",
                                     " 80M",
                                     " 60M",
                                     " 40M",
                                     " 30M",
                                     " 20M",
                                     " 17M",
                                     " 15M",
                                     " 12M",
                                     " 11M",
                                     " 10M",
                                     "  6M",
                                     "    "
                                    };
  bool stopped = false;
  for (uint8_t band_search = 0; !stopped; ++band_search) {
    if (freqMEM[0] > (USAband[band_search]  [0] - 1) &&
        freqMEM[0] < (USAband[band_search][1] + 1)
        || band_search == 12) {
      stopped = true;
      Serial.write(band_print[band_search]);
    }
  }
}

void testit(uint32_t value) {
  Serial.print(F("Val "));
  Serial.print(value);
  Serial.print(F(" -> '"));
  freqMEM[0] = value;
  bandplan();
  Serial.println(F("'"));
}

void setup() {
  Serial.begin(115200);
  testit(100000UL);
  testit(180500UL);
  testit(350500UL);
  testit(533550UL);
  testit(700500UL);
  testit(1015000UL);
  testit(1405000UL);
  testit(1856800UL);
  testit(2105000UL);
  testit(2489500UL);
  testit(2696550UL);
  testit(2805000UL);
  testit(5005000UL);
  testit(5405000UL);
}
void loop() {}
5

The original version compares the wrong value (unused mid-value) as upper bound, not the real upper bound.

  //freqMEM[0] < (USAband[band_search][1] + 1)
  freqMEM[0] < (USAband[band_search][2] + 1)

I would prefer writing it like this

  freqMEM[0] <= USAband[band_search][2]