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.

• Blackclaws: Ouch ... that was a really bad beginners error. That fixed it.
• JRobert: And it looks like your code sends 'H', 'O', 'S' (the first character is four short 4 blinks). Are you really seeing 'S', 'O', 'S'?

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