Go Down

Topic: Telegraph Project from Quick-start Guide runs incorrectly (Read 3376 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.

Zoandar


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?



It is version 0022. I'll try to remember to mention that next time. :)
Zoandar
  ---v/\/\/\v---

davekw7x

#6
Mar 13, 2011, 06:23 pm Last Edit: Mar 13, 2011, 06:54 pm by davekw7x Reason: 1

... the LED will see between 0.25 to 0.1 milliAmps of current. With Arduino Pin 13 set as an input...Enough to light-up a small LED...quite dim

Well, maybe I shouldn't have used the word "obvious" as much as I did.

However...
When I ran the sketch as posted, it was "obvious" to me that the LED was operating with normal intensity and was exhibiting the problem reported in the original post.

Furthermore...
Even a dim LED would have nothing to do with the Original Poster's complaint (and my observation) that the LED output was not flashing with correct cadence.

Furthermore...
The Arduino init() function does not (that's not) set the mode of pin 13 to anything.  The init() function sets up various conditions in some timer registers and A/D prescaler value and disconnects UART functionality from pins 0 and 1, but does not have anything to do with INPUT or OUTPUT modes of the I/O pins.
(Note that a hardware Reset causes I/O pins to be set in INPUT mode, but the Arduino Pin 13 direction is set to OUTPUT by the Telegraph constructor, when the Telegraph object is instantiated, and nothing in this sketch or this library or any startup code or anything else changes that.  Period.)

Furthermore...
I did test the change that I showed for the output_symbol() function before I posted my suggestion for a fix.  After making that change, the LED was blinking with normal brightness and with the desired cadence.

And, finally...
If you are really interested, you can do the following:

1.  Copy the following function to the sketch:
Code: [Select]

//
// From davekw7x: A way to query the mode of an Arduino I/O pin
//
// This is a simplified function to get mode of I/O pin.
//
// For a somewhat more robust one, see replies #5 and #8 at
//  http://arduino.cc/forum/index.php/topic,52806.0.html
//  
#include <pins_arduino.h>
int getPinMode(unsigned int p)
{
   // Convert designated Arduino pin to ATMega port and pin
   uint8_t pbit  = digitalPinToBitMask(p);
   uint8_t pport = digitalPinToPort(p);
   if (pport == NOT_A_PIN) {
       return -1;
   }

   // Read the ATmega DDR for this port
   volatile uint8_t *pmodereg = portModeRegister(pport);

   // Test the value of the bit for this pin and return
   // 0 if it is reset and 1 if it is set
   return ((*pmodereg & pbit) != 0);
}


2.  Change the setup() function in the sketch so that it looks like this:
Code: [Select]

#include "telegraph.h"

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

Telegraph telegraph(OUTPUT_PIN, DIT_LENGTH);

void setup()
{
   Serial.begin(9600);    
   //
   // davekw7x: Find out whether the LED pin is an Input or an Output
   //
   int mode = getPinMode(OUTPUT_PIN);
   Serial.print("Pin ");
   Serial.print(OUTPUT_PIN);
   Serial.print(" is operating in ");
   if (mode == INPUT) {
       Serial.print(" IN");
   }
   else {
       Serial.print("OUT");
   }
   Serial.println("PUT mode.");
}


Results:

Pin 13 is operating in OUTPUT mode.




Regards,

Dave

dafid


Coding Badly

Quote
It is version 0022


Then you do not need to go through all those steps.  Modify the library.  Save it.  Verify or Upload.  That's it.  No need to restart or any of the other stuff.

Zoandar


Quote
It is version 0022


Then you do not need to go through all those steps.  Modify the library.  Save it.  Verify or Upload.  That's it.  No need to restart or any of the other stuff.



Which steps, exactly, are you referring to that I don't need? I can tell you with certainty that I definitely do need to copy the changed library code (from the Telegraph sketch with its 3 tabs) over to the Library subfolder of my installation. The reason I know is because at first I forgot to do that, and even though I had thought I 'fixed' the causes of the compiler errors I was getting during verification, the errors kept showing up until I finally remembered I had to copy the files over, then restart Arduino, and the errors stopped.

I suppose, when I get good enough at this to be able to catch errors without the IDE's help, I could simply edit the library files with a text editor in situ, and then not need to remember to copy them over. But I like the IDE's assistance in showing colors, following braces, and allowing me to verify the code. :)
Zoandar
  ---v/\/\/\v---

Zoandar

Quote

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
}


Quote


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




That fixed it!! Thanks!! It is hard to make something work when the only instructions provided are broken. Apparently the author never had anyone test all the projects right before he sent this manuscript to the publisher. He may have had such a line in his original draft and it may have gotten missed when being transcribed. The sad thing about that is when you print a book, and the information in it is wrong, it stays wrong forever. :)

I have examined the page of the book where that piece of the sketch originates, and I did copy it correctly (that line you added was not there).

Now that the sketch and its libraries are working, I plan to study all the code and try to add as many comment lines as I can, describing what each line of code does throughout the entire sketch and its library files. Just as writing about something helps many people sort things out in their mind, adding the comments in my own words has the same effect. The author does discuss "some" of them as he is laying down the code, but there are things I wish he would have talked more about.

I'll keep in mind what you said in your footnote, as well as the other information presented here by others.

Thanks, all of you, for the help. :)

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

Zoandar

Now for the next question. This sketch mentions writing to the Serial Port. Does that mean I am supposed to be seeing something on the Serial Monitor? Because if I am, there is nothing there.

Thanks.
Zoandar
  ---v/\/\/\v---

davekw7x

#12
Mar 13, 2011, 10:00 pm Last Edit: Mar 13, 2011, 10:04 pm by davekw7x Reason: 1

...That fixed it!!
Huzzah!!!


Quote from: Zoandar

...when you print a book, and the information in it is wrong, it stays wrong forever.
I poked around a little and found that the book has a web site (http://pragprog.com/titles/msard/arduino), and even an errata page (http://pragprog.com/titles/msard/errata)
You can make your own contribution to the community by posting a correction (or corrections) there.  (Since I haven't bought the book, I wouldn't dream of taking it on myself to post there.)


Regards,

Dave

Footnote:
Taking something that works (or that "almost works") and analyzing it step-by-step may be a good intermediate learning process.  At least it is for some people.

After you have satisfied yourself that it works as it should, if you are still interested, you can try adding simple punctuation that real live CW operators use: '.'  ',' '?'  '/'   I wouldn't bother with other stuff like parentheses and exclamation marks, but I would use '=' for for the "double-dash"  ('-...-') that ops use as a paragraph marker (or generally to kill time without having dead air while they are thinking of what they are going to say next).

How about some prosigns?  I might use '$' as a character to tell the program to send "SK"  (Remember, "SK" is not two separate characters but '...-.-' all together as one character).  Define your own input character to send "AR" ('.-.-.') and "KN" ('-.--.').  Stuff like that.

Want to try something neat?  Send "best bent wire" as a message.  It's especially cool if you have a tone generator(or use a speaker and the Arduino tones library) so that you can actually hear the boogie beat.  I am told that WWII radio ops used that as a kind of radio check.  (That's a little before my time.)


Zoandar

Thanks, Dave!

I'll look into the website and errata page. Maybe I'll find the solutions to all the stuff I have been complaining about.  :smiley-mr-green:

BTW, when I tried your modifications posted above, to get the Serial Monitor to show "Pin 13 is operating in OUTPUT mode.", I also started getting the actual Morse Code being displayed as it gets sent. This is what I had thought the sketch was supposed to do, but that part wasn't working either (unless I misunderstood the author's intent) until I added your code. I will keep ' your ' version to use, since I like the added features. :)

I should tell you that I know pretty much nothing at all about Morse Code, but I do have a "passing" interest in it. Many years ago I had a co-worker who had a technician's class ham license offer to teach it to me so I could get into novice class ham radio if I wanted to. At that time I had other hobbies and didn't have the time or the funding to get into yet another hobby, especially one that involved. But I do think ham radio is cool and am always glad to meet someone who is into it. Not only can it be a lot of fun, I also realize that in times of crisis ham operators are usually VERY helpful in getting communications going when traditional methods fail, such as is probably happening even now in Japan with the terrible disaster they are enduring. So my hat is off to you folks! I have listened to some of the transmitted code many years ago on a very ancient Hallicrafters S40A receiver I have, which I bought from my neighbor when I was a kid in the 60s. He had been in the Merchant Marines, and actually met his eventual wife in Holland. So the radio has been around the world, I am sure. It still works (mostly - a few features never seemed to work right) as far as I know, but these days I rarely power it up. I was always fascinated by that Morse Code and a few times I used an encyclopedia we had to try to look up what it was the operators were sending to each other. But back then I was pre-teen and to even dream of getting into something like that would have been like wanting to be an astronaut as far as I was concerned. :)

That said, this project was just one in the book as I work my way through it, and didn't hold any special interest for me. But I was wondering why there were no punctuation symbols in the array, and I have seen in the past where some ham operators I had met who were using Morse Code have their own "lingo" using abbreviations, much like "IM Speak" is done in text today. So thanks for the added information, and again thanks for helping me get this working!

Take Care,

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

Coding Badly

Quote
Which steps, exactly, are you referring to that I don't need?


After rereading your posts I now understand (sometimes I'm a bit slow).  You have the library source code in the same directory as your Sketch.  After changing something in the Sketch-folder-copy-of-the-library, you copy the source files to the libraries directory and then Verify or Upload.  I thought you were going through several more steps each time you uploaded.

Quote
But I like the IDE's assistance in showing colors, following braces, and allowing me to verify the code.


I use Visual Studio, "Programmer's Notepad", or "Notepad++" to edit library source files.

Go Up