D B

Important changes to forums and questions

All forums and questions are now archived. To start a new conversation or read the latest updates go to forums.mbed.com.

5 years, 10 months ago.

Where to declare variables and when to use static variables?

I'm a rookie in C++ and I'm trying to write a program that stores data in sequential files log_0000.csv, log_0001.csv, log_0002.csv etc. Pushbuttons are used to open and close the files. The size of the file must be reported accurately immediately after each file is closed. The files are stored in the local file system but eventually I will use a SD card.

I'm using Andy A's idea (lines 37-44) of storing in memory the number of the next file but one. This stops the requirement of checking every existing file name each time the main 'while' loop restarts.

The code I've tried is below and seems to work ok, but I have a few questions about the code structure:

1. I've declared FILE *fp inside the main 'while' loop (line 36). Is there a better place to declare it? Should it be declared outside of main()?

2. Andy A uses a static unsigned int to declare the fileNumber. Should I do this (line 31)? The program seems to work with just unsigned int

3. Is the syntax in line 61 acceptable for determining filesize?

4. If the stop interrupt button is pressed while the data is being written to a file, I'm not certain that all the data will appear in the file. Is there a possibility to inhibit interrupts when writing data and is it necessary?

// This program checks for the next available filename in a sequence.
// and writes data to the filename when a start button is pressed.
// The file is closed when the stop button is pressed. File size is reported.
// A new file is written to when the start button is pressed again.

#include "mbed.h"

//Objects
LocalFileSystem local("local");   // define local file system
InterruptIn     startBtn(p15);                     
InterruptIn     stopBtn(p16);
DigitalOut      myled1(LED1); // LPC1768 on board LED1

//Declare variables
volatile bool recording = false;                   

// Function to start writing to file
void startFileContent(void){
    recording = true;
    myled1 = 1;
//    do I need 'return'?;
}

// Function to stop writing to file
void stopFileContent(void){
    recording = false;
}

int main() {
    char fileName[32];
    unsigned int fileNumber = 0;    // SHOULD THIS BE static?
    startBtn.rise(&startFileContent);          //attach stop button
    stopBtn.rise(&stopFileContent);            //attach stop button

    while (1){
        FILE *fp = NULL; //should this be in while loop?
        do {
            if (fp != NULL){
                fclose(fp);
            }
            sprintf(fileName,"/local/log_%04u.csv",fileNumber);
            fileNumber++; // this will be starting fileNumber next time while loop starts
            fp = fopen(fileName,"r");
        } while (fp != NULL);

        //"Stand by" mode
        while (recording == false) { 
            myled1= !myled1;       // myled1 flashes until startBtn is pressed
            wait(0.3); // 300 ms
        }

        fp = fopen(fileName,"w");
        printf("opening file name %s\n",fileName);
        wait (0.5); //not sure if this is needed. Might be if storing on SD card.
    
        while (recording == true) {
            fprintf(fp, "This is file # %d\n", fileNumber-1);
        wait (1.0);
        }

        unsigned int fileSize =  ftell(fp);
        fclose(fp); // close the current file
        myled1 = 0;
        printf("fileSize (bytes) =%.3u\r\n",fileSize);
    }
}

1 Answer

Andy A
5 years, 10 months ago.

1) I'm sure the text book answer is to minimize variable scope and so put the FILE *fp declaration inside the loop as you have done. Personally I'd put it just outside the while loop, that way I find it more obvious that I must manually clean up (in this case close the file) each time around the loop or risk memory leaks. Only make it global if you want to be able to write to the file from other functions. In that situation for safety the other functions should check that it's not NULL before using it.

2) Static is used to remember a variable between calls to a function, the main function is only ever called once and so static becomes redundant.

You don't link to where you are lifting the code on lines 37-44 from but I'm guessing it was a function, something like:

    FILE *openFile() {
        static unsigned int fileNumber = 0;
        char fileName[32];
        FILE *fp = NULL; 
        do {
            if (fp != NULL){
                fclose(fp);
            }
            sprintf(fileName,"/local/log_%04u.csv",fileNumber);
            fileNumber++;   // this will be starting fileNumber next time while loop starts
            fp = fopen(fileName,"r");
        } while (fp != NULL);
        return fopen(fileName,"w");
    }

In that situation fileNumber is static so that the value is remembered between calls to the function.

3) ftell(fp) should work fine (it's intended for binary files but should still work for this). Personally I'd do things a little differently, fprintf(fp,...) returns the number of bytes written so why not add up all the bytes as you write them, that way you have a running total of file size at any point in time. 

        unsigned int fileSize = 0;
        while (recording == true) {
            fileSize  += fprintf(fp, "This is file # %d\n", fileNumber-1);
            wait (1.0);
        }
        fclose(fp);
        myled1 = 0;
        printf("fileSize (bytes) =%.3u\r\n",fileSize);

4) Yes the interrupt for the button press could come during a file write. But the interrupt doesn't close the file and stop writing, the while(recording == true) controls when to close the file and that can't happen in the middle of a fprintf so you don't need to worry about this.

Accepted Answer

That's a very comprehensive answer. Thank you Andy A!

You're right about the openFile() function which I found here: https://os.mbed.com/questions/69263/Saving-analog-data-in-SD-card/ Simon Ford also wrote some code https://os.mbed.com/users/simon/code/IncrementingFilename/ aimed at beginners like me. Your code, however, makes my program faster.

1. I will cut FILE *fp = NULL; from line 36 and paste it in line 34. 2. Great explanation. I won't add static to line 31. 3. This relates a bit to my question 4 as I wasn't sure if I would 'lose' bytes during interrupts and therefore have to wait to the end to find out how many had made it to the file. Your answer to question 4 means that I can add up as I go along, as you suggest. 4. Understood.

Thanks again for your time. Much appreciated.

posted by D B 27 Jun 2018