Arduino code powering horoscope generator

This code works, in the same way that a cardboard bicycle works until you cycle through a puddle. It powers a box that plays a sound file in sync with some lights on movement detected by a PIR sensor. Once the sound file has finished it concatenates some text and two random strings from an array and prints it. Once it's done it resets so the motion sensor is active again.

```
//MUSIC MAKER SETUP
#include

#include
#include
#include
#include
#define SHIELD_RESET -1 // VS1053 reset pin (unused!)
#define SHIELD_CS 7 // VS1053 chip select pin (output)
#define SHIELD_DCS 6 // VS1053 Data/command select pin (output)
#define CARDCS 4 // Card chip select pin
#define DREQ 3 // VS1053 Data request, ideally an Interrupt pin

#include
#include "SoftwareSerial.h"
#include "Adafruit_Thermal.h"
#include
#include "skull.h"

//PIN USE
//MP3 SHIELD: 3, 4, 6, 7
//PIR SENSOR: A0, left gnd, left 5v
//LIGHTS: 10 for green data in. yellow not used.
//THERMAL PRINTER: 8 and 9

//PRINTER
int printer_TX_Pin = 8; // This is the yellow wire
int printer_RX_Pin = 9; // This is the green wire
Adafruit_Thermal printer(printer_RX_Pin, printer_TX_Pin);

//PIXELS!!!
#define PIN 10
#define NUMPIXELS 16

//PIR SENSOR
#define pir A0
#define led 13

Adafruit_NeoPixel pixels = Adafruit_NeoPixel(NUMPIXELS, PIN, NEO_GRB + NEO_KHZ800);

Adafruit_VS1053_FilePlayer musicPlayer = Adafruit_VS1053_FilePlayer(SHIELD_RESET, SHIELD_CS, SHIELD_DCS, DREQ, CARDCS);

boolean playing = false;

const char string_0[] PROGMEM = "You have a great need for other people to like and admire you. ";
const char string_1[] PROGMEM = "You have a tendency to be critical of yourself. ";
const char string_2[] PROGMEM = "You have a great deal of unused capacity which you have not turned to your advantage. ";
const char string_3[] PROGMEM = "While you have some personality weaknesses, you are generally able to compensate for them. ";
const char string_4[] PROGMEM = "Your sexual adjustment has p

I'm not extremely familiar with Arduino development so I might be a bit off but some things I noticed:

-
color1, color2, color2 are not very descriptive names. If they corresponds to color channels they should be named accordingly like red, green, blue.

-
You have a couple of methods which take a time like delay_time or gap_time. It might be an absolutely ingrained standard in Arduino development that times are always measured in milliseconds but appending the unit to the name like display_time_ms or gap_time_ms would make it clear to reader what the time unit should be.

-
I'm not sure why you copy the random word you select into a buffer and then append it to a String object. It looks to me that this is an unnecessary temporary copy. If I read the documentation right this:

output = text + buffer;

essentially gets transformed into

output = text.concat(String(buffer))

(given that text and output are String object while buffer is a char[])

So to me this looks like it could be simplified to:

int rand = random(0,13);
char* random_word = (char*)pgm_read_word(&(string_table[rand]));
String text = "Your personalised reading: ";
String output = text + random_word;

which eliminates the requirement for the temporary buffer copy.

StackExchange Code Review Q#83863, answer score: 5