0

Here is the struct:

struct RGB {
  byte r;
  byte g;
  byte b;
  byte v;
};

And here is the sorting function:

void insertion_sort(RGB arr[], size_t capacity) {
  RGB temp;
  size_t j;
  for(size_t i = 1; i < capacity; i++) {
    temp = arr[i];
    j = i - 1;
    while(j >= 0 && arr[j].v > temp.v) {
      arr[j+1] = arr[j];
      j--;
    }
    arr[j+1] = temp;
  }
}

the v member of the RGB struct is the object's position in the array before scrambling (and hopefully where it ends up after sorting). Here is the output when printing the v member of every element of the array:

165 171 53 164 171 13 13 167 156 168 163 14 15 16 17 18 19 20 20 156 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 142 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149

I get the same output every time.

There are 150 elements in the array, and everything seems fine after 21. The issue is that the elements before that are wrong, and also made up of values higher than 149. Currently, the b and g members of RGB are all 0 throughout the array, and r is evenly-spaced values from 0-254.

It should be noted that the exact same sorting function works when I recreated everything in C++, and it even works to sort an array of ints in Arduino. The only issue is sorting an array of RGB structs, which seems to be causing memory issues.

Cole Gravelle
  • 1
  • 1
  • say that `size_t j == 0`. You subtract 1 from it. What's the new value? – JohnFilleau Mar 31 '20 at 01:55
  • Also, for my own edification: does Arduino have any sort of debug interface you could use to step through this? – JohnFilleau Mar 31 '20 at 01:56
  • 1
    What level of C++ compiler does Arduino use? Is `std::sort` available? – PaulMcKenzie Mar 31 '20 at 02:13
  • @PaulMcKenzie the reason I am writing this function is to make a sorting algorithm visualizer with an LED strip. I have other sorting algorithms implemented that work fine, I just seem to be having some kind of memory issue with insertion sort. – Cole Gravelle Apr 01 '20 at 03:09
  • @ColeGravelle [See this insertion sort implementation](https://stackoverflow.com/questions/24650626/how-to-implement-classic-sorting-algorithms-in-modern-c). That's the reason why I asked what level of compiler you're using. Then using what is on that page: `insertion_sort(arr, arr + capacity, [](RGB &r1, RGB &r2) { return r1.v < r2.v;});` – PaulMcKenzie Apr 01 '20 at 03:11
  • @JohnFilleau in the first iteration of the outer loop (i == 1, j == 0), the inner while loop will be entered if arr[0] > arr[1], in which case arr[1] will be set to the value of arr[0], then j will decrement to -1, and the loop will not be re-entered. Then, arr[0] will be set to the value of temp, which is the original value of arr[1]. It is basically an over-complicated swap on the first two elements. – Cole Gravelle Apr 01 '20 at 03:13
  • @ColeGravelle that's wrong. `size_t` is an unsigned type. `0 - 1` underflows to the maximum positive number. `j` will always be greater than or equal to 0. – JohnFilleau Apr 01 '20 at 12:41

2 Answers2

0

Your problem is the definition of j.
Your options are:

  • Serial output in critical lines and analyze the serial monitor -> Arduinos way of debuging
  • using one of the hundred tested sorting functions (e.g. here on stack exchange) like quick sort etc.
  • using the std::sort

For the last option you have to:
The best solution is to use the C++ standard library function std::sort. This function is type-safe, and doesn't require you to cast anything to void * and back, and you don't have to manually enter the sizes of the array and its elements.

 #include < Arduino_Helpers.h\>
 #include < AH/STL/algorithm\>
 #include < AH/STL/iterator\>

 auto cmpfunc = [](RGB A, RGB B) { return A.value < B.value; }; 

// or if you do not like this notation use
 bool cmpfunc(RGB A, RGB B) {
    return A.value < B.value;
 }

std::sort(std::begin(RGB arr[]), std::end(RGB arr[]), cmpfunc); // Dummy for a RGB arr[] containing all your values
  -
Codebreaker007
  • 2,911
  • 1
  • 10
  • 22
  • I cannot use std::sort because I am writing this as a sorting algorithm visualizer using an LED strip to show different sorting algorithms. I already have other sorting algorithms implemented correctly, I just seem to be having a memory issue when insertion sort is used to sort an array of structs. As I have mentioned, the exact same function works to sort an array of ints (all I did to test this was change 'RGB' to 'int' in the function). It also sorts an array of structs properly as-is with no modification when compiled in C++. – Cole Gravelle Apr 01 '20 at 03:22
  • Okay so its solution one debugging - Please give the compilable minimum code to reproduce this error (edit your question). I think I have a clue what it might be but need to test it. – Codebreaker007 Apr 01 '20 at 08:04
0

The problem was because of the size_t datatype, which is unsigned and could not become a value < 0 when it was supposed to. Thank you @JohnFilleau

Cole Gravelle
  • 1
  • 1