Refactor to save bytes?

Question

I'm a painful 128 bytes of my max for this Trinket. Any refactoring I could do to get down the size?

#include <Adafruit_NeoPixel.h>
#include <Wire.h>
#include <RTClib.h>

#define LEDPIN 1
#define TONE 4
#define BTNPIN 3

RTC_DS1307 rtc;
Adafruit_NeoPixel strip = Adafruit_NeoPixel(5, LEDPIN, NEO_RGB);

const long fadeTime = 10 * 60000L; // X minutes
const int colorStops = 255;
const int delaySpeed = fadeTime / colorStops;
const int notes[] = {262,294,330,349};
const uint32_t alarmLength = 5 * 60000L; // 5 minutes

void setup() {
  rtc.begin();
  rtc.adjust(DateTime(__DATE__, __TIME__));

  pinMode(TONE,OUTPUT);
  pinMode(BTNPIN, INPUT);
  digitalWrite(BTNPIN, HIGH);

  strip.begin();
  strip.show();
}

void loop() {
  DateTime now = rtc.now();

  // If it's 5:10 am on a week day, start the fade in
  if (now.hour() == 5 && now.minute() == 10 && now.second() == 0 && now.dayOfTheWeek() != 1 && now.dayOfTheWeek() != 7){
    // Fade in light
    for(int i = 1; i<colorStops; i++){
      if (kill() == true){ break; }

      for(int np = 0;np<strip.numPixels(); np++){
        strip.setPixelColor(np, strip.Color(i,i,i));
      }
      strip.show();
      delay(delaySpeed);
    }

    // Play tone after light fully bright
    for( uint32_t tStart = millis();  (millis()-tStart) < alarmLength; ){
      if (kill() == true){ break; }

      for(int np = 0;np<strip.numPixels(); np++){
         strip.setPixelColor(np, strip.Color(random(50,255),random(50,255),random(50,255)));
      }
      strip.show();

      beep(notes[random(0,3)]);
      delay(100);
    }

    turnOffPixels();
  }
}

void beep(int delayAmount)
{
  for (uint16_t t = 0; t < 30*1000/2; t += delayAmount)
  {  
    digitalWrite(TONE,HIGH);
    delayMicroseconds(delayAmount);
    digitalWrite(TONE,LOW);
    delayMicroseconds(delayAmount);
  }  
}

void turnOffPixels() {
  for(int np = 0;np<strip.numPixels(); np++){
    strip.setPixelColor(np, strip.Color(0,0,0));
  }
  strip.show();
}

bool kill() {
  if (! digitalRead(BTNPIN)) {
    turnOffPixels();
    return true;
  } else {
    return false;
  }
}

Error...

Sketch uses 5,438 bytes (102%) of program storage space. Maximum is 5,310 bytes.
Global variables use 97 bytes of dynamic memory.

score 2 · Accepted Answer · answered May 21 '16 at 09:21

Remove the rtc.adjust(DateTime(__DATE__, __TIME__));. You only need to set the RTC once, after e.g. you insert the backup battery.

Change

pinMode(BTNPIN, INPUT);
digitalWrite(BTNPIN, HIGH);

to

pinMode(BTNPIN, INPUT_PULLUP);

Inline the beep function using the inline hint:

inline void beep(int delayAmount)
...

Shorter beep function:

inline void beep(int delayAmount)
{
  for (uint16_t t = 0; t < 30*1000; t += delayAmount)
  {  
    digitalWrite(TONE,t & 0x01);// HIGH is odd, LOW if even
    delayMicroseconds(delayAmount);
  }  
}

score 1 · Answer 2 · answered May 21 '16 at 05:48

On a quick test build for an ATmega328p type Arduino, the implementation of random() and its unique dependencies seems to be about 180 bytes, so you might see if there's a more compact way of achieving your need there, perhaps using the low byte of some mathematical expression of the time, and just retrying until you get an in-range value.

Of course if you get really stuck, you could remove the bootloader and program it with ISP from an ordinary Arduino or something like that.

score 1 · Answer 3 · answered May 21 '16 at 12:30

1

Very good suggestions so far. I have only one to add: instead of strip.Color(0,0,0) use just the constant number 0. This saves a function call.

answered May 21 '16 at 12:30

Edgar Bonet

45,094
4
42
81

James Waldby - jwpat7 · Answer 4 · 2016-05-21T03:48:04.313

For refactoring, consider using a for loop that executes twice, once for the “// Fade in light” section of code, and once for the “// Play tone after light fully bright” section. For parts of the code that differ, test the loop index and do one action or the other.

Here are two ways to get rid of a few (eg 54) bytes of code:

Instead of

 Adafruit_NeoPixel strip = Adafruit_NeoPixel(5, LEDPIN, NEO_RGB);
 ...
 for(int np = 0;np<strip.numPixels(); np++){
 ...

(with three instances of the latter), say something like

enum {numPixels = 5 };
Adafruit_NeoPixel strip = Adafruit_NeoPixel(numPixels, LEDPIN, NEO_RGB);
...
for (byte np=0; np<numPixels; ++np){
...

This will save about 40 bytes (10 bytes per for, and another 10 probably due to strip.numPixels() code not linking). (Note, the savings is due to getting rid of function calls; the change from int to byte apparently makes no difference in program size, probably because the compiler optimizes int to byte where its analysis shows it can do so.)

Instead of

if (... && now.dayOfTheWeek() != 1 && now.dayOfTheWeek() != 7){

say

if (... && now.dayOfTheWeek()+250>250){

which will save 14 bytes. This works as explained in my answer to your previous question. However, I've written +250>250 here instead of the +249>250 there because according to RTClib documentation and RTClib.cpp code, .dayOfTheWeek() returns 0 for Saturday, rather than 1 as you seem to expect.

Possibly inline beep(), since you call it only once.

Refactor to save bytes?

4 Answers4