A program disaster: becoming inevitable


17 posts
by ibanezmatt13 » Sat Dec 29, 2012 12:24 pm
Hi,

I am making a program that converts strings into morse code and then performs the morse code using one LED connected to the 11th GPIO pin of the Raspberry Pi.

import RPi.GPIO as GPIO
import time

GPIO.setmode(GPIO.BOARD)

GPIO.setup(11, GPIO.OUT)





string = raw_input("Enter ")

list_string = list(string)


last_element = len(list_string) -1


print list_string[0]

print list_string[last_element]

print list_string[0:last_element + 1]



def dot():

GPIO.output(11, True)
time.sleep(0.25)
GPIO.output(11, False)
time.sleep(0.25)

def dash():

GPIO.output(11, True)
time.sleep(0.5)
GPIO.output(11, False)
time.sleep(0.5)



def s():

counter = 1
while counter <= 3:
dot()
counter += 1

def o():

counter = 1
while counter <= 3:
dash()
counter += 1

def a():

counter = 1
while counter <= 2:
dot()
counter += 1
dash()
counter += 1

def b():

counter = 1
while counter <= 4:
dash()
counter += 1
dot()
counter += 1
dot()
counter += 1
dot()
counter += 1

def c():

counter = 1
while counter <= 4:
dash()
counter += 1
dot()
counter += 1
dot()
counter += 1
dot()
counter += 1










for letter in list_string[0]:

if letter == 's':
s()
if letter == 'o':
o()
if letter == 'a':
a()
if letter == 'b':
b()
if letter == 'c':
c()

for letter in list_string[1]:

if letter == 's':
s()
if letter == 'o':
o()
if letter == 'a':
a()
if letter == 'b':
b()
if letter == 'c':
c()

for letter in list_string[2]:

if letter == 's':
s()
if letter == 'o':
o()
if letter == 'a':
a()
if letter == 'b':
b()
if letter == 'c':
c()

for letter in list_string[3]:

if letter == 's':
s()
if letter == 'o':
o()
if letter == 'a':
a()
if letter == 'b':
b()
if letter == 'c':
c()

Now, I fully understand that this is definately the most time consuming method of creating this program, however, I have hit a big obstacle.

When the user is promted to enter a string, the user can enter a string of as many characters as they wish. The problem is, I am creating 'for loops' for all elements in turn, as can be seen in the sqaure brackets after the declaration of each for loop.

The fact is, I do not know how to do this differently. I need some how for the program to determine how many elements are in the list and then create the for loops accordingly, as opposed to me sitting here for years writing the infinite number of elements that the list could hold!

Any suggestions would be greatly appreciated.

Many thanks in advance
Ibanezmatt13
(An intermediate novice)
Posts: 128
Joined: Fri Dec 28, 2012 9:49 am
by ibanezmatt13 » Sat Dec 29, 2012 12:54 pm
I have actually just thought of something, but it may be completely wrong:

for letter in range(0, last_element):

if letter == 's':
s()
time.sleep(0.2)
if letter == 'o':
o()
time.sleep(0.2)
if letter == 'a':
a()
time.sleep(0.2)
if letter == 'b':
b()
time.sleep(0.2)
if letter == 'c':
c()
time.sleep(0.2)


I have substituted this for the numerous 'for loops.' Am I on the correct path with this? Having given the updated program a run, my LED does not light up. Any suggestions, I am really, really confused.

Ibanezmatt13
Posts: 128
Joined: Fri Dec 28, 2012 9:49 am
by -rst- » Sat Dec 29, 2012 1:25 pm
How to eat an elephant? ... Piece by piece ;)

I would strongly suggest splitting your 'problem' into smaller pieces and learn to solve them first - then put them together. Maybe you have done some of these already, but I would start like this (each step a running program at a time, only proceed when runs without errors for different inputs ...and you understand the code):

1. Write a simple program that reads in a string from the user and prints it out
2. Extend program to print out the length of the string
3. Extend program to print each character/letter of the string in a loop ('for i in range(0, length_of_string)')
4. Think up a generic way to convert a character into a morse code string = a function that takes a character as a parameter and prints out the morse string
*** first try could be just a huge 'if else' statement (try some five first letters to begin with)
*** lookup how to use dictionary objects
5. Modify your main function to call the function (in 4) for each letter in the input
6. Make that function to return the string (instead of printing out) and have your main function to print it
7. Add the dot() and dash() functions, but make them just print out the dot or dash - make your main func to call these for each character in the string returned from the func in 4
8. Write a simple program that does nothing but lights up the led
9. Extend the led program to light the led, wait for a moment and turn led off
10. Include the led code into the other program and have the dot() and dash() manage the led (instead of printing out)

Hope that helps to the correct direction...

Oh, and this is definitely worth going through: http://docs.python.org/2/tutorial/ (or http://docs.python.org/3/tutorial/ if you use Python 3)
http://raspberrycompote.blogspot.com/ - Low-level graphics and 'Coding Gold Dust'
Posts: 895
Joined: Thu Nov 01, 2012 12:12 pm
Location: Dublin, Ireland
by ibanezmatt13 » Sat Dec 29, 2012 2:49 pm
-rst- wrote:How to eat an elephant? ... Piece by piece ;)

I would strongly suggest splitting your 'problem' into smaller pieces and learn to solve them first - then put them together. Maybe you have done some of these already, but I would start like this (each step a running program at a time, only proceed when runs without errors for different inputs ...and you understand the code):

1. Write a simple program that reads in a string from the user and prints it out
2. Extend program to print out the length of the string
3. Extend program to print each character/letter of the string in a loop ('for i in range(0, length_of_string)')
4. Think up a generic way to convert a character into a morse code string = a function that takes a character as a parameter and prints out the morse string
*** first try could be just a huge 'if else' statement (try some five first letters to begin with)
*** lookup how to use dictionary objects
5. Modify your main function to call the function (in 4) for each letter in the input
6. Make that function to return the string (instead of printing out) and have your main function to print it
7. Add the dot() and dash() functions, but make them just print out the dot or dash - make your main func to call these for each character in the string returned from the func in 4
8. Write a simple program that does nothing but lights up the led
9. Extend the led program to light the led, wait for a moment and turn led off
10. Include the led code into the other program and have the dot() and dash() manage the led (instead of printing out)

Hope that helps to the correct direction...

Oh, and this is definitely worth going through: http://docs.python.org/2/tutorial/ (or http://docs.python.org/3/tutorial/ if you use Python 3)


Hi,

Many thanks for the quick reply.

I have followed what you said very carefully and I must say that have been of great help. I now have a fully functioning Morse Code program. However, there is one final issue which should be easy to correct.

Once the LED has flashed according to the inputted string, it then repeats the string a second time before ending the program. SO basically, I need to somehow break out of the for loop after it has finished Morse Coding the string.

Do you have any suggestions for breaking the loop after it has Morse Coded the string once? I'll copy my code below; please note that I have not finished incorporating all of the letters of the alphabet just yet.

Many thanks in advance
Ibanezmatt13



import RPi.GPIO as GPIO
import time

GPIO.setmode(GPIO.BOARD)

GPIO.setup(11, GPIO.OUT)





string = raw_input("Enter A String: ")

list_string = list(string)


last_element = len(list_string) -1



print list_string[0]

print list_string[last_element]

print list_string[0:last_element + 1]



def dot():

GPIO.output(11, True)
time.sleep(0.25)
GPIO.output(11, False)
time.sleep(0.25)


def dash():

GPIO.output(11, True)
time.sleep(0.5)
GPIO.output(11, False)
time.sleep(0.5)



def s():

counter = 1
while counter <= 3:
dot()
counter += 1

def o():

counter = 1
while counter <= 3:
dash()
counter += 1

def a():

counter = 1
while counter <= 2:
dot()
counter += 1
dash()
counter += 1

def b():

counter = 1
while counter <= 4:
dash()
counter += 1
dot()
counter += 1
dot()
counter += 1
dot()
counter += 1

def c():

counter = 1
while counter <= 4:
dash()
counter += 1
dot()
counter += 1
dot()
counter += 1
dot()
counter += 1

def d():

counter = 1
while counter <= 3:
dash()
counter += 1
dot()
counter += 1
dot()
counter += 1

def e():

counter = 1
while counter <= 1:
dot()
counter += 1

def f():

counter = 1
while counter <= 4:
dot()
counter += 1
dot()
counter += 1
dash()
counter += 1
dot()
counter += 1

def g():

counter = 1
while counter <= 3:
dash()
counter += 1
dash()
counter += 1
dot()
counter += 1

def h():

counter = 1
while counter <= 4:
dot()
counter += 1
dot()
counter += 1
dot()
counter += 1
dot()
counter += 1

def i():

counter = 1
while counter <= 2:
dot()
counter += 1
dot()
counter += 1

def j():

counter = 1
while counter <= 4:
dot()
counter += 1
dash()
counter += 1
dash()
counter += 1
dash()
counter += 1

def k():

counter = 1
while counter <= 3:
dash()
counter += 1
dot()
counter += 1
dash()
counter += 1

def l():

counter = 1
while counter <= 4:
dot()
counter += 1
dash()
counter += 1
dot()
counter += 1
dot()
counter += 1














for letter in range(0, last_element):

for letter in list_string:






if letter == 's':
s()

time.sleep(0.2)

if letter == 'o':
o()

time.sleep(0.2)

if letter == 'a':
a()

time.sleep(0.2)

if letter == 'b':
b()

time.sleep(0.2)

if letter == 'c':
c()

time.sleep(0.2)

if letter == 'd':
d()

time.sleep(0.2)

if letter == 'e':
e()

time.sleep(0.2)

if letter == 'f':
f()

time.sleep(0.2)

if letter == 'g':
g()

time.sleep(0.2)

if letter == 'h':
h()

time.sleep(0.2)

if letter == 'i':
i()

time.sleep(0.2)

if letter == 'j':
j()

time.sleep(0.2)

if letter == 'k':
k()

time.sleep(0.2)

if letter == 'l':
l()

time.sleep(0.2)
Posts: 128
Joined: Fri Dec 28, 2012 9:49 am
by Twinkletoes » Sat Dec 29, 2012 3:14 pm
This is a great chance to find out about automated testing.
I would solve this problem by splitting into two a different way:
1 - translate text to morse (output is "--.. .-.- -." where space is a pause)
2 - output a morse string by using dash() and dot() and pause() methods

Now you can test your translator by writing a function that translates a known string (e.g. "abc") and checks that the output is correct. You can do this by translating the short string by hand and using an if auto_translate("abc")="..--..." (fictional morse!). Then throw an error if they're not the same.

You've now separated a testable piece of code (the translator) from the untestable code (the flashing of the LED). This is the technique used to build reliable code. You can even write the tests first and then you can run them against your code as you write it so that you get confidence as you go along that it's all going to work.

Good luck!
Posts: 185
Joined: Fri May 25, 2012 9:44 pm
by davef21370 » Sat Dec 29, 2012 5:27 pm
Hi, I'm kinda busy in the kitchen while reading all this but unless I'm missing something there's no need for the counter in your functions ie.

def f():
counter = 1
while counter <= 4:
dot()
counter += 1
dot()
counter += 1
dash()
counter += 1
dot()
counter += 1

could be...

def f():
dot()
dot()
dash()
dot()

also, I think the def b() and def c() functions would give the same output, save you some debugging later ;)

Hope this helps.
Regards.
Dave.
Please feel free to tap into my abundant lack of knowledge.
User avatar
Posts: 426
Joined: Fri Sep 21, 2012 4:13 pm
Location: Up North
by Vassius » Sat Dec 29, 2012 8:54 pm
In addition to the feedback already given, I would recommend making a general output function, rather than one for each letter. It would make the code more compact and more easy to modify.

To do this, you need to separate your representation from your logic.

First, figure out a way to represent dots and dashes. Zeros for dots and ones for dashes is one way, the strings "dot" and "dash" is another.
Next, choose a representation for the morse code for a letter. The most obvious way is using a list of dots and dashes in whatever representation you chose for those.
Next, keep a dictionary where each letter is a key, and the value is the morse code representation of that letter.

When you have done this, you can construct a generic output function which takes a single letter as input, looks up the letter in the dictionary, iterates through the list of dots and dashes and do the proper output for each dot and dash.

Now you have a lot less code with a lot less "if" statements. More importantly, the code is much easier to modify. Just imagine if you'd change the name or signature of the dot() and dash() functions; in your current code, you call those functions many, many times and need to change it in all those places. With my proposed modification, you only call dot() in one place and dash() in one. Pretty neat, huh? ;)
Posts: 25
Joined: Sun Jun 03, 2012 7:56 pm
by Peter247 » Sat Dec 29, 2012 10:54 pm
I was going to say how I would do it , but Vassius just about hit my idea.
The idea of converting it into a number may not work , because if you convert into number.
Does 2 in binary = 1-0 or 0-1-0 or 0-0-1-0 . etc
I would keep it in string form eg "010" and store in a list array and use a little maths.
whatever = [ '101','001','110' ]
eg find the ascii code of each character take away 65 (A) or 97(a) and you have the number in the alphabet
a = 0 , b = 1 , c = 2 etc
Your_Morse_string = whatever[ ascii_string_char - 65 ].
http://www.peter224722.blogspot.co.uk/
Posts: 55
Joined: Wed Nov 21, 2012 12:29 am
by poglad » Sun Dec 30, 2012 5:31 pm
Yes, 0 and 1 alone won't work... if you ever want to display the morse messages as dots and dashes then you might as well store them as . and - so that you won't need a separate formatting function for display.

Anyway, what I was going to suggest was that a Dictionary would be an ideal way of storing and accessing the definitions. As its name suggests, you can simply look up the letter and get back the morse pattern.

Watching your code develop reminds me of when I learned to program on a ZX81 back in 1982... I can still remember the sudden flashes of insight when I realized that something I'd just learned/discovered meant I could now simplify some of the code. Wonderful!
User avatar
Posts: 102
Joined: Tue Jul 31, 2012 8:47 am
Location: Aberdeen, Scotland
by Vassius » Mon Dec 31, 2012 9:26 am
Ones and zeros work just fine, as long as you do it properly. I didn't mean the number (binary or decimal) 010 for dot-dash-dot, I meant the sequence or list [0, 1, 0].

Code: Select all
morse_code = [0, 1, 0]
for signal in morse_code:
    if signal == 0:
        print "dot" # or a dot() function which does the proper GPIO output
    elif signal == 1:
        print "dash" # or a dash() function which does the proper GPIO output


This would produce the output
Code: Select all
dot
dash
dot


Choosing a proper data representation is very important. It's certainly possible to write a functional program with poor representation, but you'll be writing more code, it will be less readable and harder to modify.
Posts: 25
Joined: Sun Jun 03, 2012 7:56 pm
by poglad » Mon Dec 31, 2012 1:15 pm
Vassius wrote:Ones and zeros work just fine, as long as you do it properly. I didn't mean the number (binary or decimal) 010 for dot-dash-dot, I meant the sequence or list [0, 1, 0].

Well exactly - ones and zeros alone won't work. Putting them in a list like that is one way of sorting it, of course. :)
User avatar
Posts: 102
Joined: Tue Jul 31, 2012 8:47 am
Location: Aberdeen, Scotland
by Vassius » Mon Dec 31, 2012 3:10 pm
poglad wrote:
Vassius wrote:Ones and zeros work just fine, as long as you do it properly. I didn't mean the number (binary or decimal) 010 for dot-dash-dot, I meant the sequence or list [0, 1, 0].

Well exactly - ones and zeros alone won't work. Putting them in a list like that is one way of sorting it, of course. :)


Which is exactly what I proposed in my first post. ;)
Posts: 25
Joined: Sun Jun 03, 2012 7:56 pm
by -rst- » Mon Dec 31, 2012 4:31 pm
Quickly looking at your code, you have two for loops going through the input:
Code: Select all
for letter in range(0, last_element):

for letter in list_string:

... one should be enough...

Also, there is no need to convert the input string to a list - you can very well index into the characters within the string (my step 3):
Code: Select all
a = "Hello, world!"
for i in range (0, len(a)):
    print a[i]


Definitely making one multi-purpose output function would make sense - seems you overlooked my step 4 - I was thinking of something like this (note, not optimal or full solution):
Code: Select all
def char_to_morse(achar):
    morse = ""
    if (achar == "A") or (achar == 'a'):
        morse = "."
    # rest of the letters here...
    if (achar == "P") or (achar == 'p'):
        morse = ".--."
    # and maybe some letters here...
    return morse


Does that make sense?

I would very much like to know what advantage would there be in converting the dot-dash strings ".--." into zero-one arrays? Yes, in a very low-level time-critical application it would be best to encode these as one/two byte variable length bit-fields or something, but come on guys, this is Python (I very much doubt that there is a meaningful difference between looping through a string char by char and looping through an integer array) and especially a beginner programmer (not to confuse with non-essentials). And the 'dot-dash string' representation is ready as-is for debugging...

Oh, yeah Matt(?): one very useful technique to learn: how to format your code snippets on this forum!!! :D Hint, look at the buttons on the top of the text-area you write into...
http://raspberrycompote.blogspot.com/ - Low-level graphics and 'Coding Gold Dust'
Posts: 895
Joined: Thu Nov 01, 2012 12:12 pm
Location: Dublin, Ireland
by Vassius » Mon Dec 31, 2012 7:17 pm
-rst- wrote:I would very much like to know what advantage would there be in converting the dot-dash strings ".--." into zero-one arrays? Yes, in a very low-level time-critical application it would be best to encode these as one/two byte variable length bit-fields or something, but come on guys, this is Python (I very much doubt that there is a meaningful difference between looping through a string char by char and looping through an integer array) and especially a beginner programmer (not to confuse with non-essentials). And the 'dot-dash string' representation is ready as-is for debugging...


There may not be any practical advantage in the proposed zero-one encoding; as you say, iterating over a string is just as easy as iterating a list.

But then again, I didn't recommend any particular representation. They were just examples to illustrate that there are different ways of representing the same thing and inspire a thought process. I would guess that this morse code project is more for the learning experience than for some practical application, and for that purpose I would recommend writing several versions with different data representations and different levels of abstraction.
Posts: 25
Joined: Sun Jun 03, 2012 7:56 pm
by fdion » Mon Dec 31, 2012 7:37 pm
See the other thread:

viewtopic.php?p=246341#p246341

Morse is a classic case for a dictionary. At the other end of the spectrum, on the other thread, I posted a dichotomic tree solution, with gpio.

Francois
Francois
http://raspberry-python.blogspot.com - @f_dion
User avatar
Posts: 305
Joined: Sun Sep 16, 2012 2:33 pm
Location: North Carolina, USA
by -rst- » Wed Jan 02, 2013 1:57 pm
Yes, your tree is definitely very elegant solution, but (as you say) a bit too complicated for a beginner or even for early intermediate level.

Clearly the dictionary ('map' in some other programming languages) is the one for this level - in my point #4 earlier, I too suggested this ...after taking a stab at the naive 'if then' (switch/case in other languages maybe) solution.
http://raspberrycompote.blogspot.com/ - Low-level graphics and 'Coding Gold Dust'
Posts: 895
Joined: Thu Nov 01, 2012 12:12 pm
Location: Dublin, Ireland
by RoliPi » Thu Jan 10, 2013 8:27 pm
Hi,

i don't know how you came along, but here's one solution:

Code: Select all
import RPi.GPIO as GPIO
import time

def morse(duration):       #The dash&dot are only different in the
   GPIO.output(11, True)   #duration, so it'll be the passed argument
   time.sleep(duration)         
   GPIO.output(11, False)   
   time.sleep(dot)         #Duration of the silence is equal to dot

def morse_it(text):
   for i in text:         #A string is iterable,
      for j in morsecodes[i]:   #as it is the morse-code itself
         if j=='.':      #if it's a dot,
            morse(dot)   #we pass 0.25 to the function
         else:         #if not,
            morse(dash)   #then the other one
#The for i takes each letter from the text as keys,
#morsecodes[i] finds the values to the keys,
#and for j gives directly the dot or dash signs, which are handled then


morsecodes={'A':'.-','B':'-...','C':'-.-.'} #and so on A-Z 0-9
#The dictionary is a data type in Python: key:value
#If you know the key, the value is returned, you don't have to search
dot=0.25   #You can specify here the duration of the dot
dash=3*dot   #The dash is 3-times longer than dot

text=input('Enter some text: ')   #Python3!
morse_it(text.upper())   #Pass the text to the function, uppercased,
                  #so that it matches the dictionary




I hope it's understandable with the remarks.
And i hope it works, i don't have leds to try it ;)
Posts: 18
Joined: Thu Jan 03, 2013 2:08 pm
Location: Hungary