Go Down

Topic: Telegraph Project from Quick-start Guide runs incorrectly (Read 3 times) previous topic - next topic

Zoandar

I have a project built from a book called Arduino: A Quick-Start Guide, which is a Morse Code generator. It uses some library files which I also had to create from the book. All the code compiles OK, and the project attempts to run, but it isn't running correctly. It is supposed to flash an LED on pin 13 in Morse Code to display the equivalent of "Hello World", repeating every 5 seconds. But since that is kind of hard to decrypt when watching an LED I changed the statement in the program so it is just supposed to send "SM". I chose these letters because S is "..." and M is "--", so easy to determine if the sketch is working properly. However, what I am getting is simply one "." followed by one "-", repeating every 5 seconds. It seems to be a problem with a loop of some sort.

I am pretty new to Arduino, and although I have posted other sketch code on the forum this is the first project I am posting about which uses library files that I had to write, so I am not sure of the correct format for posting the whole package in this message. I guess I need to put in the entire code for the main sketch, followed by the entire code for each of the 3 library files it uses, so anyone helping  can see all the code involved?

If that is wrong, please let me know the proper way to do it. Sorry I don't have more comments, but I am still figuring out how this is supposed to work. Since I am new to this, there may have been an error in the book's listing for part of the code, and I might not have learned enough so far to be able to spot it. But I did look everything over several times.

I suspected part of the Telegraph.cpp file where it has the segment with a FOR loop beginning with

Code: [Select]


void Telegraph::send_message(const char* message) {



But when I tried removing "strlen(message)" and replacing it with the number 4 (to get it to loop 4 times) the result was still the same as above. I do know that for each change I make to the library files I have to re-copy them from the folder in the project where they get created and paste them into the Telegraph subfolder of the Libraries folder as the book instructed, and then restart the Arduino IDE, and re-upload the sketch to the board. So I have not been able to figure out why the sketch won't loop through all the dots and dashes of "SM".

Here is the main sketch:

Code: [Select]


//Telegraph/examples/HelloWorld/HelloWorld.pde

#include "telegraph.h"

const unsigned int OUTPUT_PIN = 13;
const unsigned int DIT_LENGTH = 100;

Telegraph telegraph(OUTPUT_PIN, DIT_LENGTH);

void setup() {}

void loop() {
 telegraph.send_message("SM");   //note that there were no entries for punctuation marks in the Morse Code character array
 delay(5000);
}




Here are the 3 library files from the tabs in the Telegraph sketch which contains them:

Code: [Select]

//Telegraph/Telegraph.pde

void setup() (
)

void loop() {
}




Code: [Select]


//Telegraph/telegraph.cpp

#include <ctype.h>
#include <WProgram.h>
#include "telegraph.h"

char* LETTERS[] = {
 ".-", "-...", "-.-.", "-..", ".",      // A-E
 "..-.", "--.", "....", "..", ".---",   // F-J
 "-.-", ".-..", "--", "-.", "---",      // K-O
 ".--.", "--.-", ".-.", "...", "-.--",  // P-T
 "..-", "...-", ".--", "-..-", "-.--",  // U-Y
 "--.."
};

char* DIGITS[] = {
 "-----", ".----", "..---", "...--",    // 0-3
 "....-", ".....", "-....", "--...",    // 4-7
 "---..", "----."
};

Telegraph::Telegraph(const int output_pin, const int dit_length) {
 _output_pin = output_pin;
 _dit_length = dit_length;
 _dah_length = dit_length * 3;
 pinMode(_output_pin, OUTPUT);
}

void Telegraph::output_code(const char* code) {
 for (int i = 0; i < strlen(code); i++) {
   if (code[i] == '.')
     dit();
   else
     dah();
 }
}

void Telegraph::dit() {
 Serial.print(".");
 output_symbol(_dit_length);
}

void Telegraph::dah() {
 Serial.print("-");
 output_symbol(_dah_length);
}

void Telegraph::output_symbol(const int length) {
 digitalWrite(_output_pin, HIGH);
 delay(length);
 digitalWrite(_output_pin, LOW);
}

void Telegraph::send_message(const char* message) {
   for (int i = 0; i < strlen(message); i++) {
       const char current_char = toupper(message[i]);
   if (isalpha(current_char)) {
     output_code(LETTERS[current_char - 'A']);
     delay(_dah_length);
   } else if (isdigit(current_char)) {
     output_code(DIGITS[current_char - '0']);
     delay(_dah_length);
   } else if (current_char == ' ') {
     Serial.print(" ");
     delay(_dit_length * 7);
   }
 }
 Serial.println();
}




Code: [Select]


//telegraph/telegraph.h

#ifndef __TELEGRAPH_H__    // this line is part of the "double-include" prevention mechanism - allowing TELEGRAPH_H to only be compiled ONCE.
#define __TELEGRAPH_H__    // this line defines the "body" of the header file as a preprocessor macro __TELEGRAPH_H__ ONLY if it has not been previously defined.

class Telegraph {          //create class Telegraph
 public:                  // This is the PUBLIC part of the class that users of the class have access to
   Telegraph(const int output_pin, const int dit_length);
   void send_message(const char* message);
   
 private:                 //This is the PRIVATE part of the class only members of the class can use
   void dit();            //declare function dit()
   void dah();            //declare function dah()
   void output_code(const char* code);      //declare function "output_code" with parameter
   void output_symbol(const int length);    //declare function "output_symbol" with parameter
   
   int _output_pin;      
   int _dit_length;
   int _dah_length;
   
};

#endif


   


Zoandar
  ---v/\/\/\v---

Coding Badly

Quote
I do know that for each change I make to the library files I have to re-copy them from the folder in the project where they get created and paste them into the Telegraph subfolder of the Libraries folder as the book instructed, and then restart the Arduino IDE, and re-upload the sketch to the board.


Which version of the IDE are you using?

PaulS

Quote
But when I tried removing "strlen(message)" and replacing it with the number 4 (to get it to loop 4 times) the result was still the same as above.

But, message is "SM", so strlen(message) returns 2. The loop you are modifying is looping through each letter in the message, defining a new "letter" to send to output_code.

The Telegraph class has a fundamental flaw. Class constructors are called before the init() method in the startup sequence. The init() method initializes all the pins to input. The constructor for the Telegraph class sets the specified pin to output before init() sets it input.

The Telegraph class needs a begin() method (like Serial.begin()) in which the hardware items are placed.

Code: [Select]
Telegraph::Telegraph(const int output_pin, const int dit_length)
{
  _output_pin = output_pin;
  _dit_length = dit_length;
  _dah_length = dit_length * 3;
}

void Telegraph::begin()
{
  pinMode(_output_pin, OUTPUT);
}


Since you have the serial port initialized, and are writing some data to the serial port, add
Code: [Select]
Serial.print("Pattern to output: ");
Serial.println(code);

to the top of output_code.

This will help determine if the problem is with the code to output a single "letter", as output_code is doing, or with send_message.

davekw7x

#3
Mar 13, 2011, 04:26 pm Last Edit: Mar 13, 2011, 06:31 pm by davekw7x Reason: 1

I have a project built from a book called Arduino: A Quick-Start Guide, which is a Morse Code generator...what I am getting is simply one "." followed by one "-", repeating every 5 seconds. It seems to be a problem with a loop of some sort.

No.  It's a problem with your dit and dah functionality.

I don't have that particular book, so I can't say whether it is correct, but here's the deal for Morse:  dits and dahs in a character are separated by delays of dit duration.

So:
For a "dit" The LED should go high for _dit_length, and low for _dit_length.
For a "dah" the LED should go high for _dah_length and low for _dit_length.

To implement this you can make a simple change in your class code:

Code: [Select]

void Telegraph::output_symbol(const int length) {
   digitalWrite(_output_pin, HIGH);
   delay(length);
   digitalWrite(_output_pin, LOW);
   delay(_dit_length); // davekw7x: This separates the dits and dahs within a character
}


If the class that you posted is directly and correctly copied from the book, then it is obvious that the code wasn't tested.  That's not a Good Thing.  (See Footnote.)

Regards,

Dave

Footnote:

Paul's advice about having an "empty" constructor and a begin() method for your classes is good, and it might save you some worries later.

However...

You are getting something out of the LED so it is obvious that Pin 13 is an output pin, and it is obvious that changing the constructor and implementing begin() wouldn't help this particular program.  I mean, it's not a bad idea to change the constructor and to implement the begin(), as Paul suggested, but this won't solve your particular problem.

The cause of your particular problem is that you don't separate the elements or your characters by  _dit_length.  (So all three dits of the 'S' are run together and all two dahs of the 'M' are run together.  You are seeing a long dit and a long dah for each "SM" message.

For a more interesting (and readable at a nice, sedate 12 WPM) message make the suggested change to output_symbol() and try something like the following in your sketch:
Code: [Select]

   telegraph.send_message("CQ CQ CQ  de W1AW");


Also, to get the timing correct consider the following:
Since there is a dit-length delay as part of each dit and dah, the 3-dit-length inter-character delay of the Morse specification is implemented by tacking on two additional dit-length delays (not three) after the character, and the 7-dit-length inter-word delay is implemented by tacking on six additional dit-length delays (not seven) after the word.


dafid


Even a pin set to INPUT can drive a LED a little, as the digitalWrite(pin, HIGH) enables the pull up resistor on the pin (20 - 50 k ohms); so the LED will see between 0.25 to 0.1 milliAmps of current. Enough to light-up a small LED.

However... it will be quite dim.

Go Up