Code Review?

[Tom Gonser]

Hey folks: Not sure if the is an appropriate ask for this group, but over the past few months I've been writing a PBP app that listens to a data link for a custom GPS(non NMEA) data stream, and then displays this on an LCD - largely based on feedback from this forum and others who have been kind enought to help me along. You may even recognize some of the code - like in the interrupt and timing routine (which I am still not sure I fully understand, tho it appears to work for me)..

Now that I am pretty well done with it, I'd like to find someone to take a look at this project and give me a code review - suggest areas where I am making novice mistakes, etc. I know there will be many - I AM a novice.

Some of the things I notice in the operation of the code:

1. If it does not hear a signal for a while it crashes. Not sure why it would do that.
2. There are sections of the code that just don't work and are marked as such.
3. It takes up more codespace than I thought it would..

If someone has some time to help out that'd be great.

TG

[mister_e]

Few suggestions

• 1. add an external EEPROM to store your Text and ARRAY variables to save code space BUT BEFORE look if all of them can fit in the internal one.
  
  2. do some Subroutine with all command you call often as I2CREAD, SEROUT, LCDOUT. Just call those with all your specific need (variables, EEPROM address range for text....)
  
  3. Use internal USART(if your actual design permit) with HSERIN command
  
  4. Use BRANCHL instead of SELECT CASE Bmenu (line 302)
  
  5. this

  	for X = 1 to 16
  		LCDout "."
  	Pause 30
  	Next X

  could be LCDOUT REP "."\16

there's probably other things but begin with it

[Tom Gonser]

Thanks much! I will try these out for sure.

Any input on interrupt processor or timing routine? For some reason once I hit the interrupt to select a menu item, if THAT menu item needs to use the BO button it does not respond..

[mister_e]

not sure of me but what about if you modify that section

While selectit = 0
   Mnu=1           			' we are in the menu system
   If Mmenu = 0 then 
    	While  Mmenu = 0    ' waiting until
    	wend                ' push-button is release
    	pause 100          	' debounce time
   		Bmenu=Bmenu+1 		' increment each time B0 is presseed
   endif       ' bo pressed – go to next menu

to

FirstPass=1  
While selectit = 0
   Mnu=1           			' we are in the menu system
   If Mmenu = 0 then 
    	While  Mmenu = 0    ' waiting until
    	wend                ' push-button is release
    	pause 100          	' debounce time
   		if FirstPass =0 then Bmenu=Bmenu+1 		' increment each time B0 is presseed
   endif       ' bo pressed – go to next menu
   FirstPass=0

[Tom Gonser]

What does the new variable 'firstpass' do? Does it allow me to continue to use the BO and B1 buttons in submenus somehow?

My biggest problem is that when I interrupt, I go into the menu routine, but then if a SUBMENU routine there needs to use the buttons to choose things -- Like the LCDSettings: menu - they just don't do anything - I press buttons in this SubMenu routine (LCDsetting). This forces me to have a HUGE linear menu system which sucks... - ie 12 menu items vs say 4 top menus, each with 3 choices..

TG