Skip to content
This repository
Browse code

Improved RAM usage in OpenLog Light.

Added the fixes and improvements learned on regular OpenLog. Light seems
to be working well at 115200bps across 200,000 bytes continuous.
  • Loading branch information...
commit 2dbdede3c90f5e6a16e7eb57fc8c509a05e5e1da 1 parent 810fb10
Nathan Seidle authored January 21, 2013

Showing 1 changed file with 194 additions and 175 deletions. Show diff stats Hide diff stats

  1. 369  OpenLog_v3_Light/OpenLog_v3_Light.ino
369  OpenLog_v3_Light/OpenLog_v3_Light.ino
@@ -123,14 +123,12 @@
123 123
 #include <SerialPort.h> //This is a new/beta library written by Bill Greiman. You rock Bill!
124 124
 #include <EEPROM.h>
125 125
 
126  
-SerialPort<0, 800, 0> NewSerial;
  126
+SerialPort<0, 900, 0> NewSerial;
127 127
 //This is a very important buffer declaration. This sets the <port #, rx size, tx size>. We set
128 128
 //the TX buffer to zero because we will be spending most of our time needing to buffer the incoming (RX) characters.
129  
-//900 works on minimal implementation, doesn't work with the full command prompt
130  
-//800 doesn't work with full command prompt
131  
-//700 works very well at 115200
132  
-//600 works well at 115200
133  
-//500
  129
+//1100 fails on card init and causes FAT table corruption
  130
+//1000 works on light,
  131
+//900 works on light and is able to create config file
134 132
 
135 133
 #include <avr/sleep.h> //Needed for sleep_mode
136 134
 #include <avr/power.h> //Needed for powering down perihperals such as the ADC/TWI and Timers
@@ -146,6 +144,7 @@ SerialPort<0, 800, 0> NewSerial;
146 144
 
147 145
 #define CFG_FILENAME "config.txt" //This is the name of the file that contains the unit settings
148 146
 #define CFG_LENGTH  20 //Length of text found in config file: "115200,103,14,0,1,1\0" = 115200 bps, escape char of ASCII(103), 14 times, new log mode, verbose on, echo on. 
  147
+#define SEQ_FILENAME "SEQLOG00.TXT" //This is the name for the file when you're in sequential modeu
149 148
 
150 149
 //Internal EEPROM locations for the user settings
151 150
 #define LOCATION_BAUD_SETTING		0x01
@@ -174,12 +173,16 @@ SerialPort<0, 800, 0> NewSerial;
174 173
 #define STAT1_PORT  PORTD
175 174
 #define STAT2  5 //On PORTB
176 175
 #define STAT2_PORT  PORTB
177  
-const uint8_t statled1 = 5;  //This is the normal status LED
178  
-const uint8_t statled2 = 13; //This is the SPI LED, indicating SD traffic
  176
+const byte statled1 = 5;  //This is the normal status LED
  177
+const byte statled2 = 13; //This is the SPI LED, indicating SD traffic
179 178
 
180 179
 //Blinking LED error codes
181  
-#define ERROR_SD_INIT	3
182  
-#define ERROR_NEW_BAUD	5
  180
+#define ERROR_SD_INIT	  3
  181
+#define ERROR_NEW_BAUD	  5
  182
+#define ERROR_CARD_INIT   6
  183
+#define ERROR_VOLUME_INIT 7
  184
+#define ERROR_ROOT_INIT   8
  185
+#define ERROR_FILE_OPEN   9
183 186
 
184 187
 #define OFF   0x00
185 188
 #define ON    0x01
@@ -187,28 +190,55 @@ const uint8_t statled2 = 13; //This is the SPI LED, indicating SD traffic
187 190
 Sd2Card card;
188 191
 SdVolume volume;
189 192
 SdFile currentDirectory;
190  
-SdFile file;
191  
-
192  
-uint8_t setting_uart_speed; //This is the baud rate that the system runs at, default is 9600
193  
-uint8_t setting_system_mode; //This is the mode the system runs in, default is MODE_NEWLOG
194  
-uint8_t setting_escape_character; //This is the ASCII character we look for to break logging, default is ctrl+z
195  
-uint8_t setting_max_escape_character; //Number of escape chars before break logging, default is 3
196  
-uint8_t setting_verbose; //This controls the whether we get extended or simple responses.
197  
-uint8_t setting_echo; //This turns on/off echoing at the command prompt
198  
-
199  
-//Store error strings in flash to save RAM
200  
-void error(const char *str) {
201  
-  NewSerial.print(F("error: "));
202  
-  NewSerial.println(str);
203  
-
204  
-  if (card.errorCode()) {
205  
-    NewSerial.print(F("SD error: "));
206  
-    NewSerial.print(card.errorCode(), HEX);
207  
-    NewSerial.print(',');
208  
-    NewSerial.println(card.errorData(), HEX);
209  
-  }
210 193
 
211  
-  while(1);
  194
+byte setting_uart_speed; //This is the baud rate that the system runs at, default is 9600
  195
+byte setting_system_mode; //This is the mode the system runs in, default is MODE_NEWLOG
  196
+byte setting_escape_character; //This is the ASCII character we look for to break logging, default is ctrl+z
  197
+byte setting_max_escape_character; //Number of escape chars before break logging, default is 3
  198
+byte setting_verbose; //This controls the whether we get extended or simple responses.
  199
+byte setting_echo; //This turns on/off echoing at the command prompt
  200
+
  201
+//Passes back the available amount of free RAM
  202
+int freeRam () 
  203
+{
  204
+#if RAM_TESTING
  205
+  extern int __heap_start, *__brkval; 
  206
+  int v; 
  207
+  return (int) &v - (__brkval == 0 ? (int) &__heap_start : (int) __brkval); 
  208
+#endif
  209
+}
  210
+void printRam() 
  211
+{
  212
+#if RAM_TESTING
  213
+  NewSerial.print(F(" RAM:"));
  214
+  NewSerial.println(freeRam());
  215
+#endif
  216
+}
  217
+
  218
+//Handle errors by printing the error type and blinking LEDs in certain way
  219
+//The function will never exit - it loops forever inside blink_error
  220
+void systemError(byte error_type)
  221
+{
  222
+  NewSerial.print(F("Error "));
  223
+  switch(error_type)
  224
+  {
  225
+  case ERROR_CARD_INIT:
  226
+    NewSerial.print(F("card.init")); 
  227
+    blink_error(ERROR_SD_INIT);
  228
+    break;
  229
+  case ERROR_VOLUME_INIT:
  230
+    NewSerial.print(F("volume.init")); 
  231
+    blink_error(ERROR_SD_INIT);
  232
+    break;
  233
+  case ERROR_ROOT_INIT:
  234
+    NewSerial.print(F("root.init")); 
  235
+    blink_error(ERROR_SD_INIT);
  236
+    break;
  237
+  case ERROR_FILE_OPEN:
  238
+    NewSerial.print(F("file.open")); 
  239
+    blink_error(ERROR_SD_INIT);
  240
+    break;
  241
+  }
212 242
 }
213 243
 
214 244
 void setup(void)
@@ -242,23 +272,17 @@ void setup(void)
242 272
   if(setting_uart_speed == BAUD_38400) NewSerial.begin(38400);
243 273
   if(setting_uart_speed == BAUD_57600) NewSerial.begin(57600);
244 274
   if(setting_uart_speed == BAUD_115200) NewSerial.begin(115200);
245  
-  NewSerial.print("1");
  275
+  NewSerial.print(F("1"));
246 276
 
247 277
   //Setup SD & FAT
248  
-  if (!card.init(SPI_FULL_SPEED)) {
249  
-    NewSerial.print(F("error card.init")); 
250  
-    blink_error(ERROR_SD_INIT);
251  
-  }
252  
-  if (!volume.init(&card)) {
253  
-    NewSerial.print(F("error volume.init")); 
254  
-    blink_error(ERROR_SD_INIT);
255  
-  }
256  
-  if (!currentDirectory.openRoot(&volume)) {
257  
-    NewSerial.print(F("error openRoot")); 
258  
-    blink_error(ERROR_SD_INIT);
259  
-  }
  278
+  if (!card.init(SPI_FULL_SPEED)) systemError(ERROR_CARD_INIT);
  279
+  if (!volume.init(&card)) systemError(ERROR_VOLUME_INIT);
  280
+  currentDirectory.close(); //We close the cD before opening root. This comes from QuickStart example. Saves 4 bytes.
  281
+  if (!currentDirectory.openRoot(&volume)) systemError(ERROR_ROOT_INIT);
260 282
 
261  
-  NewSerial.print("2");
  283
+  NewSerial.print(F("2"));
  284
+
  285
+  printRam(); //Print the available RAM
262 286
 
263 287
   //Search for a config file and load any settings found. This will over-ride previous EEPROM settings if found.
264 288
   read_config_file();
@@ -268,7 +292,7 @@ void loop(void)
268 292
 {
269 293
   //If we are in new log mode, find a new file name to write to
270 294
   if(setting_system_mode == MODE_NEWLOG)
271  
-    newlog();
  295
+    append_file(newlog()); //Append the file name that newlog() returns
272 296
 
273 297
   //If we are in sequential log mode, determine if seqlog.txt has been created or not, and then open it for logging
274 298
   if(setting_system_mode == MODE_SEQLOG)
@@ -281,11 +305,13 @@ void loop(void)
281 305
 //Checks the spots in EEPROM for the next available LOG# file name
282 306
 //Updates EEPROM and then appends to the new log file.
283 307
 //Limited to 65535 files but this should not always be the case.
284  
-void newlog(void)
  308
+char* newlog(void)
285 309
 {
286  
-  uint8_t msb, lsb;
  310
+  byte msb, lsb;
287 311
   uint16_t new_file_number;
288 312
 
  313
+  SdFile newFile; //This will contain the file for SD writing
  314
+
289 315
   //Combine two 8-bit EEPROM spots into one 16-bit number
290 316
   lsb = EEPROM.read(LOCATION_FILE_NUMBER_LSB);
291 317
   msb = EEPROM.read(LOCATION_FILE_NUMBER_MSB);
@@ -310,33 +336,33 @@ void newlog(void)
310 336
   {
311 337
     //Gracefully drop out to command prompt with some error
312 338
     NewSerial.print(F("!Too many logs:1!"));
313  
-    return; //Bail!
  339
+    return(0); //Bail!
314 340
   }
315 341
 
316 342
   //If we made it this far, everything looks good - let's start testing to see if our file number is the next available
317 343
 
318 344
   //Search for next available log spot
319  
-  char new_file_name[] = "LOG00000.TXT";
  345
+  //char new_file_name[] = "LOG00000.TXT";
  346
+  char new_file_name[13];
320 347
   while(1)
321 348
   {
322 349
     new_file_number++;
323 350
     if(new_file_number > 65533) //There is a max of 65534 logs
324 351
     {
325 352
       NewSerial.print(F("!Too many logs:2!"));
326  
-      return;
  353
+      return(0); //Bail!
327 354
     }
328 355
 
329  
-    sprintf(new_file_name, "LOG%05d.TXT", new_file_number); //Splice the new file number into this file name
  356
+    sprintf_P(new_file_name, PSTR("LOG%05d.TXT"), new_file_number); //Splice the new file number into this file name
330 357
 
331 358
     //Try to open file, if fail (file doesn't exist), then break
332  
-    if (file.open(&currentDirectory, new_file_name, O_CREAT | O_EXCL | O_WRITE)) break;
  359
+    if (newFile.open(&currentDirectory, new_file_name, O_CREAT | O_EXCL | O_WRITE)) break;
333 360
   }
334  
-  file.close(); //Close this new file we just opened
335  
-  //file.writeError = false; // clear any write errors
  361
+  newFile.close(); //Close this new file we just opened
336 362
 
337 363
   //Record new_file number to EEPROM
338  
-  lsb = (uint8_t)(new_file_number & 0x00FF);
339  
-  msb = (uint8_t)((new_file_number & 0xFF00) >> 8);
  364
+  lsb = (byte)(new_file_number & 0x00FF);
  365
+  msb = (byte)((new_file_number & 0xFF00) >> 8);
340 366
 
341 367
   EEPROM.write(LOCATION_FILE_NUMBER_LSB, lsb); // LSB
342 368
 
@@ -348,8 +374,8 @@ void newlog(void)
348 374
   NewSerial.println(new_file_name);
349 375
 #endif
350 376
 
351  
-  //Begin writing to file
352  
-  append_file(new_file_name);
  377
+  //  append_file(new_file_name);
  378
+  return(new_file_name);
353 379
 }
354 380
 
355 381
 //Log to the same file every time the system boots, sequentially
@@ -360,18 +386,21 @@ void newlog(void)
360 386
 //Return anything else on sucess
361 387
 void seqlog(void)
362 388
 {
363  
-  char seq_file_name[13] = "SEQLOG00.TXT";
  389
+  SdFile seqFile;
  390
+
  391
+  char sequentialFileName[strlen(SEQ_FILENAME)]; //Limited to 8.3
  392
+  strcpy_P(sequentialFileName, PSTR(SEQ_FILENAME)); //This is the name of the config file. 'config.sys' is probably a bad idea.
364 393
 
365 394
   //Try to create sequential file
366  
-  if (!file.open(&currentDirectory, seq_file_name, O_CREAT | O_WRITE))
  395
+  if (!seqFile.open(&currentDirectory, sequentialFileName, O_CREAT | O_WRITE))
367 396
   {
368 397
     NewSerial.print(F("Error creating SEQLOG\n"));
369 398
     return;
370 399
   }
371 400
 
372  
-  file.close(); //Close this new file we just opened
  401
+  seqFile.close(); //Close this new file we just opened
373 402
 
374  
-  append_file(seq_file_name);
  403
+  append_file(sequentialFileName); 
375 404
 }
376 405
 
377 406
 //This is the most important function of the device. These loops have been tweaked as much as possible.
@@ -381,69 +410,56 @@ void seqlog(void)
381 410
 //Does not exit until Ctrl+z (ASCII 26) is received
382 411
 //Returns 0 on error
383 412
 //Returns 1 on success
384  
-uint8_t append_file(char* file_name)
  413
+byte append_file(char* file_name)
385 414
 {
  415
+  SdFile workingFile;
  416
+
386 417
   // O_CREAT - create the file if it does not exist
387 418
   // O_APPEND - seek to the end of the file prior to each write
388 419
   // O_WRITE - open for write
389  
-  if (!file.open(&currentDirectory, file_name, O_CREAT | O_APPEND | O_WRITE)) error("open1");
390  
-  if (file.fileSize() == 0) {
  420
+  if (!workingFile.open(&currentDirectory, file_name, O_CREAT | O_APPEND | O_WRITE)) systemError(ERROR_FILE_OPEN);
  421
+  if (workingFile.fileSize() == 0) {
391 422
     //This is a trick to make sure first cluster is allocated - found in Bill's example/beta code
392  
-    //file.write((uint8_t)0); //Leaves a NUL at the beginning of a file
393  
-    file.rewind();
394  
-    file.sync();
  423
+    //workingFile.write((byte)0); //Leaves a NUL at the beginning of a file
  424
+    workingFile.rewind();
  425
+    workingFile.sync();
395 426
   }  
396 427
 
397  
-  NewSerial.print('<'); //give a different prompt to indicate no echoing
  428
+  NewSerial.print(F("<")); //give a different prompt to indicate no echoing
398 429
   digitalWrite(statled1, HIGH); //Turn on indicator LED
399 430
 
400  
-#if RAM_TESTING
401  
-  NewSerial.print("Free RAM receive ready: ");
402  
-  NewSerial.println(memoryTest());
403  
-#endif
404  
-
405  
-#define LOCAL_BUFF_SIZE  32
406  
-  uint8_t localBuffer[LOCAL_BUFF_SIZE];
407  
-
408  
-  uint16_t idleTime = 0;
409  
-  const uint16_t MAX_IDLE_TIME_MSEC = 100; //The number of milliseconds before unit goes to sleep
  431
+  const byte LOCAL_BUFF_SIZE = 32; //This is the 2nd buffer. It pulls from the larger NewSerial buffer as quickly as possible.
  432
+  byte localBuffer[LOCAL_BUFF_SIZE];
410 433
 
411  
-  //Ugly calculation to figure out how many times to loop before we need to force a record (file.sync())
412  
-  /*uint32_t maxLoops;
413  
-  uint16_t timeSinceLastRecord = 0; //Sync the file every maxLoop # of bytes
414  
-  if(setting_uart_speed == BAUD_2400) maxLoops = 2400; //Set bits per second
415  
-  if(setting_uart_speed == BAUD_4800) maxLoops = 4800;
416  
-  if(setting_uart_speed == BAUD_9600) maxLoops = 9600;
417  
-  if(setting_uart_speed == BAUD_19200) maxLoops = 19200;
418  
-  if(setting_uart_speed == BAUD_38400) maxLoops = 38400;
419  
-  if(setting_uart_speed == BAUD_57600) maxLoops = 57600;
420  
-  if(setting_uart_speed == BAUD_115200) maxLoops = 115200;
421  
-  maxLoops /= 8; //Convert to bytes per second
422  
-  maxLoops /= LOCAL_BUFF_SIZE; //Convert to # of loops
423  
-  */
  434
+  const uint16_t MAX_IDLE_TIME_MSEC = 500; //The number of milliseconds before unit goes to sleep
  435
+  const uint16_t MAX_TIME_BEFORE_SYNC_MSEC = 5000;
  436
+  uint32_t lastSyncTime = millis(); //Keeps track of the last time the file was synced
424 437
 
  438
+  printRam(); //Print the available RAM
425 439
 
426 440
   //Start recording incoming characters
427 441
   while(1) { //Infinite loop
428 442
 
429  
-    uint8_t n = NewSerial.read(localBuffer, sizeof(localBuffer)); //Read characters from global buffer into the local buffer
  443
+    byte n = NewSerial.read(localBuffer, sizeof(localBuffer)); //Read characters from global buffer into the local buffer
430 444
     if (n > 0) {
431 445
       //Scan the local buffer for esacape characters
432 446
       //In the light version of OpenLog, we don't check for escape characters
433 447
 
434  
-      file.write(localBuffer, n); //Record the buffer to the card
  448
+      workingFile.write(localBuffer, n); //Record the buffer to the card
435 449
 
436 450
       STAT1_PORT ^= (1<<STAT1); //Toggle the STAT1 LED each time we record the buffer
437 451
 
438  
-      idleTime = 0; //We have characters so reset the idleTime
439  
-
440  
-      /*if(timeSinceLastRecord++ > maxLoops) { //This will force a sync approximately every second
441  
-        timeSinceLastRecord = 0;
442  
-        file.sync(); //Sync the card
443  
-      }*/
  452
+      if((millis() - lastSyncTime) > MAX_TIME_BEFORE_SYNC_MSEC) //This will force a sync approximately every 5 seconds
  453
+      { 
  454
+        //This is here to make sure a log is recorded in the instance
  455
+        //where the user is throwing non-stop data at the unit from power on to forever
  456
+        workingFile.sync(); //Sync the card
  457
+        lastSyncTime = millis();
  458
+      }
444 459
     }
445  
-    else if(idleTime > MAX_IDLE_TIME_MSEC) { //If we haven't received any characters in 2s, goto sleep
446  
-      file.sync(); //Sync the card before we go to sleep
  460
+    //No characters recevied?
  461
+    else if( (millis() - lastSyncTime) > MAX_IDLE_TIME_MSEC) { //If we haven't received any characters in 2s, goto sleep
  462
+      workingFile.sync(); //Sync the card before we go to sleep
447 463
 
448 464
       STAT1_PORT &= ~(1<<STAT1); //Turn off stat LED to save power
449 465
 
@@ -454,11 +470,7 @@ uint8_t append_file(char* file_name)
454 470
       power_spi_enable(); //After wake up, power up peripherals
455 471
       power_timer0_enable();
456 472
 
457  
-      idleTime = 0; //A received character woke us up to reset the idleTime
458  
-    }
459  
-    else {
460  
-      idleTime++;
461  
-      delay(1); //Burn 1ms waiting for new characters coming in
  473
+      lastSyncTime = millis(); //Reset the last sync time to now
462 474
     }
463 475
   }
464 476
 
@@ -469,7 +481,7 @@ uint8_t append_file(char* file_name)
469 481
 //=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
470 482
 
471 483
 //Blinks the status LEDs to indicate a type of error
472  
-void blink_error(uint8_t ERROR_TYPE) {
  484
+void blink_error(byte ERROR_TYPE) {
473 485
   while(1) {
474 486
     for(int x = 0 ; x < ERROR_TYPE ; x++) {
475 487
       digitalWrite(statled1, HIGH);
@@ -496,7 +508,7 @@ void check_emergency_reset(void)
496 508
   //Wait 2 seconds, blinking LEDs while we wait
497 509
   pinMode(statled2, OUTPUT);
498 510
   digitalWrite(statled2, HIGH); //Set the STAT2 LED
499  
-  for(uint8_t i = 0 ; i < 40 ; i++)
  511
+  for(byte i = 0 ; i < 40 ; i++)
500 512
   {
501 513
     delay(25);
502 514
     STAT1_PORT ^= (1<<STAT1); //Blink the stat LEDs
@@ -513,9 +525,10 @@ void check_emergency_reset(void)
513 525
   set_default_settings(); //Reset baud, escape characters, escape number, system mode
514 526
 
515 527
   //Try to setup the SD card so we can record these new settings
516  
-  if (!card.init()) error("card.init"); // initialize the SD card
517  
-  if (!volume.init(&card)) error("volume.init"); // initialize a FAT volume
518  
-  if (!currentDirectory.openRoot(&volume)) error("openRoot"); // open the root directory
  528
+  if (!card.init()) systemError(ERROR_CARD_INIT);
  529
+  if (!volume.init(&card)) systemError(ERROR_VOLUME_INIT);
  530
+  currentDirectory.close(); //We close the cD before opening root. This comes from QuickStart example. Saves 4 bytes.
  531
+  if (!currentDirectory.openRoot(&volume)) systemError(ERROR_ROOT_INIT);
519 532
 
520 533
   record_config_file(); //Record new config settings
521 534
 
@@ -621,21 +634,22 @@ void read_system_settings(void)
621 634
 void read_config_file(void)
622 635
 {
623 636
   SdFile rootDirectory;
624  
-  if (!rootDirectory.openRoot(&volume)) error("openRoot"); // open the root directory
  637
+  SdFile configFile;  
  638
+  if (!rootDirectory.openRoot(&volume)) systemError(ERROR_ROOT_INIT); // open the root directory
625 639
 
626  
-  char configFileName[13];
627  
-  sprintf(configFileName, CFG_FILENAME); //This is the name of the config file. 'config.sys' is probably a bad idea.
  640
+  char configFileName[strlen(CFG_FILENAME)]; //Limited to 8.3
  641
+  strcpy_P(configFileName, PSTR(CFG_FILENAME)); //This is the name of the config file. 'config.sys' is probably a bad idea.
628 642
 
629 643
   //Check to see if we have a config file
630  
-  if (!file.open(&rootDirectory, configFileName, O_READ)) {
  644
+  if (!configFile.open(&rootDirectory, configFileName, O_READ)) {
631 645
     //If we don't have a config file already, then create config file and record the current system settings to the file
632 646
 #if DEBUG
633 647
     NewSerial.println(F("No config found - creating default:"));
634 648
 #endif
635  
-    file.close();
  649
+    configFile.close();
636 650
     rootDirectory.close(); //Close out the file system before we open a new one
637 651
 
638  
-    //Record the current eeprom settings to the config file
  652
+      //Record the current eeprom settings to the config file
639 653
     record_config_file();
640 654
     return;
641 655
   }
@@ -648,22 +662,22 @@ void read_config_file(void)
648 662
   //Read up to 20 characters from the file. There may be a better way of doing this...
649 663
   char c;
650 664
   int len;
651  
-  uint8_t settings_string[CFG_LENGTH]; //"115200,103,14,0,1,1\0" = 115200 bps, escape char of ASCII(103), 14 times, new log mode, verbose on, echo on.
  665
+  byte settings_string[CFG_LENGTH]; //"115200,103,14,0,1,1\0" = 115200 bps, escape char of ASCII(103), 14 times, new log mode, verbose on, echo on.
652 666
   for(len = 0 ; len < CFG_LENGTH ; len++) {
653  
-    if( (c = file.read()) < 0) break; //We've reached the end of the file
  667
+    if( (c = configFile.read()) < 0) break; //We've reached the end of the file
654 668
     if(c == '\0') break; //Bail if we hit the end of this string
655 669
     settings_string[len] = c;
656 670
   }
657  
-  file.close();
  671
+  configFile.close();
658 672
   rootDirectory.close();
659 673
 
660 674
 #if DEBUG
661 675
   //Print line for debugging
662  
-  NewSerial.print("Text Settings: ");
  676
+  NewSerial.print(F("Text Settings: "));
663 677
   for(int i = 0; i < len; i++)
664 678
     NewSerial.write(settings_string[i]);
665 679
   NewSerial.println();
666  
-  NewSerial.print("Len: ");
  680
+  NewSerial.print(F("Len: "));
667 681
   NewSerial.println(len);
668 682
 #endif
669 683
 
@@ -676,10 +690,10 @@ void read_config_file(void)
676 690
   char new_system_echo = ON;
677 691
 
678 692
   //Parse the settings out
679  
-  uint8_t i = 0, j = 0, setting_number = 0;
  693
+  byte i = 0, j = 0, setting_number = 0;
680 694
   char new_setting[7]; //Max length of a setting is 6, the bps setting = '115200' plus '\0'
681  
-  uint8_t new_setting_int = 0;
682  
-  
  695
+  byte new_setting_int = 0;
  696
+
683 697
   for(i = 0 ; i < len; i++)
684 698
   {
685 699
     //Pick out one setting from the line of text
@@ -692,16 +706,16 @@ void read_config_file(void)
692 706
 
693 707
     new_setting[j] = '\0'; //Terminate the string for array compare
694 708
     new_setting_int = atoi(new_setting); //Convert string to int
695  
-    
  709
+
696 710
     if(setting_number == 0) //Baud rate
697 711
     {
698  
-      if( strcmp(new_setting, "2400") == 0) new_system_baud = BAUD_2400;
699  
-      else if( strcmp(new_setting, "4800") == 0) new_system_baud = BAUD_4800;
700  
-      else if( strcmp(new_setting, "9600") == 0) new_system_baud = BAUD_9600;
701  
-      else if( strcmp(new_setting, "19200") == 0) new_system_baud = BAUD_19200;
702  
-      else if( strcmp(new_setting, "38400") == 0) new_system_baud = BAUD_38400;
703  
-      else if( strcmp(new_setting, "57600") == 0) new_system_baud = BAUD_57600;
704  
-      else if( strcmp(new_setting, "115200") == 0) new_system_baud = BAUD_115200;
  712
+      if(strcmp_P(new_setting, PSTR("2400")) == 0) new_system_baud = BAUD_2400;
  713
+      else if(strcmp_P(new_setting, PSTR("4800")) == 0) new_system_baud = BAUD_4800;
  714
+      else if(strcmp_P(new_setting, PSTR("9600")) == 0) new_system_baud = BAUD_9600;
  715
+      else if(strcmp_P(new_setting, PSTR("19200")) == 0) new_system_baud = BAUD_19200;
  716
+      else if(strcmp_P(new_setting, PSTR("38400")) == 0) new_system_baud = BAUD_38400;
  717
+      else if(strcmp_P(new_setting, PSTR("57600")) == 0) new_system_baud = BAUD_57600;
  718
+      else if(strcmp_P(new_setting, PSTR("115200")) == 0) new_system_baud = BAUD_115200;
705 719
       else new_system_baud = BAUD_9600; //Default is 9600bps
706 720
     }
707 721
     else if(setting_number == 1) //Escape character
@@ -717,7 +731,7 @@ void read_config_file(void)
717 731
     else if(setting_number == 3) //System mode
718 732
     {
719 733
       new_system_mode = new_setting_int;
720  
-      if(new_system_mode == 0 || new_system_mode > 5) new_system_mode = MODE_NEWLOG; //Default is NEWLOG
  734
+      if(new_system_mode == 0 || new_system_mode > MODE_COMMAND) new_system_mode = MODE_NEWLOG; //Default is NEWLOG
721 735
     }
722 736
     else if(setting_number == 4) //Verbose setting
723 737
     {
@@ -743,15 +757,15 @@ void read_config_file(void)
743 757
   char temp_string[CFG_LENGTH]; //"115200,103,14,0,1,1\0" = 115200 bps, escape char of ASCII(103), 14 times, new log mode, verbose on, echo on.
744 758
   char temp[CFG_LENGTH];
745 759
 
746  
-  if(new_system_baud == BAUD_2400) strcpy(temp_string, "2400");
747  
-  if(new_system_baud == BAUD_4800) strcpy(temp_string, "4800");
748  
-  if(new_system_baud == BAUD_9600) strcpy(temp_string, "9600");
749  
-  if(new_system_baud == BAUD_19200) strcpy(temp_string, "19200");
750  
-  if(new_system_baud == BAUD_38400) strcpy(temp_string, "38400");
751  
-  if(new_system_baud == BAUD_57600) strcpy(temp_string, "57600");
752  
-  if(new_system_baud == BAUD_115200) strcpy(temp_string, "115200");
  760
+  if(new_system_baud == BAUD_2400) strcpy_P(temp_string, PSTR("2400"));
  761
+  if(new_system_baud == BAUD_4800) strcpy_P(temp_string, PSTR("4800"));
  762
+  if(new_system_baud == BAUD_9600) strcpy_P(temp_string, PSTR("9600"));
  763
+  if(new_system_baud == BAUD_19200) strcpy_P(temp_string, PSTR("19200"));
  764
+  if(new_system_baud == BAUD_38400) strcpy_P(temp_string, PSTR("38400"));
  765
+  if(new_system_baud == BAUD_57600) strcpy_P(temp_string, PSTR("57600"));
  766
+  if(new_system_baud == BAUD_115200) strcpy_P(temp_string, PSTR("115200"));
753 767
 
754  
-  sprintf(temp, ",%d,%d,%d,%d,%d\0", new_system_escape, new_system_max_escape, new_system_mode, new_system_verbose, new_system_echo);
  768
+  sprintf_P(temp, PSTR(",%d,%d,%d,%d,%d\0"), new_system_escape, new_system_max_escape, new_system_mode, new_system_verbose, new_system_echo);
755 769
   strcat(temp_string, temp); //Add this string to the system string
756 770
 
757 771
   NewSerial.println(temp_string);
@@ -829,6 +843,18 @@ void read_config_file(void)
829 843
 
830 844
   //All done! New settings are loaded. System will now operate off new config settings found in file.
831 845
 
  846
+  //Set flags for extended mode options  
  847
+  /* These settings are not used in light mode
  848
+  if (setting_verbose == ON)
  849
+    feedback_mode |= EXTENDED_INFO;
  850
+  else
  851
+    feedback_mode &= ((byte)~EXTENDED_INFO);
  852
+
  853
+  if (setting_echo == ON)
  854
+    feedback_mode |= ECHO;
  855
+  else
  856
+    feedback_mode &= ((byte)~ECHO);
  857
+    */
832 858
 }
833 859
 
834 860
 //Records the current EEPROM settings to the config file
@@ -839,37 +865,34 @@ void record_config_file(void)
839 865
   //config file will not be found and it will be created in some erroneus directory. The changes to user settings may be lost on the
840 866
   //next power cycles. To prevent this, we will open another instance of the file system, then close it down when we are done.
841 867
   SdFile rootDirectory;
842  
-  if (!rootDirectory.openRoot(&volume)) error("openRoot"); // open the root directory
  868
+  SdFile myFile;
  869
+  if (!rootDirectory.openRoot(&volume)) systemError(ERROR_ROOT_INIT); // open the root directory
843 870
 
844 871
   char configFileName[strlen(CFG_FILENAME)];
845  
-  sprintf(configFileName, CFG_FILENAME); //This is the name of the config file. 'config.sys' is probably a bad idea.
  872
+  strcpy_P(configFileName, PSTR(CFG_FILENAME)); //This is the name of the config file. 'config.sys' is probably a bad idea.
846 873
 
847 874
   //If there is currently a config file, trash it
848  
-  if (file.open(&rootDirectory, configFileName, O_WRITE)) {
849  
-#if DEBUG
850  
-    NewSerial.println(F("Deleting config"));
851  
-#endif
852  
-    if (!file.remove()){
  875
+  if (myFile.open(&rootDirectory, configFileName, O_WRITE)) {
  876
+    if (!myFile.remove()){
853 877
       NewSerial.println(F("Remove config failed"));
854  
-      file.close(); //Close this file
  878
+      myFile.close(); //Close this file
855 879
       rootDirectory.close(); //Close this file structure instance
856 880
       return;
857 881
     }
858 882
   }
859 883
 
860  
-  //file.close(); //Not sure if we need to close the file before we try to reopen it
  884
+  //myFile.close(); //Not sure if we need to close the file before we try to reopen it
861 885
 
862 886
   //Create config file
863  
-  if (file.open(&rootDirectory, configFileName, O_CREAT | O_APPEND | O_WRITE) == 0) {
  887
+  if (myFile.open(&rootDirectory, configFileName, O_CREAT | O_APPEND | O_WRITE) == 0) {
864 888
     NewSerial.println(F("Create config failed"));
865  
-    file.close(); //Close this file
  889
+    myFile.close(); //Close this file
866 890
     rootDirectory.close(); //Close this file structure instance
867 891
     return;
868 892
   }
869 893
   //Config was successfully created, now record current system settings to the config file
870 894
 
871 895
   char settings_string[CFG_LENGTH]; //"115200,103,14,0,1,1\0" = 115200 bps, escape char of ASCII(103), 14 times, new log mode, verbose on, echo on.
872  
-  char temp[CFG_LENGTH]; //This contains bits of the overall config string.
873 896
 
874 897
   //Before we read the EEPROM values, they've already been tested and defaulted in the read_system_settings function
875 898
   char current_system_baud = EEPROM.read(LOCATION_BAUD_SETTING);
@@ -880,32 +903,28 @@ void record_config_file(void)
880 903
   char current_system_echo = EEPROM.read(LOCATION_ECHO);
881 904
 
882 905
   //Determine current baud and copy it to string
883  
-  if(current_system_baud == BAUD_2400) strcpy(settings_string, "2400");
884  
-  if(current_system_baud == BAUD_4800) strcpy(settings_string, "4800");
885  
-  if(current_system_baud == BAUD_9600) strcpy(settings_string, "9600");
886  
-  if(current_system_baud == BAUD_19200) strcpy(settings_string, "19200");
887  
-  if(current_system_baud == BAUD_38400) strcpy(settings_string, "38400");
888  
-  if(current_system_baud == BAUD_57600) strcpy(settings_string, "57600");
889  
-  if(current_system_baud == BAUD_115200) strcpy(settings_string, "115200");
  906
+  char baudRate[6];
  907
+  if(current_system_baud == BAUD_2400) strcpy_P(baudRate, PSTR("2400"));
  908
+  if(current_system_baud == BAUD_4800) strcpy_P(baudRate, PSTR("4800"));
  909
+  if(current_system_baud == BAUD_9600) strcpy_P(baudRate, PSTR("9600"));
  910
+  if(current_system_baud == BAUD_19200) strcpy_P(baudRate, PSTR("19200"));
  911
+  if(current_system_baud == BAUD_38400) strcpy_P(baudRate, PSTR("38400"));
  912
+  if(current_system_baud == BAUD_57600) strcpy_P(baudRate, PSTR("57600"));
  913
+  if(current_system_baud == BAUD_115200) strcpy_P(baudRate, PSTR("115200"));
890 914
 
891 915
   //Convert system settings to visible ASCII characters
892  
-  sprintf(temp, ",%d,%d,%d,%d,%d\0", current_system_escape, current_system_max_escape, current_system_mode, current_system_verbose, current_system_echo);
893  
-  strcat(settings_string, temp); //Add this string to the system string
894  
-
895  
-#if DEBUG
896  
-  NewSerial.print(F("\nSetting string: "));
897  
-  NewSerial.println(settings_string);
898  
-#endif
899  
-
  916
+  sprintf_P(settings_string, PSTR("%s,%d,%d,%d,%d,%d\0"), baudRate, current_system_escape, current_system_max_escape, current_system_mode, current_system_verbose, current_system_echo);
900 917
   //Record current system settings to the config file
901  
-  if(file.write((uint8_t*)settings_string, strlen(settings_string)) != strlen(settings_string))
  918
+  if(myFile.write(settings_string, strlen(settings_string)) != strlen(settings_string))
902 919
     NewSerial.println(F("error writing to file"));
903 920
 
904 921
   //Add a decoder line to the file
905  
-  file.write("\n\rbaud,escape,esc#,mode,verb,echo\0"); //Add this string to the file
  922
+  char helperString[47]; //This probably should not be hard coded but we're doing it anyway!
  923
+  strcpy_P(helperString, PSTR("\n\rbaud,escape,esc#,mode,verb,echo\0")); //This is the name of the config file. 'config.sys' is probably a bad idea.
  924
+  myFile.write(helperString); //Add this string to the file
906 925
 
907  
-  file.sync(); //Sync all newly written data to card
908  
-  file.close(); //Close this file
  926
+  myFile.sync(); //Sync all newly written data to card
  927
+  myFile.close(); //Close this file
909 928
   rootDirectory.close(); //Close this file structure instance
910 929
   //Now that the new config file has the current system settings, nothing else to do!
911 930
 }

0 notes on commit 2dbdede

Dan

This returns a pointer to memory locations on the stack that are likely to be overwritten by subsequent function calls!

Nathan Seidle

I'm all ears if this is a problem. I changed this because I found that more RAM was released when using printRam(). I attributed this to the release of local variables when we return from newlog(), then go into append_file().

Dan

Yes, I expect more RAM would be available after the return from newlog(), but the returned pointer is being passed to append_file() and that is the potential problem waiting to happen. Since that pointer is pointing to available stack space to the compiler, depending on its position on the stack, it's contents (i.e. the specified file name) could be overwritten by the normal stack-based operations before the current contents is used. It seldom is a good idea to use any reference to a function's local variables outside of the function! I think you got lucky, the location of the file name on the stack is probably far enough up the stack to not have been clobbered before it was used in append_file()'s call to the file system open() function. A simple experiment to try is to move the definition of new_file_name to the beginning of the newlog() function to see if the file name string is still intact when the open() function is called in append_file().

Please sign in to comment.
Something went wrong with that request. Please try again.