Delay not working in function other than loop

Question

So I just got an Arduino and thought I would play around a bit.

However not five minutes in I already ran into a problem.

I tried programming a simple SOS blinking light sequence. To get rid of code duplication I refactored the on and off turning of the LED into functions and then again refactored a short blink and a long blink function which are expected to turn the LED on for a short time and then for a long time.

The problem is the delay in those functions is completely ignored.

int ledPin = 13;

void on()
{
 digitalWrite(ledPin,HIGH); 
}

void off()
{
 digitalWrite(ledPin,LOW);
}


void shortB()
{
 on();
 delay(1000);
 off();
} 

void longB()
{
 on();
 delay(2000);
 off(); 
}

void setup()
{
  pinMode(ledPin, OUTPUT); 
}

void loop()
{
 shortB();
 shortB();
 shortB();
 shortB();
 longB();
 longB();
 longB();
 shortB();
 shortB();
 shortB();  
 delay(5000);
}

My expected behavior would be that it does an SOS and then waits 5 seconds and repeats.

What really happens however is that it stay continuously lit for a second or so then turns off for 5 seconds.

Using

on();
delay(1000);
off();

In the main loop works so I guess it simply ignores the delays that are put into the other functions. My question would be why and ho do I fix it?

I know that I can use other timer function constructs however I really think this is something that should work by default.

Answer 1

You have no delay in between turning the LED off and turning it back on. Add one.

Answer 2

In addition to the other problems, you should have a break between letters. So, for example, "S" is not just dot-dot-dot it is dot-dot-dot-pause.

Example code below that implements pauses between letters, as well as longer pauses between words.

const byte LED = 13;

char * letters [26] = {
   ".-",     // A
   "-...",   // B
   "-.-.",   // C
   "-..",    // D
   ".",      // E
   "..-.",   // F
   "--.",    // G
   "....",   // H
   "..",     // I
   ".---",   // J
   "-.-",    // K
   ".-..",   // L
   "--",     // M
   "-.",     // N
   "---",    // O
   ".--.",   // P
   "--.-",   // Q
   ".-.",    // R
   "...",    // S
   "-",      // T
   "..-",    // U
   "...-",   // V
   ".--",    // W
   "-..-",   // X
   "-.--",   // Y
   "--.."    // Z
};

char * numbers [10] = 
  {
  "-----",  // 0
  ".----",  // 1
  "..---",  // 2
  "...--",  // 3
  "....-",  // 4
  ".....",  // 5
  "-....",  // 6
  "--...",  // 7
  "---..",  // 8
  "----.",  // 9
  };

const unsigned long dotLength = 100;  // mS
const unsigned long dashLength = dotLength * 3;  
const unsigned long wordLength = dashLength * 2; 

// get a line from serial
//  forces to upper case
void getline (char * buf, size_t bufsize)
{
byte i;

  // discard any old junk
  while (Serial.available ())
    Serial.read ();

  for (i = 0; i < bufsize - 1; )
    {
    if (Serial.available ())
      {
      int c = Serial.read ();

      if (c == '\n')  // newline terminates
        break;

      buf [i++] = toupper (c);
      } // end if available
    }  // end of for
  buf [i] = 0;  // terminator
  Serial.println (buf);  // echo what they typed
  }     // end of getline

void dot ()
  {
  digitalWrite (LED, HIGH);
  delay (dotLength);
  digitalWrite (LED, LOW); 
  delay (dotLength);
  }  // end of dot

void dash ()
  {
  digitalWrite (LED, HIGH);
  delay (dashLength);
  digitalWrite (LED, LOW); 
  delay (dotLength);
  }  // end of dash

void setup ()
  {
  Serial.begin (115200);
  Serial.println ();
  pinMode (LED, OUTPUT);
  }  // end of setup

void loop ()
  {
  Serial.println ("Enter your message ...");
  char buf [60];
  getline (buf, sizeof buf);


  // for each letter
  for (char * p = buf; *p; p++)
    {
    char c = *p;
    char * sequence = NULL;

    if (c >= 'A' && c <= 'Z')
      sequence = letters [c - 'A'];
    else if (c >= '0' && c <= '9')
      sequence = numbers [c - '0'];   
    else if (c == ' ')
      {
      delay (wordLength);   // gap between words
      continue;
      }

    // ignore not in table
    if (sequence == NULL)
      continue;

    // output sequence for one letter
    for (char * s = sequence; *s; s++)
      {
      if (*s == '.')
        dot ();
      else if  (*s == '-')
        dash (); 
      }
    // now a gap
    delay (dashLength);   
    }  // end of for each letter

  }  // end of loop