Shifting array elements leaves last elements blank

Question

i'm currently working on a project where I'm averaging the output of the last 10 to 20 measurements. to do this I'm saving the last 10 values in an array. right shifting the elements to update the data.

The code that I'm using to shift the values:

void shiftvals() {
    memmove(&val[1], &val[0], (sizeof(val) / sizeof(val[0]))-1);
    for (unsigned int i = 0; i < (sizeof(val) / sizeof(val[0])); i++)
    {
        Serial.print(val[i]);
        Serial.print(",");
    }
    Serial.println();
}

The code calling the function:

#define ARR_SIZE 10
uint16_t val[ARR_SIZE];

void loop(){
    Serial.print("Size of the array:\t");
    Serial.println(sizeof(val) / sizeof(val[0]));

    shiftvals();

    val[0] = (analogRead(A0));
}

Now the problem is that the last few outputs will always be 0, even though the array fills up nicely. When I increase the size of the array the amount of empty spaces also increase.

Output:

396,396,381,462,503,195,0,0,0,0,
472,472,396,381,462,247,0,0,0,0,
495,495,472,396,381,206,0,0,0,0,
435,435,495,472,396,125,0,0,0,0,

I'm seriously confused, what am I doing wrong in the memmove?

Please provide a [mcve], and what do you mean by *array fills up nicely* — Passer By, Jul 10 '17 at 10:42
the third parameter of memmove is Size_t, i figured that the sizing was off, but i didn't think of taking the defined size of the array as size_t as explained by michael @ passer by, i believe the output clarifies that point — Ron, Jul 10 '17 at 11:00
So, is C++ not an option? OP tagged this as C++ but then approved an edit to change it to C. — underscore_d, Jul 10 '17 at 11:07

Michaël Roy · Accepted Answer · 2017-07-10T10:53:59.380

2

The problem is in your call to memmove. Your sizing is too short.

void shiftvals() {
    memmove(&val[1], &val[0], (sizeof(val) / sizeof(val[0]))-1);

    // that was only 9 bytes, but val[] is 20 bytes long.

    // ...
}

Should read

void shiftvals() {
    memmove(&val[1], &val[0], (ARR_SIZE - 1) * sizeof(val[0]));
    // ...
}

edited Jul 10 '17 at 10:53

answered Jul 10 '17 at 10:48

Michaël Roy

6,338
1
15
19

score 2 · Answer 2 · answered Jul 10 '17 at 10:59

2

Do it the C++ way:

#include <iostream>
#include <array>
#include <algorithm>

std::array<int,10> val = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 };

void shiftVals()
{
  std::rotate( begin(val), end(val)-1, end(val));
}

int main()
{
  shiftVals();
  for(auto v: val ) std::cout << v << " ";
  std::cout << "\n";
}

Consider to not using global variables.

answered Jul 10 '17 at 10:59

i understand global variables are normally frowned upon, but i need this array to exist throughout the program, and i didn't want the additional class. keeping it in the loop would overwrite it. – Ron Jul 10 '17 at 11:03
1

The OP vs other users seem to be confused about whether C++ is an option or not... If it is, then this is far better code than that C-style mess. @RonaldPhilipsen Needing something to exist throughout the program does not mandate that it be global: you could it in a class or as a translate-unit-local (`static` or, in C++, an unnamed `namespace`) variable, and then pass it to wherever it is required; that is usually less difficult to reason about and less potentially dangerous than using a global. – underscore_d Jul 10 '17 at 11:06
1

@underscore_d yes, there was the "hint" C++ tag. Now it is C. – Jul 10 '17 at 11:07
@underscore_d fair point, it was c++ cause this code is running on an embedded system that is capable of running both, my preference goes to pure C however, which is why the tag was changed to reflect that – Ron Jul 10 '17 at 12:39

Shifting array elements leaves last elements blank

2 Answers2