Critique my code, please.

[tracecom]

I am planning to build a copper/zinc penny separator that uses eddy currents, and the first step is a timer that can measure the speed of a falling penny. I have breadboarded the timer in the attached schematic, and written the code below. It is working, but I am looking for improvement suggestions.

' File Name:    Optical Timer.pbp
' Author:       tracecom
' Notice:       Copyright(c) 2013 Trace Communications; All Rights Reserved
' Compiler:     PICBASIC PRO 3
' Assembler:    MPASM  
' Target PIC:   12F509
' Oscillator:   4MHz Internal
' Hardware:     CRH Solderless Breadboard
' Schematic:    PIC12F509 Optical Timer.dch
' Keywords:     SEROUT, LCD, Photo Transistor
' Date:         4/23/2013
' Version:      1.0
' Notes:        
  
Include "modedefs.bas" ' Include serial modes.
serout GPIO.0,T9600,["?BFF"] ' Set LCD backlight to maximum brightness.
PAUSE 200 ' Pause to allow LCD EEPROM to program.
SEROUT GPIO.0,T9600,["?G216"]' Configure the LCD geometry: 2x16.
PAUSE 200 ' Pause to allow LCD EEPROM to program.
LCD var GPIO.0  ' Declare a serial out pin.  
PIP var GPIO.5  ' Declare a input pin.  
RDY var GPIO.1  ' Delcare a ready pin for LED indicator.
CNT Var word    ' Declare CNT as a word variable to store count. 
mainloop:
    high RDY
    if PIP = 0 then increment
    if PIP = 1 then display
increment:
    CNT = CNT + 1
    pause 1   ' Pause 1 millisecond.
    goto mainloop
     
display:
    If CNT = 0 Then no_data ' If count is zero, goto no_data.
    serout LCD,T9600,["?l", "?m"] ' Clear screen + carriage return.
    Serout LCD,T9600,[#CNT," mSec"]' If count is not zero, display count
                                        ' + " mSec"
    low RDY
    pause 3000  ' Pause 3 seconds to allow time to read display.
    serout LCD, T9600,["?l"]
    CNT = 0     ' Reset count to zero.
    goto mainloop 
no_data:
    Serout LCD,T9600,["?m","No Data"]   ' Display "No Data" + carriage return.
    goto mainloop
End

Here's the way it is currently working.

As long as there is nothing blocking the light from white LED1 to Q1, the collector of Q2 is high, the red LED2 is off, and the PIC sends "No Data" to the LCD. The green LED3 is lit to indicate that the PIC is ready to read the time.

When the light is blocked from LED1 to Q1, the collector of Q2 goes low, which causes the counter to increment in 1 mSec steps, and the red LED2 is lit to indicate a measurement is being taken, and stored in the word variable CNT. The green LED3 is off. When the light block is removed, the count stops and the total is displayed on the LCD for 3 seconds. The shortest period of time that I have been able to record is 17 mSecs; I did that by sort of thumping a coin through the gap between LED1 and Q1.

Then, the cycle repeats.

I am just trying to learn PBP3 and I know my code is clumsy. Comments, criticisms, and corrections on the code and the schematic are welcome.

I noticed that there are some capitalization inconsistencies in the code that are not there in the IDE. Apparently, something about copy/paste changes some lower case letters to capitals.


Thank you.