Nested for loop question

Self-educated 60+ hobby-coder here, so don't bite my head off.
Trying to design a lead/acid battery cell voltmeter with relays/flying caps. Just for fun and learning.
Eight DPDT relays and a TPIC6C596 shift register on a small board. Three boards for 24 cells (48volt battery) for now.
Boards are designed and parts are ordered, so time to think software.
This is what I have so far, but I'm not sure about the nested for loop on line 53.
'rawValue' exists inside the main for loop, but can it also exist inside the nested for loop.
Or should I do it as in the second sketch ('total' is an unsigned int).
Maybe there are also other mistakes.
Leo..

// TPIC6C596 lead/acid battery cell monitor
// 6k8 resistor between Aref and 3.3volt pins to make a ~2.725volt Aref

// user input
float Aref = 2.731; // Aref used | ***calibrate voltage here***
const byte cells = 24; // number of battery cells | max 32
const byte numReadings = 45; // for averaging | max 64
const byte inputPin = A0;
const byte latchPin = 8;
const byte dataPin = 11;
const byte clockPin = 12;

unsigned int rawValue[cells]; // cell arrays | each can hold 64 A/D readings
float voltage[cells]; // voltage arrays
long data = 0; // 4 bytes | 32 relays max
int preRead; // this A/D value is not used

void setup() {
  pinMode(latchPin, OUTPUT);
  pinMode(dataPin, OUTPUT);
  pinMode(clockPin, OUTPUT);
  // start with all relays off
  digitalWrite(latchPin, LOW);
  digitalWrite(dataPin, 0);
  for (int i = 0; i < 32; i++) { // clock in LOWs
    digitalWrite(clockPin, HIGH);
    digitalWrite(clockPin, LOW);
  }
  digitalWrite(latchPin, HIGH); // transfer to outputs
  digitalWrite(latchPin, LOW);
  // three control lines are now low
  analogReference(EXTERNAL); // an external reference voltage is used
  Serial.begin(115200); // ***set serial monitor to this value***
  Serial.println(F("Lead/Acid Battery Cell Voltmeter"));
  Serial.println(F("--------------------------------"));
  delay(5000); // wait for cell sample caps to charge.
}

void loop() {
  // read the voltage on the flying caps
  for (int i = 0; i < cells; i++) { // step through the rawValue arrays
    data = 1 << i; // shifts a single HIGH bit across the bytes of a 4-byte long | one relay on at the time
    digitalWrite(latchPin, LOW); // so the relays don't chatter while sending data
    shiftOut(dataPin, clockPin, MSBFIRST, (data >> 24)); // send
    shiftOut(dataPin, clockPin, MSBFIRST, (data >> 16)); // the
    shiftOut(dataPin, clockPin, MSBFIRST, (data >> 8));  // four
    shiftOut(dataPin, clockPin, MSBFIRST, data);         // bytes
    digitalWrite(latchPin, HIGH); // activate relay
    delay(10); // wait for contact bounce
    preRead = analogRead(inputPin); // one unused reading to clear any ghost charges
    //Serial.println(preRead); // test bouncing
    for (int x = 0; x < numReadings; x++) { // multiple analogue readings for averaging
      rawValue[i] = rawValue[i] + analogRead(inputPin); // add each value
    }
    // all relays off
    digitalWrite(latchPin, LOW);
    digitalWrite(dataPin, 0);
    for (int j = 0; j < 32; j++) { // clock in 32 LOWs
      digitalWrite(clockPin, HIGH);
      digitalWrite(clockPin, LOW);
    }
    digitalWrite(latchPin, HIGH); // transfer to outputs
    digitalWrite(latchPin, LOW);
    delay(10); // wait for relay to turn off
    // calculate voltage and clear rawValue arrays
    voltage[i] = rawValue[i] * (Aref / 1023) / numReadings; // calculate voltage and store in voltage array
    if ((rawValue[i] / numReadings) == 1023) voltage[i] = 9.999; // crude overflow indicator
    rawValue[i] = 0; // clear raw array
  }

  // print the values
  Serial.print(F("Battery 1 = "));
  Serial.print(voltage[0] + voltage[1] + voltage[2] + voltage[3] + voltage[4] + voltage[5]);
  Serial.print(F("volt   Cells:  "));
  for (int i = 0; i < 6; i++) {
    Serial.print(voltage[i], 3); // three decimal places
    Serial.print(F("  "));
  }
  Serial.println(F(""));
  Serial.print(F("Battery 2 = "));
  Serial.print(voltage[6] + voltage[7] + voltage[8] + voltage[9] + voltage[10] + voltage[11]);
  Serial.print(F("volt   Cells:  "));
  for (int i = 6; i < 12; i++) {
    Serial.print(voltage[i], 3); // three decimal places
    Serial.print(F("  "));
  }
  Serial.println(F(""));
  Serial.print(F("Battery 3 = "));
  Serial.print(voltage[12] + voltage[13] + voltage[14] + voltage[15] + voltage[16] + voltage[17]);
  Serial.print(F("volt   Cells:  "));
  for (int i = 12; i < 18; i++) {
    Serial.print(voltage[i], 3); // three decimal places
    Serial.print(F("  "));
  }
  Serial.println(F(""));
  Serial.print(F("Battery 4 = "));
  Serial.print(voltage[18] + voltage[19] + voltage[20] + voltage[21] + voltage[22] + voltage[23]);
  Serial.print(F("volt   Cells:  "));
  for (int i = 18; i < 24; i++) {
    Serial.print(voltage[i], 3); // three decimal places
    Serial.print(F("  "));
  }
  Serial.println(F(""));
  Serial.println(F("---------------------"));

  delay(5000); // time between scanning all cells
}
    for (int x = 0; x < numReadings; x++) { // multiple analogue readings for averaging
      total = total + analogRead(inputPin); // add each value
    }
    rawValue[i] = total; // transfer to rawValue array
    total = 0; // reset

Yes it also exists in the nested for loop.

rawValue is a "global" variable.

I was wondering if the array pointer (i) of that variable would give problems inside the nested loop.
Leo..

No it wont give problems.

Perhaps the post on Scope at the top of the Forum would help you.

Thanks for the replies.
Just wasn't sure the value of that array could be updated inside the nested for loop, while the pointer was updated every time in the normal for loop.
Will read that scope thread.
Leo..

Edit: Updated the sketch.
Voltage measurement is now inside the main for loop.

Flying cap? :confused:

outsider:
Flying cap? :confused:

Yep.
Capacitors are connected to battery cells with DPDT relays.
Cap is connected to the two common contacts.
Cell is connected to the two NC contacts.
During scanning, the cap is disconnected from the cell and connected to ground and analogue input of the Arduino, measured, and returned.
No charge is lost. No galvanic connection between battery and Arduino.
Battery (stack of cells) can be up to ~60volt positive and/or 60volt negative.
The limit is the DC isolation of the small relays I'm using.
I will try to post a picture of the finished boards when the parts have arrived.
Leo..

Updated code.
Couldn't get bitshift to work over 2 bytes with a 'long' (data = 1 << i;)

// TPIC6C596 lead/acid battery cell monitor
// 6k8 resistor between Aref and 3.3volt pins to make ~2.725volt Aref

float Aref = 2.731; // actual Aref voltage | ***calibrate here***
const byte cells = 24; // number of battery cells | max 32
const byte numReadings = 45; // for averaging | max 64
const byte cellPin = A0; // analogue input for cell voltage
const byte latchPin = 8; // latch or RCK
const byte dataPin = 11; // serial in
const byte clockPin = 12; // clock or SRCK
unsigned int rawValue[cells]; // raw A/D arrays | each can hold 64 A/D readings
float voltage[cells]; // voltage arrays
long data = 0; // 4 bytes | 32 relays max
int preRead; // A/D value is not used

void setup() {
  pinMode(latchPin, OUTPUT);
  pinMode(dataPin, OUTPUT);
  pinMode(clockPin, OUTPUT);
  // start with all relays off
  digitalWrite(latchPin, LOW);
  digitalWrite(dataPin, LOW);
  digitalWrite(clockPin, LOW);
  for (int i = 0; i < 32; i++) { // clock in 32 LOWs
    digitalWrite(clockPin, HIGH);
    digitalWrite(clockPin, LOW);
  }
  digitalWrite(latchPin, HIGH); // transfer to outputs
  digitalWrite(latchPin, LOW);
  // three control lines are now low
  analogReference(EXTERNAL); // an external reference voltage is used
  Serial.begin(115200); // ***set serial monitor to this value***
  Serial.println(F("Lead/Acid Battery Cell Voltmeter"));
  Serial.println(F("--------------------------------"));
  delay(3000); // wait for cell sample caps to charge.
}

void loop() {
  // read the voltage on the flying caps
  data = 1; // set LSB HIGH
  for (int i = 0; i < cells; i++) { // step through the rawValue arrays
    digitalWrite(latchPin, LOW); // so the relays don't chatter while sending data
    shiftOut(dataPin, clockPin, MSBFIRST, (data >> 24)); // send
    shiftOut(dataPin, clockPin, MSBFIRST, (data >> 16)); // the
    shiftOut(dataPin, clockPin, MSBFIRST, (data >> 8));  // four
    shiftOut(dataPin, clockPin, MSBFIRST, data);         // bytes
    digitalWrite(latchPin, HIGH); // activate relay
    delay(20); // wait for contact bounce
    preRead = analogRead(cellPin); // one unused reading to clear any A/D ghost charge
    //Serial.println(preRead); // to test contact bounce time
    for (int x = 0; x < numReadings; x++) { // multiple analogue readings for averaging
      rawValue[i] = rawValue[i] + analogRead(cellPin); // add each value
    }
    // all relays off
    digitalWrite(latchPin, LOW);
    digitalWrite(dataPin, LOW);
    for (int j = 0; j < 32; j++) {
      digitalWrite(clockPin, HIGH);
      digitalWrite(clockPin, LOW);
    }
    digitalWrite(latchPin, HIGH); // transfer to outputs
    delay(10); // wait for relay to turn off
    // calculate voltage and clear rawValue arrays
    voltage[i] = rawValue[i] * (Aref / 1024) / numReadings; // calculate voltage and store in voltage array
    if ((rawValue[i] / numReadings) == 1023) voltage[i] = 9.999; // crude overflow indicator
    rawValue[i] = 0; // clear array
    data = data * 2; // shift the single HIGH bit across the four bytes of the long | one relay on at the time
  }

  // print the values
  Serial.print(F("Battery 1 = "));
  Serial.print(voltage[0] + voltage[1] + voltage[2] + voltage[3] + voltage[4] + voltage[5]);
  Serial.print(F("volt   6 Cells:  "));
  for (int i = 0; i < 6; i++) {
    Serial.print(voltage[i], 3); // three decimal places
    Serial.print(F("  "));
  }
  Serial.println(F(""));
  Serial.print(F("Battery 2 = "));
  Serial.print(voltage[6] + voltage[7] + voltage[8] + voltage[9] + voltage[10] + voltage[11]);
  Serial.print(F("volt   6 Cells:  "));
  for (int i = 6; i < 12; i++) {
    Serial.print(voltage[i], 3); // three decimal places
    Serial.print(F("  "));
  }
  Serial.println(F(""));
  Serial.print(F("Battery 3 = "));
  Serial.print(voltage[12] + voltage[13] + voltage[14] + voltage[15] + voltage[16] + voltage[17]);
  Serial.print(F("volt   6 Cells:  "));
  for (int i = 12; i < 18; i++) {
    Serial.print(voltage[i], 3); // three decimal places
    Serial.print(F("  "));
  }
  Serial.println(F(""));
  Serial.print(F("Battery 4 = "));
  Serial.print(voltage[18] + voltage[19] + voltage[20] + voltage[21] + voltage[22] + voltage[23]);
  Serial.print(F("volt   6 Cells:  "));
  for (int i = 18; i < 24; i++) {
    Serial.print(voltage[i], 3); // three decimal places
    Serial.print(F("  "));
  }
  Serial.println(F(""));
  Serial.println(F("---------------------"));

  delay(10000); // time between scanning | to be changed when bluetooth display is added
}

And a picture, as promised.
Leo..

Couldn't get bitshift to work over 2 bytes with a 'long' (data = 1 << i;)

Did you mean data = 1L << i;?

Thanks AWOL.
Code is working as it should with

data = 1;

on line 40, and

data = data * 2;

on line 67, but I'll try your suggestion.
Leo..

In that case, the compiler is probably generating data <<= 1;

Yes, that worked.

'data' is declared as a long at the start, and that did not work.
But it works if you declare it locally.

Working

long data = 0; // 4 bytes | 32 relays max

void setup() {
  Serial.begin(115200); // ***set serial monitor to this value***
    for (int i = 0; i < 24; i++) {
    data = 1L << i; // shifts a single HIGH bit across the bytes of a 4-byte long | one relay on at the time
    Serial.println(data, BIN);
  }
}

void loop() {
}

Not working

long data = 0; // 4 bytes | 32 relays max

void setup() {
  Serial.begin(115200); // ***set serial monitor to this value***
    for (int i = 0; i < 24; i++) {
    data = 1 << i; // shifts a single HIGH bit across the bytes of a 4-byte long | one relay on at the time
    Serial.println(data, BIN);
  }
}

void loop() {
}

Your not working example is missing the L

Yes, know that, but it's not clear to me why you have to force it to a 'long' locally when it is already declared as a 'long' at the start.
Doesn't really matter. The program worked with my construction, and it works with the added L.
Thanks for your help anyway. +1
Leo..

I haven't forced "data" to be a long, I've forced 1 to be a long.
It's the 1 you're shifting.

Ahhh, I think I see a dim light now.
Leo..

    for (int x = 0; x < numReadings; x++) { // multiple analogue readings for averaging
      rawValue[i] = rawValue[i] + analogRead(inputPin); // add each value
    }

You will want to set rawValue[nobbc][i][/nobbc] to zero prior to entering that loop. Oh - I see, you clear it at the end. So the first pass through the loop will be rubbish, but that's not a drama.

Not 100% sure that reading multiple times with no delay buys you anything, but meh.

I don't see any need for a rawValue array if all you are doing is doing a calcutation on it and putting the result into voltage[]. Just have a single rawValue variable, and zero it out.

If you want to get rid of noise, one option is to use a software low-pass filter rather than your multiple reads:

rawValue[i] = rawValue[i] * .75 + analogRead(inputPin) * .25;

In that instance, it would be reasonable for rawValue to be a floating-point value, because it's storing an average rather than a precise value.

PaulMurrayCbr:
You will want to set rawValue[nobbc][i][/nobbc] to zero prior to entering that loop. Oh - I see, you clear it at the end. So the first pass through the loop will be rubbish, but that's not a drama.

It's actually not. The first pass is exactly the same as the second pass.
But good point. I have changed it in the next version.

PaulMurrayCbr:
Not 100% sure that reading multiple times with no delay buys you anything, but meh.

I have tried various ways, and reading many times (32+) seems to get the most stable results.
It drowns out the occasional glitch. Results vary less than +/- 1mV on a 2.75volt scale.

PaulMurrayCbr:
If you want to get rid of noise, one option is to use a software low-pass filter rather than your multiple reads:

rawValue = rawValue * .75 + analogRead(inputPin) * .25;
In that instance, it would be reasonable for rawValue to be a floating-point value, because it's storing an average rather than a precise value.
[/quote]
I already tried various software smoothers, but I think I can't use that sort of averaging here.
Because I'm using (noisy) mechanical relays, measuring frequency will later depend on battery current.
If nothing is happening (charge/discharge), that time might be 15 minutes apart.
Only when current varies, and/or data is needed (bluetooth will be added), sample frequency will be <=15sec.
Sample time of 24 relays is ~1sec, including 24*45 A/D readings and the 20+10msec debounce/release time programmed for each relay.
Leo..