Splitting 8 char array into two 4 char array for atoi converstion in watermeter

Using Arduino Programming Questions
#1

Hi helpful community.

Here is my code which doesn’t work. The char[4] array (LastChunkExtracted) has debris on the end which makes atoi return wrong value. Lower down the page is the prototype code where I successfully tested the code.

My hardware is Duinotech UNO r3 and Arduino Compatible Data Logging Shield CAT.NO: XC4536 from Jaycar

char codeHeading[] = "SketchName: atoidebug5";
//was timeExampleSuperBasicSubDataSDx3plusmeterV23a
#include <Wire.h>
#include "RTClib.h"
RTC_DS1307 rtc;
DateTime now;
#include <SPI.h>
#include <SD.h>
File f;

//char codeHeading[] = "SketchName: 2018-08-18WaterMeterV12 ";
//Varibles used for calculations
int TimeStepUnitIndex = 1;
volatile long unsigned int PulseCurrentCount, elapsed, start;
int currentTimeStep, nextTimeStep;
long unsigned int PulsePrevousCount, PulseCurrentIncrement, PulsePreviousWriteValue  ;
boolean debug, displayNow = false, firstloop = 1, blockSdRead = 0;
int FlowRate;
//The pin location of the IO
int PulseInput = 2;

void PulseCountFunction ()
//This is the function that the interupt calls
{ PulseCurrentCount++;
  elapsed = micros() - start;
  start = micros();
}
//This is the setup function where the serial port is initialised,
//and the interrupt is attached

void setup() {
  long unsigned int LastEntryExtracted;
  int lastEntrySize = 8;
  char LastCharExtracted[lastEntrySize + 1];
  char TimeStepUnits[4][12] = {"hour", "minute", "sixSeconds", "second"};

  //  int currentTimeStep, nextTimeStep;
  Serial.begin(57600);
  Serial.println (codeHeading);
  SD.begin(10); //CS on pin 10;
  f = SD.open("data.txt", FILE_READ); //open file for reading
  unsigned long LastEntry = f.size();
  LastEntry = LastEntry - (lastEntrySize + 2);
  int LastEntryAccepted = f.seek(LastEntry);
  while (LastEntryAccepted) {
    while (f.available()) { //if there is data left in f
      int i;
      LastCharExtracted[i] = (f.read());
      i++;
    }
    LastEntryAccepted = 0;
  }
  f.close(); //then close the file
  LastEntryAccepted = sizeof(LastCharExtracted);
  LastCharExtracted[lastEntrySize] = '\0'; // terminate the string
  Serial.println(LastCharExtracted);// and display it
  //  LastEntryExtracted = atoi(LastCharExtracted);
  for (int j = 2; j >= 1; j--) {
    int n = strlen(LastCharExtracted) - (4 * j);
    Serial.print ("n  ");
    Serial.println (n);
    char LastChunkExtracted[5];
    for (int i = 0; i <= 3; i++) {
      int    k = i + n;
      LastChunkExtracted[i] = LastCharExtracted[k];
    }
    LastChunkExtracted[4] = "\0";
    Serial.print ("LastChunkExtracted  ");
    Serial.println (LastChunkExtracted);
    long int prevLastEntryExtracted;
    prevLastEntryExtracted = 10000 * LastEntryExtracted;
    LastEntryExtracted = prevLastEntryExtracted + atoi(LastChunkExtracted);
  }
  Serial.print ("LastEntryExtracted  ");
  Serial.println (LastEntryExtracted);
  if (LastEntryAccepted == lastEntrySize + 1) {
    Serial.print ("Success reading in last entry  ");
  }
  else {
    Serial.print ("Holy crap couldn't read in last entry  ");
  }
  PulseCurrentCount = LastEntryExtracted;
  PulsePreviousWriteValue = PulseCurrentCount;
  rtc.begin(); //initialise RTC

  debug = 0;
  //  pinMode(3, INPUT_PULLUP);
  pinMode(PulseInput, INPUT_PULLUP);
  attachInterrupt(0, PulseCountFunction, FALLING); //Enables interrupts
  sei(); //Enables interrupts
}

void loop() {
  // put your main code here, to run repeatedly:

}

Serial output here:
“SketchName: atoidebug5
00000269
n 0
LastChunkExtracted 0000%
n 4
LastChunkExtracted 0269%
LastEntryExtracted 3202802189”

char codeHeading[] = "SketchName: atoidebug3";
void setup() {
  // put your setup code here, to run once:
  int lastEntrySize = 8;
  long unsigned int LastEntryExtracted = 0;
  //  char  LastCharExtracted[] = "98364925";
  //  char  LastCharExtracted[] = "98000025";
  char  LastCharExtracted[] = "98360000";
  char LastChunkExtracted[4];
  Serial.begin(57600);
  Serial.println (codeHeading);
  Serial.println(LastCharExtracted);// and display it
  for (int j = 2; j >= 1; j--) {
    int n = strlen(LastCharExtracted) - (4 * j);
    Serial.print ("n  ");
    Serial.println (n);
    for (int i = 0; i <= 3; i++) {
      int    k = i + n;
      LastChunkExtracted[i] = LastCharExtracted[k];
    }
    Serial.print ("LastChunkExtracted  ");
    Serial.println (LastChunkExtracted);
    long int prevLastEntryExtracted;
    prevLastEntryExtracted = 10000 * LastEntryExtracted;
    LastEntryExtracted = prevLastEntryExtracted + atoi(LastChunkExtracted);
    Serial.print ("LastEntryExtracted  ");
    Serial.println (LastEntryExtracted);
  }
  Serial.print ("LastEntryExtracted  ");
  Serial.println (LastEntryExtracted);


}

void loop() {
  // put your main code here, to run repeatedly:

}

Serial output here:
"SketchName: atoidebug3
98360000
n 0
LastChunkExtracted 9836
LastEntryExtracted 9836
n 4
LastChunkExtracted 0000
LastEntryExtracted 98360000
LastEntryExtracted 98360000
"

Background info for the curious. I’ve made a water meter logger based on Arduino Compatible Data Logging Shield CAT.NO: XC4536 from Jaycar. I’m using the atoi function to read the last reading on the SD card at power on.

Thanks in advance. Mark

#2

You aren't doing array bounds checking. Whenever accessing or stuffing arrays, you NEED to make sure you aren't inadvertently accessing memory outside the scope of the array.

Case in point:

   while (f.available()) { //if there is data left in f
      int i;
      LastCharExtracted[i] = (f.read());
      i++;
    }

Also, well thought out variable names and whitespace in the right places are your friends :slight_smile:

#3

Hi Power_Broker. Thanks for your response.

I understand the potential for reading outside the scope of the array, however I have padded the data in the file so there can never be less than the 8 characters I need to decode.

I am explicitly writing into every index of the array, so I can't understand how the code is going wrong.

It was originally developed stand alone the cut and pasted directly into my water meter code.

Cheers

#4

bensonm:
I understand the potential for reading outside the scope of the array, however I have padded the data in the file so there can never be less than the 8 characters I need to decode.

It doesn’t cost you any extra to make your code robust enough to handle more situations than the extremely narrow one you originally wrote it for - after all, you know what they say about assumptions…

Also, can you give a high level explanation of the logic of your code? It looks like you take some readings of something and smash them together and try to convert it into a big number? What are you actually trying to do here?

Debugging steps:

1.) Add bounds checking
2.) Print each char read from the file “f” in the while loop, NOT from the array of read chars (this will ensure we know exactly what inputs we are handling)
3.) print what LastChunkExtracted is right before calling atoi()

Oddities:

1.)

LastEntry = LastEntry - (lastEntrySize + 2); //  <--- this simplifies to:   LastEntry = 2;

2.) Where does the magic number “3” come from here? Magic numbers are something you should stay away from as much as reasonable - try putting them in a const macro with a useful name instead.

    for (int i = 0; i <= 3; i++) {
      int    k = i + n;
      LastChunkExtracted[i] = LastCharExtracted[k];
    }

3.) I know I said this before, but odd variable/array names and lack of proper whitespace makes reading the program rather difficult and makes it harder to debug

#5