Code Review?

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

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
  Code:

  ---

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

  ---

  could be LCDOUT REP "."\16

there's probably other things but begin with it :)

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..

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

Code:

---

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

Code:

---

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

---

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