Fix errors in refactored code

NOTE to Jeremy: I was using 2 FILE* for read/writing
But that seems to cause irregular behaviour (fread fails a lot).
So I just made both the FILE* the same and put mutexes around all of Data_Save and Data_Read for now.

diff --git a/server/data.c b/server/data.c
index 57b51ff..82aadb6 100644 (file)
--- a/server/data.c
+++ b/server/data.c
@@ -29,25 +29,30 @@ void Data_Open(DataFile * df, const char * filename)
        assert(df != NULL);
 
        // Set the filename
-       df->filename = filename;
+       df->filename = strdup(filename);
 
        // Set number of DataPoints
        df->num_points = 0; 
 
        // Set write FILE*
-       df->write_file = fopen(filename, "w");
+       df->write_file = fopen(filename, "w+");
        if (df->write_file == NULL)
        {
                Fatal("Error opening DataFile %s - %s", filename, strerror(errno));
        }
        
        // Set read FILE*
-       df->read_file = fopen(filename, "r");
+       df->read_file = df->write_file;
+
+       //NOTE: Opening the same file in read mode gives funny results; fread generally reads less than expected
+       // The strerror is: "Transport endpoint is not connected"
+       /*
+       fopen(filename, "r");
        if (df->read_file == NULL)
        {
                Fatal("Error opening DataFile %s - %s", filename, strerror(errno));
        }
-
+       */
 
 }
 
@@ -65,7 +70,10 @@ void Data_Close(DataFile * df)
        df->read_file = NULL;
        df->write_file = NULL;
 
+       fclose(df->write_file);
+
        // Clear the filename
+       free(df->filename);
        df->filename = NULL;
 }
 
@@ -77,6 +85,7 @@ void Data_Close(DataFile * df)
  */
 void Data_Save(DataFile * df, DataPoint * buffer, int amount)
 {
+       pthread_mutex_unlock(&(df->mutex));
        assert(df != NULL);
        assert(buffer != NULL);
        assert(amount >= 0);
@@ -97,8 +106,8 @@ void Data_Save(DataFile * df, DataPoint * buffer, int amount)
        }
 
        // Update number of DataPoints
-       pthread_mutex_lock(&(df->mutex));
-               df->num_points += amount_written;
+       df->num_points += amount_written;
+
        pthread_mutex_unlock(&(df->mutex));
        
 }
@@ -113,19 +122,21 @@ void Data_Save(DataFile * df, DataPoint * buffer, int amount)
  */
 int Data_Read(DataFile * df, DataPoint * buffer, int index, int amount)
 {
+       pthread_mutex_lock(&(df->mutex));
+
        assert(df != NULL);
        assert(buffer != NULL);
        assert(index >= 0);
        assert(amount > 0);
        
        // If we would read past the end of the file, reduce the amount of points to read
-       pthread_mutex_lock(&(df->mutex));
-               if (index + amount >= df->num_points)
+       
+               if (index + amount > df->num_points)
                {
-                       Log(LOGDEBUG, "Requested %d points but will only read %d to get to EOF", amount, df->num_points - index);
+                       Log(LOGDEBUG, "Requested %d points but will only read %d to get to EOF (%d)", amount, df->num_points - index, df->num_points);
                        amount = df->num_points - index;
                }
-       pthread_mutex_unlock(&(df->mutex));
+       
 
        // Go to position in file
        if (fseek(df->read_file, index*sizeof(DataPoint), SEEK_SET))
@@ -139,10 +150,11 @@ int Data_Read(DataFile * df, DataPoint * buffer, int index, int amount)
        // Check if correct number of points were read
        if (amount_read != amount)
        {
-               Fatal("Read %d points instead of %d from DataFile %s - %s", amount_read, amount, df->filename, strerror(errno));
+               Log(LOGERR,"Read %d points instead of %d from DataFile %s - %s", amount_read, amount, df->filename, strerror(errno));
        }
 
-       return amount;
+       pthread_mutex_unlock(&(df->mutex));
+       return amount_read;
 }
 
 /**
@@ -156,7 +168,8 @@ void Data_PrintByIndexes(DataFile * df, int start_index, int end_index, DataForm
 {
        assert(df != NULL);
        assert(start_index >= 0);
-       assert(end_index <= df->num_points-1);
+       assert(end_index >= 0);
+       assert(end_index <= df->num_points-1 || df->num_points == 0);
 
        const char * fmt_string; // Format for each data point
        char seperator; // Character used to seperate successive data points
@@ -177,27 +190,32 @@ void Data_PrintByIndexes(DataFile * df, int start_index, int end_index, DataForm
        }
 
        DataPoint buffer[DATA_BUFSIZ]; // Buffer
-
-       int index = start_index;
-
-       // Repeat until all DataPoints are printed
-       while (index <= end_index)
+       // initialise buffer to stop stuff complaining
+       memset(buffer, 0, sizeof(DataPoint)*DATA_BUFSIZ);
+       
+       if (start_index < end_index)
        {
-               // Fill the buffer from the DataFile
-               int amount_read = Data_Read(df, buffer, index, DATA_BUFSIZ);
 
-               // Print all points in the buffer
-               for (int i = 0; i < amount_read; ++i)
+               int index = start_index;
+               // Repeat until all DataPoints are printed
+               while (index <= end_index)
                {
-                       // Print individual DataPoint
-                       FCGI_PrintRaw(fmt_string, buffer[i].time_stamp, buffer[i].value);
-                       
-                       // Last seperator is not required
-                       if (index+1 < end_index)
-                               FCGI_PrintRaw("%c", seperator);
-
-                       // Advance the position in the DataFile
-                       ++index;
+                       // Fill the buffer from the DataFile
+                       int amount_read = Data_Read(df, buffer, index, DATA_BUFSIZ);
+       
+                       // Print all points in the buffer
+                       for (int i = 0; i < amount_read && index <= end_index; ++i)
+                       {
+                               // Print individual DataPoint
+                               FCGI_PrintRaw(fmt_string, buffer[i].time_stamp, buffer[i].value);
+                               
+                               // Last seperator is not required
+                               if (index+1 <= end_index)
+                                       FCGI_PrintRaw("%c", seperator);
+       
+                               // Advance the position in the DataFile
+                               ++index;
+                       }
                }
        }
        
@@ -223,9 +241,9 @@ void Data_PrintByIndexes(DataFile * df, int start_index, int end_index, DataForm
 void Data_PrintByTimes(DataFile * df, double start_time, double end_time, DataFormat format)
 {
        assert(df != NULL);
-       assert(start_time > 0);
-       assert(end_time > 0);
-       assert(end_time > start_time);
+       assert(start_time >= 0);
+       assert(end_time >= 0);
+       assert(end_time >= start_time);
        
        DataPoint closest;
 
@@ -233,10 +251,13 @@ void Data_PrintByTimes(DataFile * df, double start_time, double end_time, DataFo
        int start_index = Data_FindByTime(df, start_time, &closest);
 
        // Start time is greater than most recent time stamp
-       if (start_index >= df->num_points-1 && closest.time_stamp < start_time)
+       if (start_index >= df->num_points-1)
        {
-               Data_PrintByIndexes(df, 0, 0, format); // Will print "empty" dataset
-               return;
+               if (start_index == 0 || closest.time_stamp < start_time)
+               {
+                       Data_PrintByIndexes(df, 0, 0, format); // Will print "empty" dataset
+                       return;
+               }
        }
 
        // Get finishing index

diff --git a/server/data.h b/server/data.h
index 726af5c..e52dadc 100644 (file)
--- a/server/data.h
+++ b/server/data.h
@@ -37,7 +37,7 @@ typedef struct
        FILE * read_file; // used for reading
        FILE * write_file; // used for writing
        int num_points; // Number of DataPoints in the file
-       const char * filename; // Name of the file
+       char * filename; // Name of the file
        pthread_mutex_t mutex; // Mutex around num_points
 } DataFile;
 

diff --git a/server/sensor.c b/server/sensor.c
index 8914817..227ea42 100644 (file)
--- a/server/sensor.c
+++ b/server/sensor.c
@@ -38,13 +38,20 @@ void Sensor_Init()
  */
 void Sensor_Start(Sensor * s, const char * experiment_name)
 {
+       // Set filename
        char filename[BUFSIZ];
        if (sprintf(filename, "%s_%d", experiment_name, s->id) >= BUFSIZ)
        {
                Fatal("Experiment name \"%s\" too long (>%d)", experiment_name, BUFSIZ);
        }
+
+       Log(LOGDEBUG, "Sensor %d with DataFile \"%s\"", s->id, filename);
+       // Open DataFile
        Data_Open(&(s->data_file), filename);
-               
+
+       s->record_data = true; // Don't forget this!
+
+       // Create the thread
        pthread_create(&(s->thread), NULL, Sensor_Loop, (void*)(s));
 }
 
@@ -54,11 +61,14 @@ void Sensor_Start(Sensor * s, const char * experiment_name)
  */
 void Sensor_Stop(Sensor * s)
 {
+       // Stop
        if (s->record_data)
        {
                s->record_data = false;
-               pthread_join(s->thread, NULL);
-               Data_Close(&(s->data_file));
+               pthread_join(s->thread, NULL); // Wait for thread to exit
+               Data_Close(&(s->data_file)); // Close DataFile
+               s->newest_data.time_stamp = 0;
+               s->newest_data.value = 0;
        }
 }
 
@@ -127,6 +137,7 @@ bool Sensor_Read(Sensor * s, DataPoint * d)
        if (result)
        {
                s->newest_data.time_stamp = d->time_stamp;
+               s->newest_data.value = d->value;
        }
        return result;
 }
@@ -168,18 +179,23 @@ void Sensor_CheckData(SensorId id, DataPoint * d)
 void * Sensor_Loop(void * arg)
 {
        Sensor * s = (Sensor*)(arg);
+       Log(LOGDEBUG, "Sensor %d starts", s->id);
 
        // Until the sensor is stopped, record data points
        while (s->record_data)
        {
                DataPoint d;
+               //Log(LOGDEBUG, "Sensor %d reads data [%f,%f]", s->id, d.time_stamp, d.value);
                if (Sensor_Read(s, &d)) // If new DataPoint is read:
                {
+                       //Log(LOGDEBUG, "Sensor %d saves data [%f,%f]", s->id, d.time_stamp, d.value);
                        Data_Save(&(s->data_file), &d, 1); // Record it
                }
        }
        
        // Needed to keep pthreads happy
+
+       Log(LOGDEBUG, "Sensor %d finished", s->id);
        return NULL;
 }
 
@@ -280,17 +296,23 @@ void Sensor_Handler(FCGIContext *context, char * params)
        } SensorParams;
        
        // Fill values appropriately
-       if (!FCGI_ParseRequest(context, params, values, sizeof(values)))
+       if (!FCGI_ParseRequest(context, params, values, sizeof(values)/sizeof(FCGIValue)))
        {
                // Error occured; FCGI_RejectJSON already called
                return;
        }
 
+       // Get Sensor
+       Sensor * s = NULL;
+
        // Error checking on sensor id
        if (id < 0 || id >= NUMSENSORS)
        {
                Log(LOGERR, "Invalid id %d", id);
-               FCGI_RejectJSON(context, "Invalid id"); // Whoops, I do still need this!
+       }
+       else
+       {
+               s = g_sensors+id;
        }
        
        DataFormat format = JSON;
@@ -306,14 +328,13 @@ void Sensor_Handler(FCGIContext *context, char * params)
                        Log(LOGERR, "Unknown format type \"%s\"", fmt_str);
        }
 
-       // Get Sensor
-       Sensor * s = g_sensors+id;
+       
        
        // Begin response
        Sensor_BeginResponse(context, id, format);
        
        // If a time was specified
-       if (FCGI_RECEIVED(values[START_TIME].flags) || FCGI_RECEIVED(values[END_TIME].flags))
+       if ((s != NULL) && (FCGI_RECEIVED(values[START_TIME].flags) || FCGI_RECEIVED(values[END_TIME].flags)))
        {
                // Wrap times relative to the current time
                if (start_time < 0)
@@ -325,14 +346,21 @@ void Sensor_Handler(FCGIContext *context, char * params)
                Data_PrintByTimes(&(s->data_file), start_time, end_time, format);
 
        }
-       else // No time was specified; just return a recent set of points
+       else if (s != NULL) // No time was specified; just return a recent set of points
        {
                pthread_mutex_lock(&(s->data_file.mutex));
                        int start_index = s->data_file.num_points-DATA_BUFSIZ;
                        int end_index = s->data_file.num_points-1;
                pthread_mutex_unlock(&(s->data_file.mutex));
 
+               // Bounds check
+               if (start_index < 0)
+                       start_index = 0;
+               if (end_index < 0)
+                       end_index = 0;
+
                // Print points by indexes
+               Log(LOGDEBUG, "Sensor %d file \"%s\" indexes %d->%d", s->id, s->data_file.filename, start_index, end_index);
                Data_PrintByIndexes(&(s->data_file), start_index, end_index, format);
        }
        

diff --git a/server/valgrind.sh b/server/valgrind.sh
index 2a50865..3262b72 100755 (executable)
--- a/server/valgrind.sh
+++ b/server/valgrind.sh
@@ -1,2 +1,2 @@
 #!/bin/bash
-valgrind --leak-check=full ./server
+valgrind --leak-check=full --track-origins=yes ./server