analog pin smoothing function gives strange results

This was originally posted in Teensy’s forum, as I’m using a Teensy in this specific project, but I got no answers there (it’s been there for a few days), so though of posting here (the code is 100% Arduino).

I’m trying to eliminate some noise I get in the analog pins of a Teensy 3.1 by writing this code

#define SMOOTH 100

const byte rows = 3;
const byte cols = SMOOTH;

int last_col = SMOOTH - 1;

unsigned int pots[rows][cols];

const byte num_of_data = (rows * 2) + 1;
byte transfer_data[num_of_data];

unsigned int smooth(int row_index)
{
  unsigned int smoothed;
  unsigned long accumulate;
  // accumulate analog readings
  for(int i = 0; i < SMOOTH; i++)
    accumulate += pots[row_index][i];
  // divide readings sum by the number of stored values  
  smoothed = accumulate / SMOOTH;
  
  return smoothed;
}

void setup()
{
  for(int i = 0; i < rows; i++){
    for(int j = 0; j < cols; j++){
      pots[i][j] = 0;
    }
  }
  
  analogReadResolution(13);
  
  Serial.begin(115200);
}

void loop()
{
  transfer_data[0] = 0xc0;
  int index = 1;
  
  for(int i = 0; i < rows; i++){
    for(int j = 0; j < last_col; j++) pots[i][j] = pots[i][j + 1];
    
    pots[i][last_col] = analogRead(i);
    
    unsigned int smoothed = smooth(i);
    transfer_data[index++] = smoothed & 0x007f;
    transfer_data[index++] = smoothed >> 7;
  }
  
  Serial.write(transfer_data, num_of_data);
}

which works fine. But when I put it in a greater project (the version above is for testing only), I get the following strange behavior. The first potentiometer is being read fine, but all the rest have their resolution reduced according to the number of samples I take (the SMOOTH macro). If I take 2 samples, the maximum value the pots give is 4095. If I take 3 samples, it’s 2730. If I take 4, it’s 2047 etc. If I take 1 sample it’s 8191, as it should (13-bit resolution).
I have to mention that in the greater project I’m using multiplexers. A main multiplexer is controlling other multiplexers where the pots are wired.

Posting part of the code of the greater project.
Global variables used for the analog pins are the following

#define MASTER_CONTROL0 5
#define MASTER_CONTROL1 4
#define MASTER_CONTROL2 3
#define MASTER_CONTROL3 2

#define SLAVE_CONTROL0 8
#define SLAVE_CONTROL1 7
#define SLAVE_CONTROL2 6

#define SMOOTH 40

// set global variables for multiplexers
// master multiplexers controlling the multiplexers of each module
const byte num_of_master_mux = 1;
// number of slave multiplexers
int num_of_slave_mux[num_of_master_mux] = {3};
// two dimensional array to hold number of pins used on each slave multiplexer
// rows = num_of_master_mux, columns = greatest number in num_of_slave_mux array
int num_of_pots[num_of_master_mux][3] = { { 7, 4, 1 } };
const int total_pots = 12; // sum of elements of num_of_pots
// array to store multiple readings of each knob for smoothing
unsigned int multiple_pots[total_pots][SMOOTH];
// last element index of each row of the two dimensional array
// just so that you don't need to subtract one from SMOOTH in each loop
int last_col = SMOOTH - 1;

The smoothing function is the same. It’s this

unsigned int smooth(int row_index)
{
  unsigned int smoothed;
  unsigned long accumulate;
  // accumulate analog readings
  for(int i = 0; i < SMOOTH; i++)
    accumulate += multiple_pots[row_index][i];
  // divide readings sum by the number of stored values  
  smoothed = accumulate / SMOOTH;
  
  return smoothed;
}

and the for loop inside void loop where I read the pots, is this

transfer_data[0] = 0xc0; // denote start of data stream
int index = 1; // transfer data array index offset

// read potentiometers
int row_index = 0;
// run througn all master multiplexers
for(int master_mux = 0; master_mux < num_of_master_mux; master_mux++){
  // first move the elements of current row one position to the left so we can add the new value at the end
  for(int i = 0; i < last_col; i++) multiple_pots[row_index][i] = multiple_pots[row_index][i + 1];
  // run through all slave multiplexers
  for(int slave_mux = 0; slave_mux < num_of_slave_mux[master_mux]; slave_mux++){
    digitalWrite(MASTER_CONTROL0, (slave_mux&15)>>3);
    digitalWrite(MASTER_CONTROL1, (slave_mux&7)>>2);
    digitalWrite(MASTER_CONTROL2, (slave_mux&3)>>1);
    digitalWrite(MASTER_CONTROL3, (slave_mux&1));
    // run through the pins used on each slave multiplexer
    for(int pot = 0; pot < num_of_pots[master_mux][slave_mux]; pot++){
      digitalWrite(SLAVE_CONTROL0, (pot&7)>>2);
      digitalWrite(SLAVE_CONTROL1, (pot&3)>>1);
      digitalWrite(SLAVE_CONTROL2, (pot&1));
      // store new reading to the last element of pots row
      multiple_pots[row_index][last_col] = analogRead(master_mux);
      // call the smoothing function
      unsigned int smoothed = smooth(row_index);
      // and store the smoothed value to the transfer_data array
      transfer_data[index++] = smoothed & 0x007f;
      transfer_data[index++] = smoothed >> 7;
      row_index++;
    }
  }
}

And finally I call Serial.write to write that data along with other stuff. Adding delayMicroseconds doesn’t change anything. I can’t find why is that, can anyone help? I’m getting the data either on OS X 10.8.5 or on an Odroid-U3, in Pd-0.46.

Thanks

But when I put it in a greater project

Which uses more RAM, perhaps?

void setup()
{
  for(int i = 0; i < rows; i++){
    for(int j = 0; j < cols; j++){
      pots[i][j] = 0;
    }
  }

Waste of time and code - they’re all already zero.

I think that in smooth() you should initialize 'accumulate' to 0. Local automatic variables are not initialized.

johnwasser: I think that in smooth() you should initialize 'accumulate' to 0. Local automatic variables are not initialized.

Good spot!

Was working on this as other replies came in. Uses less ram, check the change for your 2nd transfer near the end of the code:

#define SMOOTH 100

const byte rows = 3;
const byte cols = SMOOTH;

byte last_col = SMOOTH - 1;

unsigned short pots[rows][cols];

const byte num_of_data = (rows * 2) + 1;
byte transfer_data[num_of_data];

unsigned short smooth(byte row_index)
{
  unsigned short smoothed;
  unsigned long accumulate;
  // accumulate analog readings
  for(int i = 0; i < SMOOTH; i++)
    accumulate += pots[row_index][i];
  // divide readings sum by the number of stored values  
  smoothed = accumulate / SMOOTH;
  
  return smoothed;
}

void setup()
{
  for(int i = 0; i < rows; i++){
    for(int j = 0; j < cols; j++){
      pots[i][j] = 0;
    }
  }
  
  analogReadResolution(13);
  
  Serial.begin(115200);
}

void loop()
{
  transfer_data[0] = 0xc0;
  byte index = 1;
  
  for(int i = 0; i < rows; i++){
    for(int j = 0; j < last_col; j++) pots[i][j] = pots[i][j + 1];
    
    pots[i][last_col] = analogRead(i);                  //13-bit
    
    unsigned short smoothed = smooth(i);
    transfer_data[index++] = smoothed & 0x007f;         //7-bit
    transfer_data[index++] = (smoothed >> 7) & 0x003f;  //6-bit
  }
  
  Serial.write(transfer_data, num_of_data);
}

EDIT: Why not transfer 8-bit in the first byte transfer, then 5-bit in the second byte transfer?

I'm using a Teensy in this specific project

Which one? 3.1? Something else?

AWOL: Which uses more RAM, perhaps?

When I upload the version with the multiplexers, I get this

Estimated memory use: 3,732 bytes (of a 65,536 byte maximum)

This is about the RAM, isn't it? If so, doesn't look problematic.

I think that in smooth() you should initialize 'accumulate' to 0. Local automatic variables are not initialized.

Doesn't work either..

Why not transfer 8-bit in the first byte transfer, then 5-bit in the second byte transfer?

The code you refer to, works fine (it's the testing version). It doesn't work when the multiplexers come in. Also, if I transfer an 8-bit number first, it's very likely to get the same value with the first element of the array that that denotes the beginning of the data stream (0xc0), so the laptop that receives the data won't know where to start (I'm collecting data in Pure Data and this seems the best way to receive data from Arduino, as ASCII is pretty tricky).

Which one? 3.1? Something else?

Yup, it's 3.1.

Doesn't work either..

I don't see your updated code.

AWOL: I don't see your updated code.

I didn't update it, I've tried this before and didn't make any difference.

OK. Thanks for letting us know. Good luck.

AWOL:
OK.
Thanks for letting us know.
Good luck.

What I meant by that is that initializing the accumulate local variable doesn’t solve my problem, and that I have already tried that.
The updated code for this would be the following

// analog readings smoothing function
unsigned int smooth(int row_index)
{
  unsigned int smoothed;
  unsigned long accumulate = 0;
  // accumulate analog readings
  for(int i = 0; i < SMOOTH; i++)
    accumulate += multiple_pots[row_index][i];
  // divide readings sum by the number of stored values  
  smoothed = accumulate / SMOOTH;
  
  return smoothed;
}

by reading this post in this thread

johnwasser:
I think that in smooth() you should initialize ‘accumulate’ to 0. Local automatic variables are not initialized.

one can understand what doesn’t work. I appreciate people replying here, and the fact that you guys are trying to help, but didn’t think posting this little detail (which is clear from the conversation, I think), would help a great deal.

Anyway, I’m narrowing down the problems and I think it’s something with the multiplexers. I guess I’ll have to go by myself, done it before.
Thanks guys

but didn't think posting this little detail (which is clear from the conversation, I think), would help a great deal.

It helps us to know that you're at least heading in the right direction. As a low post count member here, we have no way of knowing if you're done what we expected, or implemented it in some other way.

I wouldn't, for instance, expect an experienced person to move large amounts of data around in an array and perform hundreds of unnecessary arithmetic operations, when a simple circular buffer would reduce the processor load hugely.

Then there's this

    transfer_data[index++] = smoothed & 0x007f;
    transfer_data[index++] = smoothed >> 7;

Odd.

There's even a whole website dedicated to this approach to posting here

The code you refer to, works fine (it's the testing version). It doesn't work when the multiplexers come in. Also, if I transfer an 8-bit number first, it's very likely to get the same value with the first element of the array that that denotes the beginning of the data stream (0xc0), so the laptop that receives the data won't know where to start (I'm collecting data in Pure Data and this seems the best way to receive data from Arduino, as ASCII is pretty tricky).

I'm not familiar with Pure Data, but is it possible to use an I/O for handshaking?

Looks like as you have it, while in Pure Data, you'll need to bit-shift and manipulate the data to re-build the 13-bit readings.

Also, are you sure the data is clean? Why use "unsigned int"? (unsigned ints are 2-bytes for ATMEGA based boards, 4-bytes on the Due)

Since accumulate is unsigned long and you have "smoothed = accumulate / SMOOTH;" I would make all data variables unsigned long.

Then this should ensure only the desired bits are being sent:

transfer_data[index++] = smoothed & 0x0000007F;         //7-bit
transfer_data[index++] = (smoothed >> 7) & 0x0000003F;  //6-bit
// or...
transfer_data[index++] = smoothed & 0x000000FF;         //8-bit
transfer_data[index++] = (smoothed >> 8) & 0x0000001F;  //5-bit

dlloyd: I'm not familiar with Pure Data, but is it possible to use an I/O for handshaking?

I guess it is, never tried it though. Do you think this might solve the problem?

dlloyd: Looks like as you have it, while in Pure Data, you'll need to bit-shift and manipulate the data to re-build the 13-bit readings.

I don't need to bit-shift, I receive two bytes

transfer_data[index++] = smoothed & 0x007f;
transfer_data[index++] = smoothed >> 7;

where I multiply the second by 128, and add it to the first. This way the 13-bit resolution is being re-assembled. I'm transferring data this way, cause I need a unique byte at the beginning of the array (in this case it's 0xc0) so that Pure Data knows the this is the beginning of the data. If I use any other value with an 8-bit resolution, it might be the same with 0xc0, thus confusing Pure Data, and not collecting the data properly.

dlloyd: Also, are you sure the data is clean? Why use "unsigned int"? (unsigned ints are 2-bytes for ATMEGA based boards, 4-bytes on the Due)

Since accumulate is unsigned long and you have "smoothed = accumulate / SMOOTH;" I would make all data variables unsigned long.

What do you mean by clean? Not really sure why I use "unsigned int", copied the whole technique from another project (not mine), and it works fine till now. Anyway, all data is being converted to bytes eventually, plus the analog pins don't give values with resolution greater than that of int. Even "unsigned" shouldn't be necessary, I guess...

The result of dividing by "SMOOTH", should always be from 0 till 8191, cause the values that accumulate are in that range, and "SMOOTH" is the number of values which are accumulated.

transfer_data[index++] = smoothed & 0x0000007F;         //7-bit
transfer_data[index++] = (smoothed >> 7) & 0x0000003F;  //6-bit
// or...
transfer_data[index++] = smoothed & 0x000000FF;         //8-bit
transfer_data[index++] = (smoothed >> 8) & 0x0000001F;  //5-bit

Tried this, but it still doesn't work, I get the same behavior as described in the initial post of this thread. The second version you propose wouldn't do cause of the 8-bit number, as I explained above, for transferring data to Pure Data. I'm pretty sure that the Teensy does some mess when I use the multiplexers (I've tested these multiplexers in the past and they work, might have to test them again), cause the same number of potentiometers with the same amount of accumulated reading for smoothing, work without the multiplexers. I'll keep on narrowing this down till I find it.

Thanks for your suggestions.

Some diagnostics, changes and testing. Maybe something here could be useful.
I would try printing your data to the serial monitor to see where the problems exist.

const byte transfer_start = 0xC0;
const byte rows = 3;
const byte cols = 3;
unsigned short readData = 100;
unsigned short pots[rows][cols];

const byte num_of_data = rows * cols;
unsigned short transfer_data[num_of_data];

// analog readings smoothing function
unsigned short smooth(int row_index)
{
  //  unsigned long smoothed = 0;
  unsigned long accumulate = 0;
  // accumulate analog readings
  for (int i = 0; i < cols; i++)
  {
    accumulate += pots[row_index][i];
  }
  // divide the row's sum by the number of values in the row
  return accumulate / cols;
}

void setup()
{
  Serial.begin(115200);
  delay(1000);
  // initialize pots
  for (int i = 0; i < rows; i++) {
    for (int j = 0; j < cols; j++) {
      readData += 200;
      pots[i][j] =  readData;
    }
  }
  // initialize transfer_data
  int k = 0;
  for (int i = 0; i < cols; i++) {
    for (int j = 0; j < rows; j++) {
      transfer_data[k] =  pots[i][j];
      k += 1;
    }
  }
  Serial.print("num_of_data: ");  Serial.println(num_of_data);
  Serial.println("\npots:\tCOL1\tCOL2\tCOL3");
  Serial.print("ROW1\t");
  Serial.print(pots[0][0]); Serial.print("\t");
  Serial.print(pots[0][1]); Serial.print("\t");
  Serial.println(pots[0][2]);
  Serial.print("ROW2\t");
  Serial.print(pots[1][0]); Serial.print("\t");
  Serial.print(pots[1][1]); Serial.print("\t");
  Serial.println(pots[1][2]);
  Serial.print("ROW3\t");
  Serial.print(pots[2][0]); Serial.print("\t");
  Serial.print(pots[2][1]); Serial.print("\t");
  Serial.println(pots[2][2]);
  Serial.println();
  Serial.print("transfer_data: ");
  for (int i = 0; i < num_of_data; i++) {
    Serial.print(transfer_data[i]);
    Serial.print(" ");
  }
  Serial.println();
}

void loop() {

  Serial.print("\ntransfer_start: "); Serial.println(transfer_start, HEX);
  Serial.println();

  for (int i = 0; i < rows; i++) {

    unsigned short smoothed = smooth(i);
    transfer_data[i] = smoothed;
    Serial.print("average of row "); Serial.print(i + 1); Serial.print(": ");
    Serial.println(transfer_data[i], DEC);
    Serial.print("13-bits:  "); Serial.println(transfer_data[i], BIN);
    transfer_data[i] = smoothed & 0x007f;
    Serial.print("7-bits:   "); Serial.println(transfer_data[i], BIN);
    transfer_data[i] = smoothed >> 7 & 0x003f;
    Serial.print("6-bits:   "); Serial.println(transfer_data[i], BIN);
    Serial.println();
  }
  delay(1000000); // done
}

@dlloyd, tried your code, had to add a delay(1000); at the end instead of while(1); cause it wasn't printing anything. Couldn't get the prints from the setup, but got the prints from the loop. Here they are

transfer_start: C0

average of row 1: 500
13-bits:  111110100
7-bits:   1110100
6-bits:   11

average of row 2: 1100
13-bits:  10001001100
7-bits:   1001100
6-bits:   1000

average of row 3: 1700
13-bits:  11010100100
7-bits:   100100
6-bits:   1101

These look ok, don't they? Haven't had time to debug the circuit, I'm still positive that it has to do with the multiplexer chips, dunno.

Looks like on your Arduino, it isn't printing on the first iteration ... I edited reply #14's code so that the function is before setup, Serial.begin(115200); is first in setup with 1 sec delay. Everything should print on the first pass through the loop (tested on Arduino Due/ IDE 1.5.8 / Windows7).

As I began checking your original code I was getting some high numbers near the end of the transfer, so I just ended up making a lot of changes just to get a test script working.

These look ok, don't they? Haven't had time to debug the circuit, I'm still positive that it has to do with the multiplexer chips, dunno.

Looks OK, but it's not your original code. If you could test original code to see if it prints the values to the serial monitor OK, then it must be something in hardware.

dlloyd: If you could test original code to see if it prints the values to the serial monitor OK, then it must be something in hardware.

I did that. Instead of doing

unsigned int smoothed = smooth(row_index);
transfer_data[index++] = smoothed & 0x007f;
transfer_data[index++] = (smoothed >> 7) & 0x003f;

And then transferring the whole array, I did

unsigned int smoothed = smooth(row_index);
if(row_index == 0){ // change this to choose which potentiometer to print to the monitor
  Serial.println(smoothed);
}

and I had exactly the same behavior, resolution reduction in all potentiometers except from the first one. I'll try to see what's going on with the hardware.

Uncompiled, untested.

const byte SMOOTH = 100;
const byte rows = 3;
const byte cols = SMOOTH;
const byte num_of_data = (rows * 2) + 1;


unsigned int pots[rows][cols];
unsigned long sums [rows];

void setup()
{
  analogReadResolution(13);
  Serial.begin(115200);
}

void loop()
{
  byte index = 1;
  static byte tail;
  byte transfer_data[num_of_data];
  transfer_data[0] = 0xc0;
  
  for(byte i = 0; i < rows; i++)
  {
    sums [i] -= pots[i][tail];
    sums [i] += pots[i][tail] = analogRead(i);
    
    unsigned int smoothed = sums [i] / SMOOTH;
    transfer_data[index++] = smoothed & 0x007f;
    transfer_data[index++] = smoothed >> 7;
  }
  tail++;
  if (tail >= SMOOTH)
    tail = 0;
  
  Serial.write(transfer_data, num_of_data);
}
#define TEST
//TEST UNO/MEGA

#define SMOOTH 8
#define rows 3
unsigned int pots[rows]={0};

void setup() {
#ifndef TEST
  analogReadResolution(13);
#endif
  Serial.begin(9600); }

void loop() {
  Serial.print((char)0xc0);
  for(int i=0; i<rows; i++) {
    pots[i] = ((long long int)pots[i]*(SMOOTH-1) + 
              analogRead(i))       / SMOOTH;  
 #ifndef TEST
      Serial.print((char)(pots[i] & 0x7f));
      Serial.print((char)( (pots[i]>>7)));      
 #else
      pots[2]=analogRead(0);
      pots[1]=analogRead(0);
      Serial.print(','); Serial.print(pots[i]);
 #endif
  }
#ifdef TEST
  Serial.println("");
  delay(300);
#endif
}