Critique my code, please.

[wdmagic]

I agree Robert, I wish more people would post schematics, pictures and full code. although I know whats its like if your afraid someone will steal your ideas... but the ideas may have only gotten into the head by being sparked by ideas here. I know there is alot like that for me. So.. share the love people and start posting more Pic's (no pun intended), schematics and code

[Jerson]

This seems redundant to me

    if PIP = 0 then increment
    if PIP = 1 then display
increment:

It could be

    if PIP = 1 then display
increment:

Another possible improvement would be to consider using an interrupt to capture the change in state of PIP. Without that, you could miss / misread the timing since the code could be updating the display and doing a pause thereby losing time before you poll the PIP pin.

[HenrikOlsson]

Hi,
Just a quick note

CNT = CNT + 1
IF CNT > 65535 then ' This adds a overload catch so it doesnt run away.
CNT = 0
goto no_data2
ENDIF

The above doesn't really work because CNT is declared as a WORD and can not contain values larger than 65535. The IF statement will never evaluate true so the program will never jump to no_data2. Once CNT is incremented "beyond" 65535 it will automatically wrap around to 0.

/Henrik.

[pedja089]

Correct way to do that is:

CNT = CNT + 1
IF CNT =0 then ' This adds a overload catch so it doesnt run away.
    CNT = 0
    goto no_data2
ENDIF