RobTillaart / RunningAverage

Arduino library to calculate the running average by means of a circular buffer.
MIT License
53 stars 10 forks source link

Last values could be expanded to subsection of the buffer #19

Closed jokohoko closed 2 years ago

jokohoko commented 2 years ago

Instead of only providing statistics on the last n elements of the buffer, these functions could return info on a subset of the buffer. Like: float getAverageLast(uint16_t start, uint16_t count) get the average of count elements after start.

Making the functions overflow so that one value gives the last elements and two values gives this subset. Would be useful to compare various regions in the buffer and also when doing a running average comparison between now and x amount of recordings ago. I now used to store multiple rolling buffers to do this.

I might have a crack at it later and do a pull request. But now have to finish my project and this way i do not forget the idea!

RobTillaart commented 2 years ago

Thanks for the ideas, A PR would be great of course!

(added bold tags to the function so it pops out)

RobTillaart commented 2 years ago

Some points:

Should name be getAverageSubSet(start, count) to indicate the difference Note if count == size of internal buffer the subset is the same as all elements. So no need for a count default imho.

RobTillaart commented 2 years ago

something like this?

// returns NAN if no elements.
float RunningAverage::getAverageSubset(uint16_t start, uint16_t count)
{
  if (_count == 0)
  {
    return NAN;
  }

  uint16_t cnt = _count;
  if (cnt > count) cnt = count;

  float sum = 0;   //  do not disrupt global _sum
  for (uint16_t i = 0; i < cnt; i++)
  {
    uint16_t idx = _index + start + i;
    while (idx >= _partial) idx -= _partial;
    sum += _array[idx];
  }
  return sum / cnt;
}
jokohoko commented 2 years ago

I made this for my project back then. But still have to verify if it works. (ended up using a different method to track changes over time). Using the same methodology as i found in the getAverageLast function before.

float RunningAverage::getAverageLast(uint16_t count){
    getSubsetAverage(_count-count, count);
}

float RunningAverage::getSubsetAverage(uint16_t start, uint16_t count)
{
    uint16_t cnt = count;
    if (cnt > _count) cnt = _count;
    if (cnt == 0) return NAN;

    float sum = 0;
    uint16_t idx = _index + start + count;
    idx = idx % _size;
    for (uint16_t i = 0; i < cnt; i++)
    {
        if (idx == 0) idx = _size;
        idx--; 
        sum += _array[idx];
    }
    return sum / cnt;
}

Main reason i did not post it yet and wanted to test was to see if the idx--; had to be before or after the sum to be correct. And i was trying to make both function the same code. With one calling the other.

But you choose as developer obviously. To me these functions for subset average, subset min and max would be a great addition.

RobTillaart commented 2 years ago

The best way is to write a test sketch that verifies some known values and compare the old functions with the new wrapper based.

Also important is to check border cases in such sketch. Think of count is zero, no elements added, less elements than requested. I try to have such checks in the unit test in TEST folder. If you create a PullRequest and add these tests too, the new functions are automatically verified with every change/ release.

RobTillaart commented 2 years ago

@jokohoko Please add examples to the Pull Request too.

RobTillaart commented 2 years ago

@jokohoko Any progress to report?

RobTillaart commented 2 years ago

@jokohoko As this issue is open for about a month without PR from your side I close it. Feel free to reopen it if you have your PR ready.