Code check and refinement suggestions?

[kevlar129bp]

Hello all,

Can you fine folks peruse my code to see if it's viable, and possibly offer suggestions for refinement.

Thanks a ton,
Chris

'*****************************************************************************************
'*  Name    : SIMPLE TACH.BAS                                                            *
'*  Author  : Kevlar                                                                     *
'*  Notice  : Copyright (c) 2012 Lightspeed Engineering                                  *
'*          : All Rights Reserved                                                        *
'*  Date    : 12/9/2012                                                                  *
'*  Version : 1.0                                                                        *
'*  Notes   : PIC16F1936 (on-chip vref)                                                  *
'*          :                                                                            *
'*****************************************************************************************
@ ERRORLEVEL -230
@ ERRORLEVEL -306   ; turn off crossing page boundary message
@ ERRORLEVEL -303   ; suppress Program word too large 

'                               PIC16F1936
'                               -----U-----
'   MCLR                E.3     |1      28|     B.7      PULSE INPUT
'   OUT 2               A.0     |2      27|     B.6      LCD RS     
'   OUT 1               A.1     |3      26|     B.5      LCD E
'   VREF-               A.2     |4      25|     B.4      LCD DB4
'   150 VDC             A.3/AN3 |5      24|     B.3      LCD DB5
'   OUT 3               A.4     |6      23|     B.2      LCD DB6
'   50 AMPS             A.5/AN4 |7      22|     B.1      LCD DB7
'   VSS                         |8      21|     B.0/AN12 LM34 TEMP (N/A in this design)
'   OSC 1               A.7     |9      20|              VDD     
'   OSC 2               A.6     |10     19|              VSS     
'   Heartbeat LED       C.0     |11     18|     C.7      232 Rx
'   GPIO 1              C.1     |12     17|     C.6      232 Tx
'   GPIO 2              C.2     |13     16|     C.5      Tx LED
'   GPIO 3, 1 WIRE IO   C.3     |14     15|     C.4      Rx LED
'                               -----------


'*********************************INCLUDES************************************************
INCLUDE "DT_INTS-14.bas"        ' Base Interrupt System
INCLUDE "ReEnterPBP.bas"        ' Include if using PBP interrupts
'*****************************************************************************************

'*********************************INTERRUPTS**********************************************
ASM
INT_LIST  macro    ; IntSource,        Label,  Type, ResetFlag?
    INT_Handler    IOC_INT,     _PulseINT,   PBP,  yes
    INT_Handler   TMR1_INT,     _Overflow,   PBP,  yes    
    INT_Handler     RX_INT,      _RS232in,   PBP,  yes
    endm
    INT_CREATE               ; Creates the interrupt processor
ENDASM
'*****************************************************************************************

'**********************************ENABLE INTS********************************************
@ INT_ENABLE  IOC_INT             ; enable Port B Change interrupts
@ INT_ENABLE  TMR1_INT            ; enable Timer 1 interrupts
@ INT_ENABLE  RX_INT              ; enable 232 Rx interrupts
'*****************************************************************************************

'*********************************OSCILLATOR**********************************************
DEFINE OSC 20                   ' Change these to match your Hardware   
'*****************************************************************************************

'*********************************TIMER1**************************************************
Preload  var word        
preload  = 3036      '(Preload for 100mS Interrupts)
TimerOn  con 49
TimerOff con 48
T1CON    = TimerOff  '7=GateInv,6=GateEn,5-4=Prescale(8),3=LPosc,2=T1Sync,1=ClkSrc,0=TmrON
TMR1H    = preload.highbyte'preload.HIGHBYTE 
TMR1L    = Preload.lowbyte 'preload.LOWBYTE
'*****************************************************************************************

'*********************************USART***************************************************
DEFINE HSER_RCSTA 90h           ' Enable serial port & continuous receive
DEFINE HSER_TXSTA 24h           ' Enable transmit, BRGH = 1
DEFINE HSER_CLROERR 1           ' Clear overflow automatically
DEFINE HSER_SPBRG 86            ' 57600 Baud @ 20MHz, -0.22%
SPBRGH = 0
BAUDCON.3 = 1                   ' Enable 16 bit baudrate generator                   
'*****************************************************************************************

'*********************************LCD*****************************************************
DEFINE  LCD4X20  1
LCD_DB4 VAR PORTB.4 
LCD_DB5 VAR PORTB.3 
LCD_DB6 VAR PORTB.2 
LCD_DB7 VAR PORTB.1 
LCD_RS VAR PORTB.6 
LCD_E VAR PORTB.5 
LCD_Lines CON 2                 ' use "2" for 4 lines) 
LCD_DATAUS CON 50 
LCD_COMMANDUS CON 2000 
INCLUDE "LCD_AnyPin.pbp"        ' Will NOT compile without a "LCDOUT" command
Line1 con 2                     'LCD Line 1
Line2 con 192                   'LCD Line 2
Line3 con 148                   'LCD Line 3
Line4 con 212                   'LCD Line 4
'*****************************************************************************************

'*********************************PINS & ALIASES******************************************
HrtbtLED    var PORTC.0         'Active LOW
Rx232       var PORTC.7
Tx232       var PORTC.6         
INVOLT      VAR PORTA.3
INTEMP      VAR PORTB.0
INAMPS      VAR PORTA.5
OUT1        VAR PORTA.1         'Active HIGH         
OUT2        VAR PORTA.0         'Active HIGH
OUT3        VAR PORTA.4         'Active HIGH
INDEX       VAR PORTB.7
GPIO1       VAR PORTC.1
GPIO2       VAR PORTC.2
GPIO3       VAR PORTC.3
RxLED       var PORTC.4         'Active LOW
TxLED       var PORTC.5         'Active LOW
'*****************************************************************************************

'*********************************REGISTERS***********************************************
TRISA  = %11101000      '7=Osc1,6=Osc2,5=AN4(AMPS),4=OUT3,3=AN2(VOLTS),2=VREF-,1=OUT1,0=OUT2
TRISB  = %10000001      '7=INDEX INPUT,6=LCDrs,5=LCDe,4=LCDd4,3=LCDd5,2=LCDd6,1=LCDd7,0=AN12(TEMP)
TRISC  = %10000000      '7=Rx232,6=Tx232,5=TxLED,4=RxLED,3=GPIO3,2=GPIO2,1=GPIO1,0=HrtbtLED
ANSELA = %00011000      '7=N/A,6=N/A,5=AN5,4=AN4(AMPS),3=AN3(VOLTS),2=AN2,1=AN1,0=AN0
ANSELB = %00000001      '7=N/A,6=N/A,5=AN13,4=AN11,3=AN9,2=AN8,1=AN10,0=AN12(TEMP)
IOCBP  = %10000000      '7=PULSE INPUT
FVRCON = %11000010      '7=FVR On,6=FVR Ready,5=Temp En,4=Temp Range,3-2=Comp/DAC On,1-0=ADC Ref (2.048)
ADCON1 = %10100011      '7=Right Justify,6-4=Fosc,3=N/A,2=ADC Ref-,1-0=ADC Ref+
DEFINE ADC_BITS 10      'ADCIN resolution  (Bits)
DEFINE ADC_CLOCK 2      'ADC clock source  (Fosc/32)
DEFINE ADC_SAMPLEUS 11  'ADC sampling time (uSec)
'*****************************************************************************************

'*********************************VARIABLES***********************************************
wsave   VAR BYTE    $70     SYSTEM      ' alternate save location for W 
' --- IF any of these three lines cause an error ?? ------------------------
'       Comment them out to fix the problem ----
' -- Which variables are needed, depends on the Chip you are using -- 
'wsave1  VAR BYTE    $A0     SYSTEM      ' location for W if in bank1
'wsave2  VAR BYTE    $120    SYSTEM      ' location for W if in bank2
wsave3  VAR BYTE    $1A0    SYSTEM      ' location for W if in bank3

PulseCNT  VAR WORD          'Pulse Count per 6 Timer1 Interrupts
LoopCNT     con 5           'Count for ADC Averaging AND TIMER1 Overflows
wTemp       var word        'Universal WORD Sized Variable
Busy        VAR BIT         'One Wire Bus Busy Flag
Volts       var word        'Actual Voltage
VHolder     Var WORD
VCurrentCNT var byte
QVolts      con 1466        '150/1023=.1466, Ex: 750(ADVAL)*1466=1099500, Volts=DIV32 1000 (1099)
Amps        var word
AHolder     Var WORD
ACurrentCNT var byte
QAmps       con 489         '50/1023=.0489, Ex: 512(ADVAL)*489=250368, Amps=DIV32 1000 (250)
TempC       var word
TempF       var word
R_Temp      VAR	WORD
TempDigits  VAR BYTE
Dummy       VAR BYTE
RPM var word
BufferJunk  VAR BYTE
Address VAR byte                        ' byte var for module address
SerialIn var byte
CmdAddress var byte                     ' commanded address
CmdPrefix var byte
CmdOutput var byte                      ' commanded output
Command var byte                        ' will hold "Q" or "O"
EEPromLoc VAR byte
TripVal Var byte
ReReadVals Var bit
OvflReadNow con 6
OvflCNT var byte

TripVoltsLow1 var byte      'EEProm Location 10
TripVoltsHigh1 var byte     'EEProm Location 11
TripTempLow1 var byte       'EEProm Location 12
TripTempHigh1 var byte      'EEProm Location 13
TripAmpsLow1 var byte       'EEProm Location 14
TripAmpsHigh1 var byte      'EEProm Location 15

TripVoltsLow2 var byte      'EEProm Location 20
TripVoltsHigh2 var byte     'EEProm Location 21
TripTempLow2 var byte       'EEProm Location 22
TripTempHigh2 var byte      'EEProm Location 23
TripAmpsLow2 var byte       'EEProm Location 24
TripAmpsHigh2 var byte      'EEProm Location 25

TripVoltsLow3 var byte      'EEProm Location 30
TripVoltsHigh3 var byte     'EEProm Location 31
TripTempLow3 var byte       'EEProm Location 32
TripTempHigh3 var byte      'EEProm Location 33
TripAmpsLow3 var byte       'EEProm Location 34
TripAmpsHigh3 var byte      'EEProm Location 35

'*****************************************************************************************

'**********************************EEPROM VALUES******************************************
EEPROM 10,[0]
EEPROM 12,[0]
EEPROM 14,[0]
EEPROM 20,[0]
EEPROM 22,[0]
EEPROM 24,[0]
EEPROM 30,[0]
EEPROM 32,[0]
EEPROM 34,[0]
'*****************************************************************************************

'*********************************Initialize Variables************************************
PulseCNT        = 0
VCurrentCNT     = 0
VHolder         = 0
ACurrentCNT     = 0
AHolder         = 0
OvflCNT         = 0
'*****************************************************************************************

'*********************************Code Goes Here First************************************
ReadTripVals:
    read 10,TripVoltsLow1   'Default will be 0
    read 11,TripVoltsHIGH1  'Default will be 255
    read 12,TripTempLow1    'Default will be 0
    read 13,TripTempHigh1   'Default will be 255
    read 14,TripAmpsLow1    'Default will be 0
    read 15,TripAmpsHigh1   'Default will be 255
    read 20,TripVoltsLow2   'Default will be 0
    read 21,TripVoltsHIGH2  'Default will be 255
    read 22,TripTempLow2    'Default will be 0
    read 23,TripTempHigh2   'Default will be 255
    read 24,TripAmpsLow2    'Default will be 0
    read 25,TripAmpsHigh2   'Default will be 255
    read 30,TripVoltsLow3   'Default will be 0
    read 31,TripVoltsHigh3  'Default will be 255
    read 32,TripTempLow3    'Default will be 0
    read 33,TripTempHigh3   'Default will be 255
    read 34,TripAmpsLow3    'Default will be 0
    read 35,TripAmpsHigh3   'Default will be 255
    ReReadVals = 0

T1CON   = timeron           'Timer ON

Main:    

    if ReReadVals = 1 then      'If ANY Trip Values were changed,
        goto ReadTripVals       'reload ALL Trip Values
    endif

    GOSUB GetVoltage
    GOSUB GetTemp
    GOSUB GetCurrent
        
    '----------------------------VOLTAGE-------------------------------    
    if (volts/10 < TripVoltsLow1) OR  (volts/10 < TripVoltsLow2) OR (volts/10 < TripVoltsLow3) then
        HIGH OUT1   'ON
    ELSE
        LOW OUT1    'OFF
    ENDIF
    IF (VOLTS/10 > TRIPVOLTSHIGH1) OR (VOLTS/10 > TRIPVOLTSHIGH2) OR (VOLTS/10 > TRIPVOLTSHIGH3) THEN
        HIGH OUT1   'ON
    ELSE
        LOW OUT1    'OFF
    ENDIF
    
    '----------------------------TEMPERATURE---------------------------    
    if (tempf/100 < triptemplow1) OR (tempf/100 < triptemplow2) OR (tempf/100 < triptemplow3) then
        HIGH out2
    ELSE
        LOW OUT2
    endif
    if (tempf/100 > triptempHigh1) OR (tempf/100 > triptemphigh2) OR (tempf/100 > triptemphigh3) then
        high out2
    ELSE
        LOW OUT2
    endif

    '----------------------------AMPERAGE------------------------------    
   if (amps/10 < tripampslow1) OR (amps/10 < tripampslow2) OR (amps/10 < tripampslow3) then
       HIGH out3
   ELSE
        LOW OUT3
   endif
   if (amps/10 > tripampshigh1) OR (amps/10 > tripampshigh2) OR (amps/10 > tripampshigh3) then
       High out3
   ELSE
        LOW OUT3
   endif

    LCDout $fe, 1   ' XXXXXXXXXXXXXXXXXXXX  <-- 20 DIGITS
        
        'Voltage Display
        If Volts = 0 then
            Lcdout $fe, LINE1, "VOLTAGE:       0.0 V"
        endif
        if (Volts > 0) AND (Volts < 10) then
            Lcdout $fe, LINE1, "VOLTAGE:       0.",DEC Volts," V"
        endif
        if (Volts > 9) AND (Volts < 100) then
            Lcdout $fe, LINE1, "VOLTAGE:       ",DEC Volts DIG 1,".",DEC Volts DIG 0," V"
        endif
        if (Volts > 99) AND (Volts < 1000) then
            Lcdout $fe, LINE1, "VOLTAGE:      ",DEC Volts DIG 2,DEC Volts DIG 1,".",DEC Volts DIG 0," V"
        endif
        if Volts > 1000 then
            Lcdout $fe, LINE1, "VOLTAGE:     ",DEC Volts DIG 3,DEC Volts DIG 2,DEC Volts DIG 1,".",DEC Volts DIG 0," V"
        endif
        
        'Temperature Display
        If TempF < 10000 then
            Lcdout $fe, LINE2, "TEMP   :     ",DEC TempF DIG 3,DEC TempF DIG 2,".",DEC2 TempF," F"
        endif
        If TempF > 9999 then
            Lcdout $fe, LINE2, "TEMP   :    ",DEC TempF DIG 4,DEC TempF DIG 3,DEC TempF DIG 2,".",DEC2 TempF," F"
        endif
    
        'Current Display
        If Amps = 0 then
            Lcdout $fe, LINE3, "CURRENT:       0.0 A"
        endif
        if (Amps > 0) AND (Amps < 10) then
            Lcdout $fe, LINE3, "CURRENT:       0.",DEC Amps," A"
        endif
        if (Amps > 9) AND (Amps < 100) then
            Lcdout $fe, LINE3, "CURRENT:       ",DEC Amps DIG 1,".",DEC Amps DIG 0," A"
        endif
        if (Volts > 99) AND (Volts < 1000) then
            Lcdout $fe, LINE3, "CURRENT:      ",DEC Amps DIG 2,DEC Amps DIG 1,".",DEC Amps DIG 0," A"
        endif

        'RPM Display
        
        If RPM = 0 then
            Lcdout $fe, LINE4, "RPM    :         0 R"
        endif
        if (RPM > 0) AND (RPM < 10) then
            Lcdout $fe, LINE4, "RPM    :         ",DEC RPM," R"
        endif
        if (RPM > 9) AND (RPM < 100) then
            Lcdout $fe, LINE4, "RPM    :        ",DEC2 RPM," R"
        endif
        if (RPM > 99) AND (RPM < 1000) then
            Lcdout $fe, LINE4, "RPM    :       ",DEC3 RPM," R"
        endif
        if RPM > 1000 then
            Lcdout $fe, LINE4, "RPM    :      ",DEC4 RPM," R"
        endif

goto main
'*****************************************************************************************

'************************************VOLTAGE SUB******************************************
GetVoltage:
    'VOLTAGE TO BE FED THROUGH A 73.25:1 SCALER
    '150vdc / 73.25 = 2.0477vdc
    VCurrentCNT = VCurrentCNT + 1
    if VCurrentCNT > LoopCNT then
        Volts = VHolder / LoopCNT   '5453 / 5 = 1090
        VCurrentCNT = 0
        VHolder = 0
    else
        ADCON0 = %10001111      '7=N/A,6-2=Channel,1=Go/Done,0=Enable
        WHILE ADCON0.1
        WEND
   	    wTemp.HighByte = ADRESH
   	    wTemp.LowByte = ADRESL
   	    Dummy = wTemp * QVolts
   	    Volts = DIV32 1000          '1099   1095    1088    1075    1096
   	    VHolder = VHolder + Volts   '1099   2194    3282    4375    5453            
    endif
RETURN
'*****************************************************************************************

'************************************TEMPERATURE SUB**************************************
GetTemp:
    OWOUT GPIO3, 1, [$CC, $44]                          'Skip ROM search & do temp conversion
    Wait_Up:
        OWIN GPIO3, 4, [Busy]                           'Read busy-bit
        IF Busy = 0 THEN Wait_Up                        'Still busy..?, Wait_Up..!
    OWOUT GPIO3, 1, [$CC, $BE]                          'Skip ROM search & read scratchpad memory
    OWIN GPIO3, 2, [R_Temp.Lowbyte, R_Temp.Highbyte]    'Read two bytes / end comms
    Dummy = 1125 * R_Temp                               '
    TempF = DIV32 100
    TempF = TempF + 3200
RETURN
'*****************************************************************************************

'************************************CURRENT SUB******************************************
GetCurrent:
    'CURRENT SHUNT = 75mV @ 50A
    'VOLTAGE TO BE AMPLIFIED WITH A MAX4460, GAIN SET AT ~27.3
    '~27.3 * .075 = 2.0475
    ACurrentCNT = ACurrentCNT + 1
    if ACurrentCNT > LoopCNT then
        Amps = AHolder / LoopCNT
        ACurrentCNT = 0
        AHolder = 0
    else
        ADCON0 = %10010011      '7=N/A,6-2=Channel,1=Go/Done,0=Enable
        WHILE ADCON0.1
        WEND
   	    wTemp.HighByte = ADRESH
   	    wTemp.LowByte = ADRESL
   	    Dummy = wTemp * QAmps
   	    Amps = DIV32 1000
   	    AHolder = AHolder + Amps
    endif
RETURN
'*****************************************************************************************

'************************************PULSE INTERRUPT**************************************
PulseINT:
    PulseCNT = PulseCNT + 1
RETURN
'*****************************************************************************************

'************************************TIMER INTERRUPT**************************************
Overflow:
@ INT_DISABLE IOC_INT
        T1CON   = timeroff
        OvflCNT = OvflCNT + 1
        if OvflCNT = OvflReadNow then                       '1 PulseCNT in .6 Seconds
            'Read PulseCNT                                  'f (RPS) = 1 / t
            'Calculate RPM                                  'f (RPS) = 1 / .6
            RPM = PulseCNT * 100                            'f (RPS) = 1.666666
            'Reset OvflCNT to 0                             'RPM = RPS * 60
            OvflCNT = 0                                     'RPM = 100                         
            'Reset PulseCNT to 0                          
            PulseCNT = 0
            toggle HrtbtLED                                 'On Off every 600 mS                                              
        endif        
        TMR1H = preload.highbyte
        TMR1L = Preload.lowbyte
        T1CON = timeron
@ INT_ENABLE IOC_INT
RETURN
'*****************************************************************************************

'***********************************SERIAL INTERRUPT**************************************
RS232in:
@ INT_DISABLE IOC_INT
@ INT_DISABLE TMR1_INT
@ INT_DISABLE RX_INT
    low RxLED   'Rx LED ON
    HSERIN [dec3 SerialIn]
    If SerialIn = 255 then gosub SetupModule
    while PIR1.5
        SerialIn = RCREG
    wend    
    high RxLED  'Rx LED OFF
@ INT_ENABLE IOC_INT
@ INT_ENABLE TMR1_INT
@ INT_ENABLE RX_INT
RETURN
'*****************************************************************************************

SetupModule:

HSERIN [CmdPrefix,dec3 CmdAddress]
    
    select case CmdPrefix
        '-------------------------Address Module----------------------------
        case "A"                    
            Address = CmdAddress
            write 0,Address
            LOW TxLED
                HSEROUT ["ACK:A",DEC3 Address,13,10]            
            HIGH TxLED
            read 0,Address
        '------------------------------------------------------------------
        '-------------------------Command Module----------------------------            
        case "B"                    
            '--------------------Address Mismatch--------------------------
            if CmdAddress <> Address then
                goto Junk
            endif
            '--------------------------------------------------------------
            hserin[command]     'O, Q, or M
            select case command
                case "O"    '(OUTPUT)
                    HSERIN[dec CmdOutput]   ' "CmdOutput" is holding 1,2 or 3
                    select case CmdOutput
                        case 1
                            toggle OUT1
                            pause 1500
                            toggle OUT1
                        case 2
                            toggle OUT2
                            pause 1500 
                            toggle OUT2
                        case 3
                            toggle OUT3
                            pause 1500 
                            toggle OUT3
                        CASE ELSE
                            goto Junk            
                        END SELECT
                Case "Q"                                                '(QUERY)    '$BxxxQ
                    low TxLED                                           'Tx LED ON
                    HSEROUT ["ACK:B",DEC3 Address,":",13,10,_           'Send Address First
                             "VOLTS: ",DEC VOLTS,13,10,_                'VOLTS: 10500
                             "TEMP: ",DEC TempF,13,10,_                 'TEMP: 102.4
                             "CURRENT: ",DEC Amps,13,10]                'AMPS: 2350
                            TRISA  = %11111011                          'A.4=OUT3,A.1=OUT1,A.0=OUT2
                            HSEROUT ["OUTPUT 1: ",dec PORTA.1,13,10,_   'OUTPUT 1: 0
                                     "OUTPUT 2: ",DEC PORTA.0,13,10,_   'OUTPUT 2: 1
                                     "OUTPUT 3: ",DEC PORTA.4,13,10]    'OUTPUT 3: 1
                            TRISA  = %11101000                          'A.4=OUT3,A.1=OUT1,A.0=OUT2
                    high TxLED                                          'Tx LED OFF
                Case "M"                                                '$B001 "M" 11025511255
                    'Output 1
                    '10=LowVolts, 11=HighVolts, 12=LowTemp, 13=HighTemp, 14=LowCurrent, 15=HighCurrent
                    'Output 2
                    '20=LowVolts, 21=HighVolts, 22=LowTemp, 23=HighTemp, 24=LowCurrent, 25=HighCurrent
                    'Output 3
                    '30=LowVolts, 31=HighVolts, 32=LowTemp, 33=HighTemp, 34=LowCurrent, 35=HighCurrent
                    'Output 4
                    '40=LowVolts, 41=HighVolts, 42=LowTemp, 43=HighTemp, 44=LowCurrent, 45=HighCurrent
                    '255B001                  Board Address
                    '       M                 Map
                    '        10               EEProm Location (Low Trip)
                    '          255            Low Trip Value   
                    '             11          EEProm Location (High Trip)
                    '               255       High Trip Value
                    HSERIN[dec2 eepromloc]
                    HSERIN[dec3 tripval]
                    write eepromloc,tripval
                    HSERIN[dec2 eepromloc]
                    HSERIN[dec3 tripval]
                    write eepromloc,tripval
                    ReReadVals = 1
                case else
                    goto junk
                end select
        end select
Junk:
return

[Normnet]

Chris

An new feature in the upcoming update of FineLineIDE "Unused" finds:
Label Ln 405: PulseINT:
Label Ln 411: Overflow:
Label Ln 433: RS232in:


Norm

[wdmagic]

Your useing a 1936 chip, really too old. How about upgrading it to a 2013 one!

[HenrikOlsson]

Hi Chris,
That's quite a bit of code to go thru but one thing I noticed is code like this (which you have 3 sets of):

    '----------------------------VOLTAGE-------------------------------    
    if (volts/10 < TripVoltsLow1) OR  (volts/10 < TripVoltsLow2) OR (volts/10 < TripVoltsLow3) then
        HIGH OUT1   'ON
    ELSE
        LOW OUT1    'OFF
    ENDIF
    IF (VOLTS/10 > TRIPVOLTSHIGH1) OR (VOLTS/10 > TRIPVOLTSHIGH2) OR (VOLTS/10 > TRIPVOLTSHIGH3) THEN
        HIGH OUT1   'ON
    ELSE
        LOW OUT1    'OFF
    ENDIF

Here you're making the calculation volts/10 no less than 6 times even though the result will be the same each time. It would be more efficient from a speed and program memory point to do the calculation one time and using the result of that calculation.

More, HIGH/LOW are convenient commands but if have your TRIS-registers set correctly (and I think you have) you might as well do OUT1 = 1 or OUT1 = 0 etc, that's also more efficient both in execution time and program space.

The display routine (or part of it):

    LCDout $fe, 1   ' XXXXXXXXXXXXXXXXXXXX  <-- 20 DIGITS
        
        'Voltage Display
        If Volts = 0 then
            Lcdout $fe, LINE1, "VOLTAGE:       0.0 V"
        endif
        if (Volts > 0) AND (Volts < 10) then
            Lcdout $fe, LINE1, "VOLTAGE:       0.",DEC Volts," V"
        endif
        if (Volts > 9) AND (Volts < 100) then
            Lcdout $fe, LINE1, "VOLTAGE:       ",DEC Volts DIG 1,".",DEC Volts DIG 0," V"
        endif
        if (Volts > 99) AND (Volts < 1000) then
            Lcdout $fe, LINE1, "VOLTAGE:      ",DEC Volts DIG 2,DEC Volts DIG 1,".",DEC Volts DIG 0," V"
        endif
        if Volts > 1000 then
            Lcdout $fe, LINE1, "VOLTAGE:     ",DEC Volts DIG 3,DEC Volts DIG 2,DEC Volts DIG 1,".",DEC Volts DIG 0," V"
        endif

Here you are storing the string VOLTAGE: in program memory 5 times. It would be more effecient to first print out VOLTAGE: and then, depending on the value of Volts, print the rest. I also think that it can be optimzed quite a bit by using the modulo operator ( // ) instead of printing the individual digits like that but I'm not sure if you're achieving anything special by doing it the way you are. If Volts are in units of 0.1 volts (ie 1234 is 123.4V) then you could simply do

LCDOUT $FE, 1
LCDOUT $FE, LINE1, "Voltage: ", DEC3 Volts/10, ".", DEC Volts//10

Well, that's a couple of ideas you might look into.

/Henrik.

[kevlar129bp]

Thank you a million Henrik,

I can see all of your points. I'll work on those suggestions and further refine from there. BTW, I could not make heads or tails from the first two responses to this thread...I read them six times each and still felt like I had just entered the twilight zone ...but I sincerely appreciate your post and guidance.

Thanks again,
Chris

[HenrikOlsson]

Hi Chris,
You're welcome.
I think the other Chris (wdmagic) is just trying to be funny and I believe what Norm is saying is that you have a couple of labels which you aren't GOTO'ing or GOSUB'ing from anywhere in the code.

However, FineLine (which is an IDE that Norm is developing) apparently isn't picking up on the fact that they ARE defined as interrupt handlers using DT-Ints and therefor ARE being "used".

Even if they weren't "used" it wouldn't really matter - you can have a label at each and every line in the code if you wanted to, it won't take up any more space and it won't run any slower.

I can see the unused feature in FineLine being good for finding variables which are declared but not used though.

Keep it up!

/Henrik.

[richard]

I think al the isr's need to end with @ INT_RETURN not return

[HenrikOlsson]

Definitely so, good catch!

[richard]

I think the ioc_int isr may need to reset its flag manually too , dt-ints can need some manual intervention for clearing ioc interrupt flags for some chips ,just ticking yes to clr the flag may not work . pays to read the data sheet

[Demon]

I try to keep defines, registers, wsave and other critical stuff at the top.

Robert

[kevlar129bp]

Thanks to all you guys,

Fixed the Interrupt Returns, working on the LCD display routines and redundant math. Awesome suggestions!

On the topic of the LCD routines, is this what you meant Henrik:
Lcdout $fe, LINE1 + 15, DEC Volts/10,".",DEC Volts//10," V"
That's for volts that are below 10...

As far as post 2, I don't use Fineline so I wasn't aware of that caveat. Maybe I should check it out
As far as post 3, I double checked the datasheet and I didn't see anywhere that it was originally designed for the W.O.P.R. computer, so I figured I was good

Thanks again all,

Chris

[kevlar129bp]

Wow,

With the refinements so far, I went from 4367 words used to 3153 words used! Happy, Happy, Happy

Please let me know if I can put the code on a diet any further...

Thanks guys

Chris

'*****************************************************************************************
'*  Name    : SIMPLE TACH.BAS                                                            *
'*  Author  : Kevlar                                                                     *
'*  Notice  : Copyright (c) 2012 Lightspeed Engineering                                  *
'*          : All Rights Reserved                                                        *
'*  Date    : 12/9/2012                                                                  *
'*  Version : 1.0                                                                        *
'*  Notes   : PIC16F1936 (on-chip vref)                                                  *
'*          :                                                                            *
'*****************************************************************************************
@ ERRORLEVEL -230
@ ERRORLEVEL -306   ; turn off crossing page boundary message
@ ERRORLEVEL -303   ; suppress Program word too large 

'                               PIC16F1936
'                               -----U-----
'   MCLR                E.3     |1      28|     B.7      PULSE INPUT
'   OUT 2               A.0     |2      27|     B.6      LCD RS     
'   OUT 1               A.1     |3      26|     B.5      LCD E
'   VREF-               A.2     |4      25|     B.4      LCD DB4
'   150 VDC             A.3/AN3 |5      24|     B.3      LCD DB5
'   OUT 3               A.4     |6      23|     B.2      LCD DB6
'   50 AMPS             A.5/AN4 |7      22|     B.1      LCD DB7
'   VSS                         |8      21|     B.0/AN12 LM34 TEMP (N/A in this design)
'   OSC 1               A.7     |9      20|              VDD     
'   OSC 2               A.6     |10     19|              VSS     
'   Heartbeat LED       C.0     |11     18|     C.7      232 Rx
'   GPIO 1              C.1     |12     17|     C.6      232 Tx
'   GPIO 2              C.2     |13     16|     C.5      Tx LED
'   GPIO 3, 1 WIRE IO   C.3     |14     15|     C.4      Rx LED
'                               -----------


'*********************************INCLUDES************************************************
INCLUDE "DT_INTS-14.bas"        ' Base Interrupt System
INCLUDE "ReEnterPBP.bas"        ' Include if using PBP interrupts
'*****************************************************************************************

'*********************************INTERRUPTS**********************************************
ASM
INT_LIST  macro    ; IntSource,        Label,  Type, ResetFlag?
    INT_Handler    IOC_INT,     _PulseINT,   PBP,  yes
    INT_Handler   TMR1_INT,     _Overflow,   PBP,  yes    
    INT_Handler     RX_INT,      _RS232in,   PBP,  yes
    endm
    INT_CREATE               ; Creates the interrupt processor
ENDASM
'*****************************************************************************************

'**********************************ENABLE INTS********************************************
@ INT_ENABLE  IOC_INT             ; enable Port B Change interrupts
@ INT_ENABLE  TMR1_INT            ; enable Timer 1 interrupts
@ INT_ENABLE  RX_INT              ; enable 232 Rx interrupts
'*****************************************************************************************

'*********************************OSCILLATOR**********************************************
DEFINE OSC 20                   ' Change these to match your Hardware   
'*****************************************************************************************

'*********************************TIMER1**************************************************
Preload  var word        
preload  = 3036      '(Preload for 100mS Interrupts)
TimerOn  con 49
TimerOff con 48
T1CON    = TimerOff  '7=GateInv,6=GateEn,5-4=Prescale(8),3=LPosc,2=T1Sync,1=ClkSrc,0=TmrON
TMR1H    = preload.highbyte'preload.HIGHBYTE 
TMR1L    = Preload.lowbyte 'preload.LOWBYTE
'*****************************************************************************************

'*********************************USART***************************************************
DEFINE HSER_RCSTA 90h           ' Enable serial port & continuous receive
DEFINE HSER_TXSTA 24h           ' Enable transmit, BRGH = 1
DEFINE HSER_CLROERR 1           ' Clear overflow automatically
DEFINE HSER_SPBRG 86            ' 57600 Baud @ 20MHz, -0.22%
SPBRGH = 0
BAUDCON.3 = 1                   ' Enable 16 bit baudrate generator                   
'*****************************************************************************************

'*********************************LCD*****************************************************
DEFINE  LCD4X20  1
LCD_DB4 VAR PORTB.4 
LCD_DB5 VAR PORTB.3 
LCD_DB6 VAR PORTB.2 
LCD_DB7 VAR PORTB.1 
LCD_RS VAR PORTB.6 
LCD_E VAR PORTB.5 
LCD_Lines CON 2                 ' use "2" for 4 lines) 
LCD_DATAUS CON 50 
LCD_COMMANDUS CON 2000 
INCLUDE "LCD_AnyPin.pbp"        ' Will NOT compile without a "LCDOUT" command
Line1 con 2                     'LCD Line 1
Line2 con 192                   'LCD Line 2
Line3 con 148                   'LCD Line 3
Line4 con 212                   'LCD Line 4
'*****************************************************************************************

'*********************************PINS & ALIASES******************************************
HrtbtLED    var PORTC.0         'Active LOW
Rx232       var PORTC.7
Tx232       var PORTC.6         
INVOLT      VAR PORTA.3
INTEMP      VAR PORTB.0
INAMPS      VAR PORTA.5
OUT1        VAR PORTA.1         'Active HIGH         
OUT2        VAR PORTA.0         'Active HIGH
OUT3        VAR PORTA.4         'Active HIGH
INDEX       VAR PORTB.7
GPIO1       VAR PORTC.1
GPIO2       VAR PORTC.2
GPIO3       VAR PORTC.3
RxLED       var PORTC.4         'Active LOW
TxLED       var PORTC.5         'Active LOW
'*****************************************************************************************

'*********************************REGISTERS***********************************************
TRISA  = %11101000      '7=Osc1,6=Osc2,5=AN4(AMPS),4=OUT3,3=AN2(VOLTS),2=VREF-,1=OUT1,0=OUT2
TRISB  = %10000001      '7=INDEX INPUT,6=LCDrs,5=LCDe,4=LCDd4,3=LCDd5,2=LCDd6,1=LCDd7,0=AN12(TEMP)
TRISC  = %10000000      '7=Rx232,6=Tx232,5=TxLED,4=RxLED,3=GPIO3,2=GPIO2,1=GPIO1,0=HrtbtLED
ANSELA = %00011000      '7=N/A,6=N/A,5=AN5,4=AN4(AMPS),3=AN3(VOLTS),2=AN2,1=AN1,0=AN0
ANSELB = %00000001      '7=N/A,6=N/A,5=AN13,4=AN11,3=AN9,2=AN8,1=AN10,0=AN12(TEMP)
IOCBP  = %10000000      '7=PULSE INPUT
FVRCON = %11000010      '7=FVR On,6=FVR Ready,5=Temp En,4=Temp Range,3-2=Comp/DAC On,1-0=ADC Ref (2.048)
ADCON1 = %10100011      '7=Right Justify,6-4=Fosc,3=N/A,2=ADC Ref-,1-0=ADC Ref+
DEFINE ADC_BITS 10      'ADCIN resolution  (Bits)
DEFINE ADC_CLOCK 2      'ADC clock source  (Fosc/32)
DEFINE ADC_SAMPLEUS 11  'ADC sampling time (uSec)
'*****************************************************************************************

'*********************************VARIABLES***********************************************
wsave   VAR BYTE    $70     SYSTEM      ' alternate save location for W 
' --- IF any of these three lines cause an error ?? ------------------------
'       Comment them out to fix the problem ----
' -- Which variables are needed, depends on the Chip you are using -- 
'wsave1  VAR BYTE    $A0     SYSTEM      ' location for W if in bank1
'wsave2  VAR BYTE    $120    SYSTEM      ' location for W if in bank2
wsave3  VAR BYTE    $1A0    SYSTEM      ' location for W if in bank3

PulseCNT  VAR WORD          'Pulse Count per 6 Timer1 Interrupts
LoopCNT     con 5           'Count for ADC Averaging AND TIMER1 Overflows
wTemp       var word        'Universal WORD Sized Variable
Busy        VAR BIT         'One Wire Bus Busy Flag
Volts       var word        'Actual Voltage
CompareVolts var byte
VHolder     Var WORD
VCurrentCNT var byte
QVolts      con 1466        '150/1023=.1466, Ex: 750(ADVAL)*1466=1099500, Volts=DIV32 1000 (1099)
Amps        var word
CompareAmps var byte
AHolder     Var WORD
ACurrentCNT var byte
QAmps       con 489         '50/1023=.0489, Ex: 512(ADVAL)*489=250368, Amps=DIV32 1000 (250)
TempC       var word
TempF       var word
CompareTemp var byte
R_Temp      VAR	WORD
TempDigits  VAR BYTE
Dummy       VAR BYTE
RPM var word
BufferJunk  VAR BYTE
Address VAR byte                        ' byte var for module address
SerialIn var byte
CmdAddress var byte                     ' commanded address
CmdPrefix var byte
CmdOutput var byte                      ' commanded output
Command var byte                        ' will hold "Q" or "O"
EEPromLoc VAR byte
TripVal Var byte
ReReadVals Var bit
OvflReadNow con 6
OvflCNT var byte

TripVoltsLow1 var byte      'EEProm Location 10
TripVoltsHigh1 var byte     'EEProm Location 11
TripTempLow1 var byte       'EEProm Location 12
TripTempHigh1 var byte      'EEProm Location 13
TripAmpsLow1 var byte       'EEProm Location 14
TripAmpsHigh1 var byte      'EEProm Location 15

TripVoltsLow2 var byte      'EEProm Location 20
TripVoltsHigh2 var byte     'EEProm Location 21
TripTempLow2 var byte       'EEProm Location 22
TripTempHigh2 var byte      'EEProm Location 23
TripAmpsLow2 var byte       'EEProm Location 24
TripAmpsHigh2 var byte      'EEProm Location 25

TripVoltsLow3 var byte      'EEProm Location 30
TripVoltsHigh3 var byte     'EEProm Location 31
TripTempLow3 var byte       'EEProm Location 32
TripTempHigh3 var byte      'EEProm Location 33
TripAmpsLow3 var byte       'EEProm Location 34
TripAmpsHigh3 var byte      'EEProm Location 35

'*****************************************************************************************

'**********************************EEPROM VALUES******************************************
EEPROM 10,[0]
EEPROM 12,[0]
EEPROM 14,[0]
EEPROM 20,[0]
EEPROM 22,[0]
EEPROM 24,[0]
EEPROM 30,[0]
EEPROM 32,[0]
EEPROM 34,[0]
'*****************************************************************************************

'*********************************Initialize Variables************************************
PulseCNT        = 0
VCurrentCNT     = 0
VHolder         = 0
ACurrentCNT     = 0
AHolder         = 0
OvflCNT         = 0
'*****************************************************************************************

'*********************************Code Goes Here First************************************

'Assign LCD static chars
LCDout $fe, 1      'XXXXXXXXXXXXXXXXXXXX  <-- 20 DIGITS
Lcdout $fe, LINE1, "VOLTAGE:"
Lcdout $fe, LINE2, "TEMP   :"
Lcdout $fe, LINE3, "CURRENT:"
Lcdout $fe, LINE4, "RPM    :"

ReadTripVals:
    read 10,TripVoltsLow1   'Default will be 0
    read 11,TripVoltsHIGH1  'Default will be 255
    read 12,TripTempLow1    'Default will be 0
    read 13,TripTempHigh1   'Default will be 255
    read 14,TripAmpsLow1    'Default will be 0
    read 15,TripAmpsHigh1   'Default will be 255
    read 20,TripVoltsLow2   'Default will be 0
    read 21,TripVoltsHIGH2  'Default will be 255
    read 22,TripTempLow2    'Default will be 0
    read 23,TripTempHigh2   'Default will be 255
    read 24,TripAmpsLow2    'Default will be 0
    read 25,TripAmpsHigh2   'Default will be 255
    read 30,TripVoltsLow3   'Default will be 0
    read 31,TripVoltsHigh3  'Default will be 255
    read 32,TripTempLow3    'Default will be 0
    read 33,TripTempHigh3   'Default will be 255
    read 34,TripAmpsLow3    'Default will be 0
    read 35,TripAmpsHigh3   'Default will be 255
    ReReadVals = 0

T1CON   = timeron           'Timer ON

Main:    

    if ReReadVals = 1 then      'If ANY Trip Values were changed,
        T1CON   = timeroff
        goto ReadTripVals       'reload ALL Trip Values
    endif

    GOSUB GetVoltage
    GOSUB GetTemp
    GOSUB GetCurrent
        
    '----------------------------VOLTAGE-------------------------------    
    if (CompareVolts < TripVoltsLow1) OR  (CompareVolts < TripVoltsLow2) OR (CompareVolts < TripVoltsLow3) then
        OUT1 = 1    'ON
    ELSE
        OUT1 = 0    'OFF
    ENDIF
    IF (CompareVolts > TRIPVOLTSHIGH1) OR (CompareVolts > TRIPVOLTSHIGH2) OR (CompareVolts > TRIPVOLTSHIGH3) THEN
        OUT1 = 1    'ON
    ELSE
        OUT1 = 0    'OFF
    ENDIF
    
    '----------------------------TEMPERATURE---------------------------    
    if (CompareTemp < triptemplow1) OR (CompareTemp < triptemplow2) OR (CompareTemp < triptemplow3) then
        OUT2 = 1
    ELSE
        OUT2 = 0
    endif
    if (CompareTemp > triptempHigh1) OR (CompareTemp > triptemphigh2) OR (CompareTemp > triptemphigh3) then
        OUT2 = 1
    ELSE
        OUT2 = 0
    endif

    '----------------------------AMPERAGE------------------------------    
   if (CompareAmps < tripampslow1) OR (CompareAmps < tripampslow2) OR (CompareAmps < tripampslow3) then
        OUT3 = 1
   ELSE
        OUT3 = 0
   endif
   if (CompareAmps > tripampshigh1) OR (CompareAmps > tripampshigh2) OR (CompareAmps > tripampshigh3) then
        OUT3 = 1
   ELSE
        OUT3 = 0
   endif

        'Voltage Display
        if Volts < 100 then                         '+15
            Lcdout $fe, LINE1 + 13,"  ", DEC Volts/10,".",DEC Volts//10," V"
        endif
        if (Volts > 99) AND (Volts < 1000) then     '+14
            Lcdout $fe, LINE1 + 13," ", DEC2 Volts/10,".",DEC Volts//10," V"
        endif
        if Volts > 1000 then                        '+13
            Lcdout $fe, LINE1 + 13, DEC3 Volts/10,".",DEC Volts//10," V"
        endif
        
        'Temperature Display
        If TempF < 10000 then                       '+13
            Lcdout $fe, LINE2 + 12," ", DEC2 TempF/100,".",DEC2 TempF//100," F"
        endif
        If TempF > 9999 then                        '+12
            Lcdout $fe, LINE2 + 12, DEC3 TempF/100,".",DEC2 TempF//100," F"
        endif
    
        'Current Display
        if Amps < 100 then                          '+15
            Lcdout $fe, LINE3 + 14," ", DEC Amps/10,".",DEC Amps//10," A"
        endif
        if (Volts > 99) AND (Volts < 1000) then     '+14
            Lcdout $fe, LINE3 + 14, DEC2 Amps/10,".",DEC Amps//10," A"
        endif

        'RPM Display
        if RPM < 1000 then                  '+15 
            Lcdout $fe, LINE4 + 14," ", DEC3 RPM," R"
        endif
        if RPM > 1000 then                  '+14
            Lcdout $fe, LINE4 + 14, DEC4 RPM," R"
        endif

goto main
'*****************************************************************************************

'************************************VOLTAGE SUB******************************************
GetVoltage:
    'VOLTAGE TO BE FED THROUGH A 73.25:1 SCALER
    '150vdc / 73.25 = 2.0477vdc
    VCurrentCNT = VCurrentCNT + 1
    if VCurrentCNT > LoopCNT then
        Volts = VHolder / LoopCNT   '5453 / 5 = 1090
        CompareVolts = Volts / 10
        VCurrentCNT = 0
        VHolder = 0
    else
        ADCON0 = %10001111      '7=N/A,6-2=Channel,1=Go/Done,0=Enable
        WHILE ADCON0.1
        WEND
   	    wTemp.HighByte = ADRESH
   	    wTemp.LowByte = ADRESL
   	    Dummy = wTemp * QVolts
   	    Volts = DIV32 1000          '1099   1095    1088    1075    1096
   	    VHolder = VHolder + Volts   '1099   2194    3282    4375    5453            
    endif
RETURN
'*****************************************************************************************

'************************************TEMPERATURE SUB**************************************
GetTemp:
    OWOUT GPIO3, 1, [$CC, $44]                          'Skip ROM search & do temp conversion
    Wait_Up:
        OWIN GPIO3, 4, [Busy]                           'Read busy-bit
        IF Busy = 0 THEN Wait_Up                        'Still busy..?, Wait_Up..!
    OWOUT GPIO3, 1, [$CC, $BE]                          'Skip ROM search & read scratchpad memory
    OWIN GPIO3, 2, [R_Temp.Lowbyte, R_Temp.Highbyte]    'Read two bytes / end comms
    Dummy = 1125 * R_Temp                               '
    TempF = DIV32 100
    TempF = TempF + 3200
    CompareTemp = tempf/100
RETURN
'*****************************************************************************************

'************************************CURRENT SUB******************************************
GetCurrent:
    'CURRENT SHUNT = 75mV @ 50A
    'VOLTAGE TO BE AMPLIFIED WITH A MAX4460, GAIN SET AT ~27.3
    '~27.3 * .075 = 2.0475
    ACurrentCNT = ACurrentCNT + 1
    if ACurrentCNT > LoopCNT then
        Amps = AHolder / LoopCNT
        CompareAmps = amps/10
        ACurrentCNT = 0
        AHolder = 0
    else
        ADCON0 = %10010011      '7=N/A,6-2=Channel,1=Go/Done,0=Enable
        WHILE ADCON0.1
        WEND
   	    wTemp.HighByte = ADRESH
   	    wTemp.LowByte = ADRESL
   	    Dummy = wTemp * QAmps
   	    Amps = DIV32 1000
   	    AHolder = AHolder + Amps
    endif
RETURN
'*****************************************************************************************

'************************************PULSE INTERRUPT**************************************
PulseINT:
    PulseCNT = PulseCNT + 1
    IOCBF.7 = 0
@ INT_RETURN
'*****************************************************************************************

'************************************TIMER INTERRUPT**************************************
Overflow:
@ INT_DISABLE IOC_INT
        T1CON   = timeroff
        OvflCNT = OvflCNT + 1
        if OvflCNT = OvflReadNow then                       '1 PulseCNT in .6 Seconds
            'Read PulseCNT                                  'f (RPS) = 1 / t
            'Calculate RPM                                  'f (RPS) = 1 / .6
            RPM = PulseCNT * 100                            'f (RPS) = 1.666666
            'Reset OvflCNT to 0                             'RPM = RPS * 60
            OvflCNT = 0                                     'RPM = 100                         
            'Reset PulseCNT to 0                          
            PulseCNT = 0
            toggle HrtbtLED                                 'On Off every 600 mS                                              
        endif        
        TMR1H = preload.highbyte
        TMR1L = Preload.lowbyte
        T1CON = timeron
@ INT_ENABLE IOC_INT
@ INT_RETURN
'*****************************************************************************************

'***********************************SERIAL INTERRUPT**************************************
RS232in:
@ INT_DISABLE IOC_INT
@ INT_DISABLE TMR1_INT
@ INT_DISABLE RX_INT
    low RxLED   'Rx LED ON
    HSERIN [dec3 SerialIn]
    If SerialIn = 255 then gosub SetupModule
    while PIR1.5
        SerialIn = RCREG
    wend    
    high RxLED  'Rx LED OFF
@ INT_ENABLE IOC_INT
@ INT_ENABLE TMR1_INT
@ INT_ENABLE RX_INT
@ INT_RETURN
'*****************************************************************************************

SetupModule:

HSERIN [CmdPrefix,dec3 CmdAddress]
    
    select case CmdPrefix
        '-------------------------Address Module----------------------------
        case "A"                    
            Address = CmdAddress
            write 0,Address
            LOW TxLED
                HSEROUT ["ACK:A",DEC3 Address,13,10]            
            HIGH TxLED
            read 0,Address
        '------------------------------------------------------------------
        '-------------------------Command Module----------------------------            
        case "B"                    
            '--------------------Address Mismatch--------------------------
            if CmdAddress <> Address then
                goto Junk
            endif
            '--------------------------------------------------------------
            hserin[command]     'O, Q, or M
            select case command
                case "O"    '(OUTPUT)
                    HSERIN[dec CmdOutput]   ' "CmdOutput" is holding 1,2 or 3
                    select case CmdOutput
                        case 1
                            toggle OUT1
                            pause 1500
                            toggle OUT1
                        case 2
                            toggle OUT2
                            pause 1500 
                            toggle OUT2
                        case 3
                            toggle OUT3
                            pause 1500 
                            toggle OUT3
                        CASE ELSE
                            goto Junk            
                        END SELECT
                Case "Q"                                                '(QUERY)    '$BxxxQ
                    low TxLED                                           'Tx LED ON
                    HSEROUT ["ACK:B",DEC3 Address,":",13,10,_           'Send Address First
                             "VOLTS: ",DEC VOLTS,13,10,_                'VOLTS: 10500
                             "TEMP: ",DEC TempF,13,10,_                 'TEMP: 102.4
                             "CURRENT: ",DEC Amps,13,10]                'AMPS: 2350
                            TRISA  = %11111011                          'A.4=OUT3,A.1=OUT1,A.0=OUT2
                            HSEROUT ["OUTPUT 1: ",dec PORTA.1,13,10,_   'OUTPUT 1: 0
                                     "OUTPUT 2: ",DEC PORTA.0,13,10,_   'OUTPUT 2: 1
                                     "OUTPUT 3: ",DEC PORTA.4,13,10]    'OUTPUT 3: 1
                            TRISA  = %11101000                          'A.4=OUT3,A.1=OUT1,A.0=OUT2
                    high TxLED                                          'Tx LED OFF
                Case "M"                                                '$B001 "M" 11025511255
                    'Output 1
                    '10=LowVolts, 11=HighVolts, 12=LowTemp, 13=HighTemp, 14=LowCurrent, 15=HighCurrent
                    'Output 2
                    '20=LowVolts, 21=HighVolts, 22=LowTemp, 23=HighTemp, 24=LowCurrent, 25=HighCurrent
                    'Output 3
                    '30=LowVolts, 31=HighVolts, 32=LowTemp, 33=HighTemp, 34=LowCurrent, 35=HighCurrent
                    'Output 4
                    '40=LowVolts, 41=HighVolts, 42=LowTemp, 43=HighTemp, 44=LowCurrent, 45=HighCurrent
                    '255B001                  Board Address
                    '       M                 Map
                    '        10               EEProm Location (Low Trip)
                    '          255            Low Trip Value   
                    '             11          EEProm Location (High Trip)
                    '               255       High Trip Value
                    HSERIN[dec2 eepromloc]
                    HSERIN[dec3 tripval]
                    write eepromloc,tripval
                    HSERIN[dec2 eepromloc]
                    HSERIN[dec3 tripval]
                    write eepromloc,tripval
                    ReReadVals = 1
                case else
                    goto junk
                end select
        end select
Junk:
return

[kevlar129bp]

Doh, I spoke too soon! I can't believe I missed the WOPR Register

[HenrikOlsson]

Hi Chris,
I don't follw you on the WOPR register buy anyway

On the topic of the LCD routines, is this what you meant Henrik:
Lcdout $fe, LINE1 + 15, DEC Volts/10,".",DEC Volts//10," V"

Yes and no. With that you, again, have the same calculation in several places. I wan thinking more in line with this:

        'Voltage Display
        if Volts < 100 then                         '+15
            Lcdout $fe, LINE1 + 15
        endif
        
        if (Volts > 99) AND (Volts < 1000) then     '+14
            Lcdout $fe, LINE1 + 14
        endif
        
        if Volts > 1000 then                        '+13
            Lcdout $fe, LINE1 + 13
        endif
        
        LCDOUT DEC Volts/10,".",DEC Volts//10," V"

Doing that fror current ant RPM too should save you a bit more space.

/Henrik.

[kevlar129bp]

Thanks Henrik,

I see the further refinement now. BTW, the WOPR is in reference to the awesome computer in war games :-)

Thanks again... Back to refining,
Chris

[kevlar129bp]

Hey Henrik,

Just got back to my pc, and have a question...

With my current code being

        'Voltage Display
        if Volts < 100 then                         '+15
            Lcdout $fe, LINE1 + 13,"  ", DEC Volts/10,".",DEC Volts//10," V"
        endif
        if (Volts > 99) AND (Volts < 1000) then     '+14
            Lcdout $fe, LINE1 + 13," ", DEC2 Volts/10,".",DEC Volts//10," V"
        endif
        if Volts > 1000 then                        '+13
            Lcdout $fe, LINE1 + 13, DEC3 Volts/10,".",DEC Volts//10," V"
        endif

if the "Volts" drops from, let's say 1234, to 789, wont I have to overwrite the "1" with a "space"?
If I don't, won't the LCD display "VOLTAGE: 178.9 V", or will it display the proper "VOLTAGE: 78.9 V", (the "1" being an artifact from the previous display)

I guess I'm sorta confused ,

Thanks for taking the time to help me out,
Chris

[HenrikOlsson]

It depends on where in the code you clear the screen.
If you only clear the screen at the start of the program then yes, you need "erase" whatever garbage that might or might not be there - which it looks like you ARE doing with the code you just posted.

If you clear the screen at each update (ie you refresh the whole screen) then you obviously don't need to "erase" any "garbage" that might be on there - which is the method in the snippet I posted earlier.

Up to you to decide which is the most appropriate. (Thinking about it I'd probably go with your method of "erasing" whatever might be there, it'll be quicker but MIGHT take slightly more code space.)

/Henrik.

PS. Are you actually running this code on anything to see if it works or are you just churning out code "blind" so to speak?

[kevlar129bp]

Hello Henrik.

If you don't see a large problem with me wiping the individual characters, than I'll give that a run. As far as "live" testing, not as of yet. I designed and etched a board some time ago that I just plop different code on when it hits me. As far as the purpose of this code, I have dreams of hooking it to a dc brushed gearhead motor that I fitted with an encoder (not a true servo motor ) that will be mounted to a rock tumbler. The board will monitor, as I'm sure you can tell, voltage, current, temp and rpm (through the index pulse of the encoder). The power supplies are 2 HP ESP120 (51.4V, 57A) 3KW server supplies wired in series. Right now, you're probably thinking WTF! Why would he run those? Well, all I can say is, they're the cheapest power you'll ever find . I've juiced 'em in series, and they haven't missed a beat. The only tricky part is isolating the high-side supply from earth as well as humans. I have them both in a fully insulated 1/4" polycarbonate enclosure. As far as wanting the code for the pic finalized (or almost) before the first run is nothing more than it's kinda a pain in the butt doing the remove, burn, reinsert routine (the LCD is mounted on stand-offs above the chip carrier as well as no ICSP on my board). If you think I've got workable code, I'll go for it. Do you have any slicker ways to get finer RPM from my encoder scheme? I have to admit that I have brain fried over that part!

Thanks again Henrik,
Chris

[HenrikOlsson]

Hi Chris,
As long as it works I have no problems and, like I said, "erasing" the individual characthers sounds like the best solution.

Sounds like a fun project! Be careful, lots of power there!

Since your chip has a USART and you're laready using it to communicate with a PC (by the looks of it) I'm assuming the board has a MAX232 or similar device so why not use a bootloader to get the code into the PIC? (And next time, put a ICSP header on there...)

As for the RPM, since you apparently aren't interested in direction and/or position why not run one of the A/B channels of the encoder to a timer/counter input and configure the appropriate timer in the PIC as a counter. Read it at every TMR1 interrupt and you have motor speed.

/Henrik.

[wdmagic]

I just add 3-5 spaces at the end of my strings so any leftover characters are replaced with a space. I did have some code that forced a specific number of characters for a number so 123.5 would be displayed as that, however 57.1 would be displayed as 057.1, therefore no change in character location is needed. if I can find my code Ill post it. you might search for leading 0's? dont know. henrik or one of the other mods may help with this too.

[HenrikOlsson]

Hi,
Yes, if you want (or don't but can live with) leading zeros then use DEC3 (or DEC2, DEC4 etc) to specify the number of digits with which the number will be displayed. In Chris's case he's trying to right justify the displayed number (without leading zeros) so he's determining the number of digits in the number and then prints "leading spaces" to erases what ever might be there from before. Again - whatever works.

/Henrik.

[AvionicsMaster1]

An approach I've used for adding ICSP on a thru hole project where one isn't installed is to solder the important leads ICSPDAT, ICSPCLK and MCLR to a socket that will go between the the chip and the circuit card. As long as the circuit won't inhibit the loading because of a lowish resistance on clk or data you can install this temporarily and remove it when you're finished. Though I've yet to figure out a work around for surface mount projects.

I've learned it isn't much more expensive to add the ICSP header and a switch to isolate the chip when needed for loading a program. I know this approach isn't possible in all instances but if you've got the space why not use it.

Finally I want to thank you guys for helping out Kevlar and explaining why your code helps/is faster/more accurate. Exactly what I need to improve my capabilities.

[kevlar129bp]

Thanks WD, Henrik and Avionics,

I was pondering the leading zero method, which certainly is not a deal breaker for me...I just thought it would be sexier to right justify If I find a need for a tad more speed, I'll definitely use leading zeros. On the ICSP thing, I think I might modify my board to have it. I think my short term pain in changing the etch would pay off well in the long term, which is usually the case in life isn't it... Did you guys happen to have any thoughts on the RPM Sub? Another thing I noticed is, what are the odds that I may miss an encoder index pulse while inside the Timer ISR (because I disable the IOC Interrupt)? Are there any ways that I could safeguard against this situation, or should I sweat it? This will obviously be a more critical situation at low rpm...

I can't thank you guys enough for your thoughts and willingness to help (Ditto what Avionics wrote ),
Chris

EDIT: Sorry Henrik, I completely missed your previous post in regards to the RPM thing. I thought about that A or B channels, which would be way more accurate...if I only knew how many PPR they are (way old proprietary Renco's from a ct scan machine). I suppose it would be pretty simple to whip up some code to count the pulses as I rotate the motor by hand...hmmmm.

[richard]

from the data sheet

"The ISR determines the source of the interrupt by
polling the interrupt flag bits. The interrupt flag bits must
be cleared before exiting the ISR to avoid repeated
interrupts. Because the GIE bit is cleared, any interrupt
that occurs while executing the ISR will be recorded
through its interrupt flag, but will not cause the
processor to redirect to the interrupt vector."

therefore since your overflow routine is an isr the int_disable lines are redundant and just waste time .
since position is not important to lose a pulse woud probably not matter