Help passing array element of struct to function[Solved]

I’m trying to write a program that uses an array of structs that each contain several identically sized arrays (raw readings). I have a function (moving average) that I want to apply to each of these arrays at various points in my main loop, but there must be something I’m not understanding about passing an array element of a struct as a parameter. Here’s the relevant code:

// all this is in the global space - it's defined at the beginning of the sketch

#define CURRENT_AVERAGE 3

  struct Valve_rec
  {
    int ValveCurA[CURRENT_AVERAGE+1];   // latest VVS valve current test readings in A direction
    int ValveCurB[CURRENT_AVERAGE+1];   // latest VVS valve current test readings in B direction
    int BlockCurA[CURRENT_AVERAGE+1];   // latest block valve current test readings in open direction
    int BlockCurB[CURRENT_AVERAGE+1];   // latest block valve current test readings in shut direction
  };
  Valve_rec Valves[7];


int MoveAverage(int NewValue, int Values[], byte Count)
// insert new value into Values array, compute average, and leave in Values[0] (and return it)
{
  byte Iter;
  int Result = 0;

  for (Iter = 1; Iter += 1; Iter < Count)
  {
    Result += Values[Iter];
    Values[Iter + 1] = Values[Iter];
  }
  Values[1] = NewValue;
  Result += NewValue;
  Values[0] = Result / Count;
  return Values[0];
}


void loop()

  int Value, Current;
  static byte ThisScan = 1;


// and somewhere buried in the main loop...
// (ValveIO is const array of struct that contains the pin numbers for
// each of the external connections, which works fine.)


    Value = analogRead(ValveIO[ThisScan].Curr_pin);
    Current = MoveAverage(Value, Valves[ThisScan].ValveCurA, CURRENT_AVERAGE);

I also tried explicitly providing the base address of the array, calling the function thus:

   Current = MoveAverage(Value, &Valves[ThisScan].ValveCurA[0], CURRENT_AVERAGE);

Running this program, using either method of passing the array, seems to be stomping on some memory somewhere, because another part of the program that was working perfectly is now having its variables corrupted; if I comment out the array writes in MoveAverage() (and make no other changes), that part works fine again. Also, the values I get for my resultant moving average are all over the place, and bear no resemblance to my actual data readings (meaning again that the array element references are probably pointing off into space).

(I understand that this isn’t the most efficient way to do a moving average; I could use a circular buffer instead of shifting the whole buffer around. I’ve got plenty of (CPU) time right now, and want to make it work before optimizing it. I’m also aware of the issues inherent in using ints rather than floats for my calculations. Again, that’s not my current issue.)

So, what am I doing wrong? It’s quite possible that I just have some logic flaw in my MoveAverage routine that I’m not seeing - it certainly wouldn’t be unusual for me. I’ve looked at it a bunch of times, though, and it looks OK to me. I’ve read that struct variables are passed by value, not reference, but I figured that arrays in a struct would behave like arrays anywhere else…

Thanks in advance for any help!
~~
Mark

Have you tried: Valves[ThisScan].ValCurrA ?

  for (Iter = 1; Iter += 1; Iter < Count)

That is NOT how a for loop is written.

No one can help you fix problems in parts of the code that is not posted.

And, array indexes start at 0, not 1.

PaulS:
And, array indexes start at 0, not 1.

You are right about the erroneous for loop, but in this case index 0 is used to store the average of the other indicies, so it has to start from 1 :slight_smile:

for (Iter = 1; Iter < Count; Iter++) //The right for loop..

@arduino_new: I berlieve that's the last line of my first code snippet.

@Danois: I believe the relevant portions of the code are posted.

@PaulS: Actually, that's a perfectly valid form of a for loop. The three items in a for loop are, respectively, the initialization statement, the loop test, and the loop "increment" statement. The reason I quoted "increment" is that any valid statement (include a compound statement) is legal. The way I write the loop is clearer to me, and in fact has less potential for unintended side effects.

Also @PaulS: Yes, I'm familiar with C / C++ arrays. I'm using element 0 of the array to hold the final (averaged) value; the individual readings are stored in elements 1..CURRENT_AVERAGE (as noted by @Danois90).

So, I've found the problem (with the help of another set of eyes). The second form of my call (passing the address of the element explicitly) was closer to correct, except that I forgot that the & operator binds most strongly, so it was providing the address of the structure array element, not the element of the array in the structure. A pair of parentheses solved the problem:

    Current = MoveAverage(Value, &(Valves[ThisScan].ValveCurA[0]), CURRENT_AVERAGE);

Thanks for taking the time to read and comment - I do appreciate the help!
~~
Mark

And yes, I see now what was wrong with the for loop - sorry for my density earlier…

markesys:

Current = MoveAverage(Value, &(Valves[ThisScan].ValveCurA[0]), CURRENT_AVERAGE);

The problem with your "MoveAverage" method is that is does not take "Values" as a "changeable argument". Either you need to pass the reference or a pointer to the array in order to be able to modify it within the method.

int MoveAverageA(int Value, int &Values[], byte Count) {};
int MoveAverageB(int Value, int *Values, byte Count) {};

Current = MoveAverageA(Value, Valves[ThisScan].ValveCurA, CURRENT_AVERAGE);
Current = MoveAverageB(Value, Valves[ThisScan].ValveCurA, CURRENT_AVERAGE);

Both methods should work with a simpler syntax.