"What''s a Microcontroller" Chapter 4 project 1
Archiver
Posts: 46,084
Gary Sims:
Thanks for taking the time to look at my code and teach me from
experience. You have really opened my mind when it comes to the
possibilities of the stamp.
I was only in chapter 4 at the time of my first message and never
learned the case statements or gosub. Now that I learned this from
you I went back and cleaned up some of my earlier projects. I was
surprised that not only did it clean up my spaghetti code but it
also reduced the amount of eprom space I was using.
You mentioned an article by Jon Williams on tricks of style. Do you
know where I might find this article? I would like to read it.
Thanks again friend,
Lu Kwan
--- In basicstamps@yahoogroups.com, "Gary W. Sims" <simsgw@c...>
wrote:
> Lu Kwan:
>
> Some more suggestions on your code (which is very good for an
early program by the way).
>
> 1. Some tricks of style. These will help make it easier for you to
read your own code, especially when you come back to a program days
or weeks after you wrote it. Jon Williams has written an excellent
article on this, so I'll stick to some of the simplest issues.
First, use indentation. Instead of writing:
>
> DO
> DEBUG "Enter PULSOUT duration:", CR
> DEBUGIN DEC duration
> IF duration < 350 THEN
> DEBUG "Value of duration must be above 349", CR
> PAUSE 1000
> ENDIF
> IF duration > 1150 THEN
> DEBUG "Value of duration must be less then 1151",CR
> PAUSE 1000
> ENDIF
> LOOP UNTIL duration > 349 AND duration < 1151
>
> Write this instead:
>
> DO
> DEBUG "Enter PULSOUT duration:", CR
> DEBUGIN DEC duration
> IF duration < 350 THEN
> DEBUG "Value of duration must be above 349", CR
> PAUSE 1000
> ENDIF
> IF duration > 1150 THEN
> DEBUG "Value of duration must be less then 1151",CR
> PAUSE 1000
> ENDIF
> LOOP UNTIL duration > 349 AND duration < 1151
>
>
> Second, use parentheses liberally in expressions. This won't help
with the earliest version of PBasic as I remember, but in almost all
coding situations it is better to overuse parentheses than to use
too few. Thus we would say:
>
> LOOP UNTIL ((duration > 349) AND (duration < 1151))
>
> Finally, try to get used to case statements before you get into
really complex coding situations. They help enormously for clarity,
and to shorten these comments, I'll add another clarity change to
this final suggested form for that loop:
>
> DO
> DEBUG "Enter PULSOUT duration:", CR
> DEBUGIN DEC duration
> SELECT duration
> CASE <350
> DEBUG "Value of duration must be above 349", CR
> PAUSE 1000
> CASE >1150
> DEBUG "Value of duration must be less then 1151",CR
> PAUSE 1000
> CASE ELSE
> GOSUB MoveTheServo
> ENDSELECT
> LOOP UNTIL ((duration > 349) AND (duration < 1151))
>
>
> The final change was to put the servo code in a subroutine and
call that subroutine as a final case in the select. That change is
entirely a matter of personal style, because it is done to organize
the code in the same way I picture the process that is being coded.
My picture of this problem goes like this:
>
> <Get a user input to evaluate>
> <report errors of type 1>
> <report errors of type 2>
> <report errors of type ...>
> <do as the user asked>
> <repeat>
>
> You don't have to share my image -- there is always more than one
way to skin a cat -- but good code results from building a clear
image of the process, and then using care to make sure the code is
consistent with that image. We criticize "spaghetti" code, which I
won't get into here, but that word image is probably clear. In my
experience, spaghetti code arises from spaghetti thinking. And
that's the strongest value of that "meta code" I mentioned last
time. Force yourself to describe the process in meta-code to
straighten out your own thinking. Then you're ready to write the
actual code.
>
> Once again, this is not a criticism of your own program but long
term advice. You did a good job of structuring this program.
>
> 2. Locality. Try to avoid situations where a distant piece of code
will be affected by something you write here if that effect is not
obvious, intentional, and "intuitive" to use a catch-all word. I did
something in that loop re-write that is none of those three. You
tested the value of "duration" in three places. That was not what we
economical coding, but it was clear and intuitive, and it all
happened in the same short bit of code. You did it well. Then I
confused things with my editing.
>
> I left the same three tests of duration, but right in the middle I
added a jump to a subroutine that might well be hundreds of lines of
code away from the series of tests. Now suppose the subroutine
modifies the value of "duration". It might in some future version of
this program, and someone working only on the subroutine could
justly consider the variable to be their own to do with as they
wish. It is their "argument" we say, so it's not unreasonable for
them to play with it. We have a lot of techniques in other computer
languages intended to deal with exactly this problem. This example
just illustrates how it arises. Protecting ourselves in PBasic could
be done several ways (including building a different image of the
process than the one my mind holds), but to stay consistent with my
own image, I would write that command loop this way to avoid future
problems:
>
> GoodCmd=0
> DO
> DEBUG "Enter PULSOUT duration:", CR
> DEBUGIN DEC duration
> SELECT duration
> CASE <350
> DEBUG "Value of duration must be above 349", CR
> PAUSE 1000
> CASE >1150
> DEBUG "Value of duration must be less then 1151",CR
> PAUSE 1000
> CASE ELSE
> GoodCmd=1
> GOSUB MoveTheServo
> ENDSELECT
> LOOP UNTIL 1=GoodCmd
>
> And of course earlier in the code we'd have the declaration:
>
> GoodCmd var BIT
>
> Notice by the way that I wrote the equality test with the
constant "1" on the left instead of the right. This is a trick of
experienced coders you might as well learn. In some situations it is
possible to assign the value one to GoodCmd when you really meant to
test it for equality. If you always put the constant part on the
left when it's a comparison you have in mind, then the compiler will
tell you immediately if you've accidentally structured it as an
assignment. One important part of style is intentionally writing
things in ways that let the compiler syntax checking highlight
problems for you.
>
> 3. Handling the "kill" button. Your change works, but there's a
human interface issue to consider. A user might well consider
that "kill" should work at anytime. It has kind of a global feel,
doesn't it? Like pulling the plug, it would "feel" more natural if
it worked anytime we pressed the button. You put the whole process
into a global loop and tested for IN4 after each command was
completed.
>
> That works, but let's say the user enters an invalid duration and
gets fed up: "KILL!" he mutters while pressing the button. (Somehow
this image just screams out "male user" by the way<g>.) We will
never see the button press. We're not going to let him kill the
process until he gives us a valid duration and we move the servo in
response. It might not just be pique that caused the "kill" input.
Maybe the servoed device is entangled in something. Maybe the servo
is smoking. Maybe a lot of things that we leave to the user's
judgment.
>
> Moreover, he'll have to let go the kill button to enter a correct
duration and it will be as if it never was pressed. After the servo
is moved, we'll go back for another pulse count value and refuse to
see the kill button again. A user would have to be aware of this
coding issue and hold down the kill button as the servo moves, so
that we would see it depressed when we complete the servo sequence.
In general it is bad design to compel a user to know how the code is
structured to use the program. This is one aspect of what we
call "an intuitive interface" design.
>
> So for several reasons, it would be better if we could respond to
the "kill" button anytime it is pressed. Ah... now the rub. We can't
do that as easily as we would like. The simplest (that is, the
crudest) solution is to pepper the code with this statement:
>
> IF 1=IN4 THEN GOTO WaitForStartButton
>
>
> Besides being crude, this is poor coding. Many languages won't
even let us do it, and I'm not sure exactly what would happen under
PBasic. It looks plausible, but the fatal error is I just
used "GOTO" to reach a subroutine, instead of "GOSUB". Skipping
thirty pages of discussion, don't ever do that. So how about using:
>
> IF 1=IN4 THEN GOSUB WaitForStartButton
>
> Now I have a different problem. This code will wait alright, but
when it returns, it will return to whichever place spotted the kill
button being pressed. We can't predict where that was so we can't
restore the screen to the appropriate prompt. Are we in the middle
of getting a valid duration? Or was the user still entering the
pulse count? We can't say. This is a problem called "restoring the
state" and dealing with it is another significant part of coding. In
fact, it is an important part of language design and design of the
processors themselves.
>
> We aren't going to fix it with some pretty bit of PBasic code.
Even the polling feature you'll learn about later won't fix this
one. The problem is fundamental. We have a sequence of events that
must occur in a specific order (Get pulse count; get pulse duration;
move servo). This is one type of what we call "synchronous" events.
And our fundamental problem is that we've inserted an "asynchronous"
event: a button press. Well, not "we". The asynchronous event was
inserted by whoever wrote the requirements, whoever described this
experiment.
>
> I'm sure whoever that was didn't intend for you to worry about
solving these advanced issues at your stage. The simple answer is
the one you used, and since this is an experiment rather than a
polished product, you are the user. You will know that "kill" only
works at the moment the servo quits moving, and if you time it wrong
the experiment will serve to teach you about this aspect of coding.
It's a valuable lesson to learn about a future challenge even it's
beyond your skill set to solve it just now.
>
> How would I solve it? Personally, I'd use hardware. I'd make
that "kill" button reset the stamp. In more complex situations, I'd
use judgment tempered by the awareness of this problem developed
over a few decades. It can be anything from the simple choice of
making "kill" act as a reset right up to completely intractable.
>
> Gotta go suddenly. No time to re-read this carefully. I hope it's
clear enough.
>
> Gary
>
> [noparse][[/noparse]Non-text portions of this message have been removed]
Thanks for taking the time to look at my code and teach me from
experience. You have really opened my mind when it comes to the
possibilities of the stamp.
I was only in chapter 4 at the time of my first message and never
learned the case statements or gosub. Now that I learned this from
you I went back and cleaned up some of my earlier projects. I was
surprised that not only did it clean up my spaghetti code but it
also reduced the amount of eprom space I was using.
You mentioned an article by Jon Williams on tricks of style. Do you
know where I might find this article? I would like to read it.
Thanks again friend,
Lu Kwan
--- In basicstamps@yahoogroups.com, "Gary W. Sims" <simsgw@c...>
wrote:
> Lu Kwan:
>
> Some more suggestions on your code (which is very good for an
early program by the way).
>
> 1. Some tricks of style. These will help make it easier for you to
read your own code, especially when you come back to a program days
or weeks after you wrote it. Jon Williams has written an excellent
article on this, so I'll stick to some of the simplest issues.
First, use indentation. Instead of writing:
>
> DO
> DEBUG "Enter PULSOUT duration:", CR
> DEBUGIN DEC duration
> IF duration < 350 THEN
> DEBUG "Value of duration must be above 349", CR
> PAUSE 1000
> ENDIF
> IF duration > 1150 THEN
> DEBUG "Value of duration must be less then 1151",CR
> PAUSE 1000
> ENDIF
> LOOP UNTIL duration > 349 AND duration < 1151
>
> Write this instead:
>
> DO
> DEBUG "Enter PULSOUT duration:", CR
> DEBUGIN DEC duration
> IF duration < 350 THEN
> DEBUG "Value of duration must be above 349", CR
> PAUSE 1000
> ENDIF
> IF duration > 1150 THEN
> DEBUG "Value of duration must be less then 1151",CR
> PAUSE 1000
> ENDIF
> LOOP UNTIL duration > 349 AND duration < 1151
>
>
> Second, use parentheses liberally in expressions. This won't help
with the earliest version of PBasic as I remember, but in almost all
coding situations it is better to overuse parentheses than to use
too few. Thus we would say:
>
> LOOP UNTIL ((duration > 349) AND (duration < 1151))
>
> Finally, try to get used to case statements before you get into
really complex coding situations. They help enormously for clarity,
and to shorten these comments, I'll add another clarity change to
this final suggested form for that loop:
>
> DO
> DEBUG "Enter PULSOUT duration:", CR
> DEBUGIN DEC duration
> SELECT duration
> CASE <350
> DEBUG "Value of duration must be above 349", CR
> PAUSE 1000
> CASE >1150
> DEBUG "Value of duration must be less then 1151",CR
> PAUSE 1000
> CASE ELSE
> GOSUB MoveTheServo
> ENDSELECT
> LOOP UNTIL ((duration > 349) AND (duration < 1151))
>
>
> The final change was to put the servo code in a subroutine and
call that subroutine as a final case in the select. That change is
entirely a matter of personal style, because it is done to organize
the code in the same way I picture the process that is being coded.
My picture of this problem goes like this:
>
> <Get a user input to evaluate>
> <report errors of type 1>
> <report errors of type 2>
> <report errors of type ...>
> <do as the user asked>
> <repeat>
>
> You don't have to share my image -- there is always more than one
way to skin a cat -- but good code results from building a clear
image of the process, and then using care to make sure the code is
consistent with that image. We criticize "spaghetti" code, which I
won't get into here, but that word image is probably clear. In my
experience, spaghetti code arises from spaghetti thinking. And
that's the strongest value of that "meta code" I mentioned last
time. Force yourself to describe the process in meta-code to
straighten out your own thinking. Then you're ready to write the
actual code.
>
> Once again, this is not a criticism of your own program but long
term advice. You did a good job of structuring this program.
>
> 2. Locality. Try to avoid situations where a distant piece of code
will be affected by something you write here if that effect is not
obvious, intentional, and "intuitive" to use a catch-all word. I did
something in that loop re-write that is none of those three. You
tested the value of "duration" in three places. That was not what we
economical coding, but it was clear and intuitive, and it all
happened in the same short bit of code. You did it well. Then I
confused things with my editing.
>
> I left the same three tests of duration, but right in the middle I
added a jump to a subroutine that might well be hundreds of lines of
code away from the series of tests. Now suppose the subroutine
modifies the value of "duration". It might in some future version of
this program, and someone working only on the subroutine could
justly consider the variable to be their own to do with as they
wish. It is their "argument" we say, so it's not unreasonable for
them to play with it. We have a lot of techniques in other computer
languages intended to deal with exactly this problem. This example
just illustrates how it arises. Protecting ourselves in PBasic could
be done several ways (including building a different image of the
process than the one my mind holds), but to stay consistent with my
own image, I would write that command loop this way to avoid future
problems:
>
> GoodCmd=0
> DO
> DEBUG "Enter PULSOUT duration:", CR
> DEBUGIN DEC duration
> SELECT duration
> CASE <350
> DEBUG "Value of duration must be above 349", CR
> PAUSE 1000
> CASE >1150
> DEBUG "Value of duration must be less then 1151",CR
> PAUSE 1000
> CASE ELSE
> GoodCmd=1
> GOSUB MoveTheServo
> ENDSELECT
> LOOP UNTIL 1=GoodCmd
>
> And of course earlier in the code we'd have the declaration:
>
> GoodCmd var BIT
>
> Notice by the way that I wrote the equality test with the
constant "1" on the left instead of the right. This is a trick of
experienced coders you might as well learn. In some situations it is
possible to assign the value one to GoodCmd when you really meant to
test it for equality. If you always put the constant part on the
left when it's a comparison you have in mind, then the compiler will
tell you immediately if you've accidentally structured it as an
assignment. One important part of style is intentionally writing
things in ways that let the compiler syntax checking highlight
problems for you.
>
> 3. Handling the "kill" button. Your change works, but there's a
human interface issue to consider. A user might well consider
that "kill" should work at anytime. It has kind of a global feel,
doesn't it? Like pulling the plug, it would "feel" more natural if
it worked anytime we pressed the button. You put the whole process
into a global loop and tested for IN4 after each command was
completed.
>
> That works, but let's say the user enters an invalid duration and
gets fed up: "KILL!" he mutters while pressing the button. (Somehow
this image just screams out "male user" by the way<g>.) We will
never see the button press. We're not going to let him kill the
process until he gives us a valid duration and we move the servo in
response. It might not just be pique that caused the "kill" input.
Maybe the servoed device is entangled in something. Maybe the servo
is smoking. Maybe a lot of things that we leave to the user's
judgment.
>
> Moreover, he'll have to let go the kill button to enter a correct
duration and it will be as if it never was pressed. After the servo
is moved, we'll go back for another pulse count value and refuse to
see the kill button again. A user would have to be aware of this
coding issue and hold down the kill button as the servo moves, so
that we would see it depressed when we complete the servo sequence.
In general it is bad design to compel a user to know how the code is
structured to use the program. This is one aspect of what we
call "an intuitive interface" design.
>
> So for several reasons, it would be better if we could respond to
the "kill" button anytime it is pressed. Ah... now the rub. We can't
do that as easily as we would like. The simplest (that is, the
crudest) solution is to pepper the code with this statement:
>
> IF 1=IN4 THEN GOTO WaitForStartButton
>
>
> Besides being crude, this is poor coding. Many languages won't
even let us do it, and I'm not sure exactly what would happen under
PBasic. It looks plausible, but the fatal error is I just
used "GOTO" to reach a subroutine, instead of "GOSUB". Skipping
thirty pages of discussion, don't ever do that. So how about using:
>
> IF 1=IN4 THEN GOSUB WaitForStartButton
>
> Now I have a different problem. This code will wait alright, but
when it returns, it will return to whichever place spotted the kill
button being pressed. We can't predict where that was so we can't
restore the screen to the appropriate prompt. Are we in the middle
of getting a valid duration? Or was the user still entering the
pulse count? We can't say. This is a problem called "restoring the
state" and dealing with it is another significant part of coding. In
fact, it is an important part of language design and design of the
processors themselves.
>
> We aren't going to fix it with some pretty bit of PBasic code.
Even the polling feature you'll learn about later won't fix this
one. The problem is fundamental. We have a sequence of events that
must occur in a specific order (Get pulse count; get pulse duration;
move servo). This is one type of what we call "synchronous" events.
And our fundamental problem is that we've inserted an "asynchronous"
event: a button press. Well, not "we". The asynchronous event was
inserted by whoever wrote the requirements, whoever described this
experiment.
>
> I'm sure whoever that was didn't intend for you to worry about
solving these advanced issues at your stage. The simple answer is
the one you used, and since this is an experiment rather than a
polished product, you are the user. You will know that "kill" only
works at the moment the servo quits moving, and if you time it wrong
the experiment will serve to teach you about this aspect of coding.
It's a valuable lesson to learn about a future challenge even it's
beyond your skill set to solve it just now.
>
> How would I solve it? Personally, I'd use hardware. I'd make
that "kill" button reset the stamp. In more complex situations, I'd
use judgment tempered by the awareness of this problem developed
over a few decades. It can be anything from the simple choice of
making "kill" act as a reset right up to completely intractable.
>
> Gotta go suddenly. No time to re-read this carefully. I hope it's
clear enough.
>
> Gary
>
> [noparse][[/noparse]Non-text portions of this message have been removed]