Please check my code
NWCCTV
Posts: 3,629
After designing and building a 2 Pushbutton circuit on a PCB I wrote the attached code to control one or two HB-25 motor controllers/motors. This is a very simple program in that if the first button is pressed motor 1 goes forward and motor 2 goes in reverse. If the second button is pushed motor 1 goes in reverse and motor 2 goes forward. There is no ramping done. The code works fine and everything works as it is suppose to. I was just wondering if someone could take a look at it and let me know if it looks good or needs changing.
' {$STAMP BS2}
' {$PBASIC 2.5}
' -----[ Program Description ]---------------------------------------------
' This is a simple pushboutton test for one or two HB-25's. There are 2
' Pushbuttons connected to pins 3 and 4. HB-25 is connected to Pin 15. Pin
' 3 controls forward and pin 4 controls reverse. Future updates for various
' speeds planned.
' -----[ I/O Definitions ]----------------------------------
HB25 PIN 15 ' I/O Pin For HB-25
' -----[ Initialization ]--------------------------------------------------
DO : LOOP UNTIL HB25 = 1 ' Wait For HB-25 Power Up
LOW HB25 ' Make I/O Pin Output/Low
PAUSE 5 ' Wait For HB-25 To Initialize
PULSOUT HB25, 750 ' Stop Motor 1
PAUSE 1 ' 1 mS Delay
PULSOUT HB25, 750 ' Stop Motor 2 (If Connected)
' The Above 2 Lines May Be Removed
' if using only 1 motor
' -----[ Program Code ]-----------------------------------------
DO
' Start DO Loop
IF (IN3 = 1) THEN ' Checks if Pushbutton on IN3 is pressed,
' If Yes then send pulseout command to Pin
PULSOUT HB25, 500 ' 15 to rotate motor 1 forward.
PAUSE 1 ' 1 mS Delay For Motor 2 Pulse
PULSOUT HB25, 1000 ' send pulsout command to pin 15
' to rotate motor 2 reverse.
PAUSE 10 ' wait 10 ms
ELSE
' If no then
PULSOUT HB25, 750 ' send pulsout stop command equivelent
' to motor 1.
PAUSE 1 ' 1 mS Delay For Motor 2 Pulse
PULSOUT HB25, 750 ' send pulsout stop command equivelent
' to motor 2.
PAUSE 10 ' wait 10 ms
IF (IN4 = 1) THEN ' Checks if Pushbutton on IN4 is pressed,
' If Yes then send pulseout command to Pin
PULSOUT HB25, 1000 ' 15 to rotate motor 1 reverse.
PAUSE 1 ' 1 mS Delay For Motor 2 Pulse
PULSOUT HB25, 500 ' send pulseout command to Pin 15 to
' rotate motor 2 forward.
PAUSE 10 ' wait 10 ms
ELSE
' If no then
PULSOUT HB25, 750 ' send pulsout stop command equivelent
' to motor 1.
PAUSE 1 ' 1 mS Delay For Motor 2 Pulse
PULSOUT HB25, 750 ' send pulsout stop command equivelent
' to motor 2.
PAUSE 10 ' wait 10 ms
ENDIF ' Since there are 2 IF statements there
ENDIF ' needs TO be 2 ENNDIF's.
LOOP ' Continuous loop.
Comments
I don't program the BS2 much but I'd think you'd want the first "ENDIF" before the second "IF".
I also think there's probably a better way of doing this (as I think you're suggesting by asking about the program).
I think you'd want:
do if button A pressed forward 1 backward 2 elseif button B pressed backward 1 forward 2 else ' no button is pressed stop both endif loopHave the button you want to take precedence be the first condition test (if).
[FONT=courier new][SIZE=1]DO SELECT INA >> 2 ' looking at the state of p3 and p4 as one unit CASE 1 GOSUB forward CASE 2 GOSUB backward CASE 0 GOSUB stop ENDSELECT PAUSE 20 LOOP[/SIZE][/FONT]I actually forgot about SUB!!!! I know what to do for the STOP command but I am a bit lost on the above code.
Here's the same loop section with proper (IMO) indentation (since Spin has made me think all code should be similarly indented).
DO ' Start DO Loop [COLOR=#ff0000]IF [/COLOR](IN3 = 1) THEN ' Checks if Pushbutton on IN3 is pressed, ' If Yes then send pulseout command to Pin PULSOUT HB25, 500 ' 15 to rotate motor 1 forward. PAUSE 1 ' 1 mS Delay For Motor 2 Pulse PULSOUT HB25, 1000 ' send pulsout command to pin 15 ' to rotate motor 2 reverse. PAUSE 10 ' wait 10 ms [COLOR=#ff0000]ELSE [/COLOR] ' If no then PULSOUT HB25, 750 ' send pulsout stop command equivelent ' to motor 1. PAUSE 1 ' 1 mS Delay For Motor 2 Pulse PULSOUT HB25, 750 ' send pulsout stop command equivelent ' to motor 2. PAUSE 10 ' wait 10 ms [COLOR=#00ff00]IF[/COLOR] (IN4 = 1) THEN ' Checks if Pushbutton on IN4 is pressed, ' If Yes then send pulseout command to Pin PULSOUT HB25, 1000 ' 15 to rotate motor 1 reverse. PAUSE 1 ' 1 mS Delay For Motor 2 Pulse PULSOUT HB25, 500 ' send pulseout command to Pin 15 to ' rotate motor 2 forward. PAUSE 10 ' wait 10 ms [COLOR=#00ff00]ELSE [/COLOR] ' If no then PULSOUT HB25, 750 ' send pulsout stop command equivelent ' to motor 1. PAUSE 1 ' 1 mS Delay For Motor 2 Pulse PULSOUT HB25, 750 ' send pulsout stop command equivelent ' to motor 2. PAUSE 10 ' wait 10 ms [COLOR=#00ff00]ENDIF ' Closes second IF statement [/COLOR][COLOR=#ff0000]ENDIF [/COLOR][COLOR=#ff0000]' Closes first IF statement [/COLOR]LOOP ' Continuous loop.(The forum software keeps messing up the indentation. I think the above is pretty close to what I'm after.)
Seeing the code this way makes it clear that "IN4" is checked only if "IN3" doesn't equal 1. Before "IN4" is checked a stop command is sent to both motors and if "IN4" isn't equal to one a second stop command is issued. Since the first stop command is redundant, I think the code would be more efficient like this.
DO ' Start DO Loop IF (IN3 = 1) THEN ' Checks if Pushbutton on IN3 is pressed, ' If Yes then send pulseout command to Pin PULSOUT HB25, 500 ' 15 to rotate motor 1 forward. PAUSE 1 ' 1 mS Delay For Motor 2 Pulse PULSOUT HB25, 1000 ' send pulsout command to pin 15 ' to rotate motor 2 reverse. ELSEIF (IN4 = 1) THEN ' Checks if Pushbutton on IN4 is pressed, ' If Yes then send pulseout command to Pin PULSOUT HB25, 1000 ' 15 to rotate motor 1 reverse. PAUSE 1 ' 1 mS Delay For Motor 2 Pulse PULSOUT HB25, 500 ' send pulseout command to Pin 15 to ' rotate motor 2 forward. ELSE ' If no then PULSOUT HB25, 750 ' send pulsout stop command equivelent ' to motor 1. PAUSE 1 ' 1 mS Delay For Motor 2 Pulse PULSOUT HB25, 750 ' send pulsout stop command equivelent ' to motor 2. ENDIF ' needs TO be 2 ENNDIF's. PAUSE 17 ' since no matter what you want a pause only list it once ' use pause length to make entire loop take 20ms to complete LOOP ' Continuous loop.I think this is the way it should be indented, assuming I'm understanding the BS2's if, elseif, else, endif statements. And someone, please correct me if I have the indentation based on the flow of the program incorrect.
Of course I'm sure Tracy's code would be better but I do not know enough about the BS2's case statement to help.
SELECT INA >> 2 ' looking at the state of p3 and p4 as one unit
What that statement does is to grab the state of the input pins p0 to p4 all at once as one binary nibble,
INA -- captures the input binary value, 4 bits, for example % 0100
and then shifts that nibble two bits right to isolate the state of pins p3 and p4 where your buttons are attached.
INA >> 2 -- shifts that value 2 right, for example % 0100 becomes % 0001, p3 is pressed
That becomes the value used by the SELECT CASE command, and there are 4 possible states of those two buttons, 00,01,10 and 11.
SELECT INA >> 2 ' looking at the state of p3 and p4 as one unit CASE 1 ' 01 button state GOSUB forward CASE 2 ' 10 GOSUB backward CASE 0 ' 00 GOSUB stop ENDSELECTCASE 3, binary 11, is not included. But could be. Do you see what will happen if both are pressed at once?
The behavior is a bit different from your program, or Duane's modification. There is not precedence of one button over the other, because the state INA is read all at once at one point in time.