Optimizing the code for a finite state machine datalogger

I have recently put together a prototype for a new wireless device for operant behavioral research experiments. It is essentially a "Skinner box". When a test subject breaks one of two infrared beams, the Adafruit Feather 32u4 Adalogger controls a pump to deliver a liquid reward and an indicator RGB LED, and also writes the data to a microSD card. I built the program using a finite state machine library to control the transitions between states. The device works well but now I am trying to optimize the code beyond my simple skills. I am especially concerned about the repetition to print the data to microSD. Can anyone suggest ways to improve this code? I appreciate any advice or suggestions that you can provide.

//  Attach the Finite State Machine, RGB LED, and SD libraries
#include <Fsm.h>
#include <RGBLed.h>
#include <SD.h>
 
char filename[] = "10LOG000.csv";
 
//  Define pins for sensors, LED, pump
const int sensorPin1 = 12;
const int sensorPin2 = 11;
const int     redPin = 5;
const int   greenPin = 6;
const int    bluePin = 9;
const int    pumpPin = 10;
const int    cardPin = 4;
 
//  Define variables
int sensor1 = 0; // stores sensor1 value
int sensor2 = 0; // stores sensor2 value
 int sensor = 0; // triggers state transitions
  int timer = 0; // triggers state transitions
 
//  Define timers and intervals
unsigned long  t__sess = 7200000;
unsigned long  t__time = 900000;
 
unsigned long  t_prime = 0;
unsigned long  t_power = 0;
unsigned long  t_start = 0;
unsigned long  t_rewar = 0;
unsigned long  t_timeo = 0;
unsigned long  t_ready = 0;
unsigned long  t__stop = 0;
 
unsigned long  t__resp = 0;
unsigned long  t__rein = 0;
unsigned long  t_inact = 0;
unsigned long prevTime = 0;
 
// Define counters
int c__resp = 0;
int c__rein = 0;
int c_inact = 0;
int c_prime = 0;
int c_power = 0;
int c_start = 0;
int c_rewar = 0;
int c_timeo = 0;
int c_ready = 0;
int c__stop = 0;
 
//  Define the RGBLed object to use the RGB LED library functions
RGBLed led(redPin, greenPin, bluePin, COMMON_CATHODE);
 
//  Define the SD card object
File myFile;
 
// Define the custom functions for the state machine
// Enter functions run once, update functions loop indefinitely, exit functions run once upon exit
void power_enter() {
  t_power = millis();
  c_power++;
  myFile = SD.open(filename, FILE_WRITE);
  myFile.print("power,");
  myFile.print(c_power);
  myFile.print(",");
  myFile.println(t_power);
  myFile.close();
}
void power_update() {
  led.setColor(255, 255, 255); // white
  readSensor1();
  readSensor2();
}
void power_exit() {
}
 
void prime_enter() {
  t_prime = millis();
  c_prime++;
  myFile = SD.open(filename, FILE_WRITE);
  myFile.print("prime,");
  myFile.print(c_prime);
  myFile.print(",");
  myFile.println(t_prime);
  myFile.close();
}
void prime_update() {
  led.setColor(0, 255, 0); // green
  unsigned long pump_interval = 4500;
  if (millis() - t_prime < pump_interval) {
    digitalWrite(pumpPin, HIGH);
  }
  else {
    digitalWrite(pumpPin, LOW);
  }
  readSensor1();
  readSensor2();
}
void prime_exit() {
}
 
void start_enter() {
  t_start = millis();
  c_start++;
  myFile = SD.open(filename, FILE_WRITE);
  myFile.print("start,");
  myFile.print(c_start);
  myFile.print(",");
  myFile.println(t_start);
  myFile.close();
}
void start_update() {
  led.setColor(0, 0, 255); // blue
  beamBroke();
  readSensor1();
  readSensor2();
  sessionTimer();
}
void start_exit() {
}
 
void rewar_enter() {
  t_rewar = millis();
  c_rewar++;
  myFile = SD.open(filename, FILE_WRITE);
  myFile.print("pump,");
  myFile.print(c_rewar);
  myFile.print(",");
  myFile.println(t_rewar);
  myFile.close();
}
void rewar_update() {
  led.setColor(255, 0, 0); // red
  unsigned long pump_interval = 1500;
  if (millis() - t_rewar < pump_interval) {
    digitalWrite(pumpPin, HIGH);
  }
  else {
    digitalWrite(pumpPin, LOW);
  }
  ;
  readSensor1();
  readSensor2();
  sessionTimer();
}
void rewar_exit() {
}
 
void timeo_enter() {
  t_timeo = millis();
  c_timeo++;
  myFile = SD.open(filename, FILE_WRITE);
  myFile.print("timeo,");
  myFile.print(c_timeo);
  myFile.print(",");
  myFile.println(t_timeo);
  myFile.close();
}
void timeo_update() {
  led.setColor(255, 0, 255); // purple
  readSensor1();
  readSensor2();
  sessionTimer();
}
void timeo_exit() {
}
 
void ready_enter() {
  t_ready = millis();
  c_ready++;
  myFile = SD.open(filename, FILE_WRITE);
  myFile.print("ready,");
  myFile.print(c_ready);
  myFile.print(",");
  myFile.println(t_ready);
  myFile.close();
}
void ready_update() {
  led.setColor(0, 0, 255); // blue
  beamBroke();
  readSensor1();
  readSensor2();
  sessionTimer();
}
void ready_exit() {
}
 
void stop__enter() {
  t__stop = millis();
  c__stop++;
  myFile = SD.open(filename, FILE_WRITE);
  myFile.print("stop,");
  myFile.print(c__stop);
  myFile.print(",");
  myFile.println(t__stop);
  myFile.close();
  digitalWrite(pumpPin, LOW);
}
void stop__update() {
  led.setColor(0, 0, 0);    // off
  digitalWrite(pumpPin, LOW);
}
void stop__exit() {
}
 
// Define the states, including custom functions
State power(power_enter, power_update, power_exit);
State prime(prime_enter, prime_update, prime_exit);
State start(start_enter, start_update, start_exit);
State rewar(rewar_enter, rewar_update, rewar_exit);
State timeo(timeo_enter, timeo_update, timeo_exit);
State ready(ready_enter, ready_update, ready_exit);
State stop_(stop__enter, stop__update, stop__exit);
 
// Start the machine in the POWER state
Fsm fsm_operant(&power);
 
// Define the custom functions
void beamBroke() {
  sensor = digitalRead(sensorPin1);
  if (sensor == LOW) {
    t__rein = millis();
    c__rein++;
    myFile = SD.open(filename, FILE_WRITE);
    myFile.print("rein,");
    myFile.print(c__rein);
    myFile.print(",");
    myFile.println(t__rein);
    myFile.close();
    fsm_operant.trigger(sensor);
  }
}
 
void readSensor1() {
  sensor1 = digitalRead(sensorPin1);
  if (sensor1 == LOW) {
    t__resp = millis();
    c__resp++;
  myFile = SD.open(filename, FILE_WRITE);
  myFile.print("resp,");
  myFile.print(c__resp);
  myFile.print(",");
  myFile.println(t__resp);
  myFile.close();
  }
}
 
void readSensor2() {
  sensor2 = digitalRead(sensorPin2);
  if (sensor2 == LOW) {
    t_inact = millis();
    c_inact++;
    myFile = SD.open(filename, FILE_WRITE);
    myFile.print("inact,");
    myFile.print(c_inact);
    myFile.print(",");
    myFile.println(t_inact);
    myFile.close();
  }
}
 
void sessionTimer() {
  if (millis() - prevTime >= t__time) {
    prevTime = millis();
    myFile = SD.open(filename, FILE_WRITE);
    myFile.print("time,");
    myFile.print("1,");
    myFile.println(prevTime);
    myFile.close();
  }
  if (millis() - t_start >= t__sess) {
    fsm_operant.trigger(timer);
  }
}
 
void setup() {
//  Define timed transitions        from     to    time
  fsm_operant.add_timed_transition(&power, &prime, 3000, NULL);
  fsm_operant.add_timed_transition(&prime, &start, 8000, NULL);
  fsm_operant.add_timed_transition(&rewar, &timeo, 8000, NULL);
  fsm_operant.add_timed_transition(&timeo, &ready, 6000, NULL);
 
// Define variable transitions from    to     var
  fsm_operant.add_transition(&start, &rewar, sensor, NULL);
  fsm_operant.add_transition(&ready, &rewar, sensor, NULL);
  fsm_operant.add_transition(&start, &stop_,  timer, NULL);
  fsm_operant.add_transition(&rewar, &stop_,  timer, NULL);
  fsm_operant.add_transition(&timeo, &stop_,  timer, NULL);
  fsm_operant.add_transition(&ready, &stop_,  timer, NULL);
 
  pinMode(sensorPin1, INPUT);
  pinMode(sensorPin2, INPUT);
  pinMode(pumpPin, OUTPUT);
  pinMode(cardPin, OUTPUT);
  digitalWrite(sensorPin1, HIGH);
  digitalWrite(sensorPin2, HIGH);
  SD.begin(cardPin);
  for (uint8_t i = 0; i < 1000; i++) {
    filename[6] = i/10 + '0';
    filename[7] = i%10 + '0';
    if (! SD.exists(filename)) {
      myFile = SD.open(filename, FILE_WRITE);
      break;
    }
}
  myFile = SD.open(filename, FILE_WRITE);
  myFile.print("event,");
  myFile.print("count,");
  myFile.println("time");
  myFile.close();
}
 
void loop() {
  fsm_operant.run_machine();
}

Are you asking for FREE help with a commercial product?

Paul

Paul_KD7HB:
Are you asking for FREE help with a commercial product?

Sorry, I guess I should have specified that this is an academic research project and I will be releasing all plans, code, and files as open-source when I'm happy with it.

Confused..
Problem - Your code is working exactly as needed but you want to “optimize” it.

Whatever you may think optimizing means to me, it would be highly unlikely I would guess what your trying to accomplish.

I agree it’s a lot of code for just two input pins to control a output. You have used a state machine library that could run a small factory, but if it fits in the microprocessor, what’s really the issue?

If it works to your satisfaction, it is fine.

However, if you hope that other people will find the code useful (and actually use it), consider adding comments so that potential users would have some clue what the various parts are doing.

A year down the road, and changes are required, you will probably have forgotten much of what happens where, as well.

Slumpert:
Confused..
Problem - Your code is working exactly as needed but you want to “optimize” it.

Whatever you may think optimizing means to me, it would be highly unlikely I would guess what your trying to accomplish.

I agree it’s a lot of code for just two input pins to control a output. You have used a state machine library that could run a small factory, but if it fits in the microprocessor, what’s really the issue?

I understand. I think my major issue is the number of times I call the print-to-SD card functions, including opening and closing that file. Is there a way to streamline that part of the code to make it more processor- or battery-efficient?

The trade-off is reliability. Closing the file forces the write. In the worst case, if the power fails, you only lose the last thing written. By leaving the file open you are at the mercy of whatever buffering strategy is used by the SD card library.

As @jremington suggests, were I in your shoes I would add comments. (And change a few names. "Sensor" is way too generic of a name.)

cschmoutz:
Is there a way to streamline that part of the code to make it more processor-efficient?

I promise. The processor is not going on-strike if you over work it.

cschmoutz:
Is there a way to streamline that part of the code to make it more battery-efficient?

Is that a problem?

cschmoutz:
to make it more processor- or battery-efficient?

If you can run the whole thing at 3v from a 3v battery you will be a lot more battery-efficient.

...R

Thank you all for the reassurances. I am new to the arduino (and programming and design) game and constantly second-guessing my choices. I will definitely be commenting my code more, for both others and future-me.

Good comments explain WHY the code does what it does, not what it does. Compare the two items below...

 x = x >> 1;  //divide by 2
distance = distance / 2; //ultrasonic sensor measures distance out to and back from an object, so distance to the object is half of that measurement.