"What''s a Microcontroller" Chapter 4 project 1
Archiver
Posts: 46,084
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]
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]