Shop OBEX P1 Docs P2 Docs Learn Events
Any suggestions for making my code more elegant? — Parallax Forums

Any suggestions for making my code more elegant?

ITSengITSeng Posts: 5
edited 2008-10-17 15:57 in BASIC Stamp
As I've posted recently, I forgot my old user name so·I have a new one. I posted an extensive thread a few years ago about a submarine project I'm 99% finished with. The sub uses 2 BS2s, the code below is used to control the vehicle, the other BS2 does all of the sensor functions. I'd like some suggestions to see if I can clean this up a little.

What it does (it does work perfectly and is stable):
Controls 2 trolling motors with 2 HB25s, sends feedback to a GUI
Controls 2 servos that aim the camera
Controls a lighting system
Controls solenoid valves for a compressed CO2 system for diving and surfacing

Everytime·I look at it I get the feeling there is a simpler way to do some of the functions.

The letter "C" means a camera function in the code.


I'll try to add a picture of my "custom" circuit set-up. [noparse]:)[/noparse]




Post Edited (ITSeng) : 10/16/2008 10:03:11 PM GMT

Comments

  • SRLMSRLM Posts: 5,045
    edited 2008-10-16 22:17
    All I see is Blue and Black, not and barely any green. A simple label on each method is useful, along with all variable. Also, you can avoid "magic numbers", which are numbers that are in your code, but don't have any explanation. For example, PIN numbers can be easily explained if you use a name for them (like rightMotor and leftMotor) or LCDPin. What is Buttons1 or Buttons2? What are the values for light? In short, documentation will make your life a whole lot easier in the future when you want to look at the code again. Also, what are you sending via SEROUT? Letters? Numbers? Commands? The general goal is to make your code self explanatory.
  • ITSengITSeng Posts: 5
    edited 2008-10-16 22:50
    Yea, I knew someone would point out my lack of green. It's self explanatory to me. ·smile.gif

    LM = left motor
    RM = right motor

    upC = camera up

    displ = display left (motor)

    this is an address and an·ASCI srting sent to the display software for instance:
    [noparse][[/noparse]$B0,$FE,2,77,111,116,111,114,115,32,82,101,97,100,121,32,32,32,32,32,CR]

    Things like what is in the "datain" subroutine, it just seems like there's an easier way.

    You think this code looks complicated? The code for the sensor BS2 is 5X as complicated.
  • Lee HarkerLee Harker Posts: 104
    edited 2008-10-17 15:07
    One thing I usually look for to condense a program is to look at areas where there are lots of If Then statements in a row with systematic conditions. In your program where you are reading the pushbuttons, you could reduce to·a couple of lines using a combination of NCD and Branch.

    Also you have lots of data being sent out serially with lots of repeats. That may also be able to be condensed by using the repeat modifier for SEROUT.

    In the motor display section, you may find that SELECT would be handy.

    Just my $.02
  • HumanoidoHumanoido Posts: 5,770
    edited 2008-10-17 15:57
    Fascinating project. Keep up the good work. If you want to make the code "more elegent" take a look at the declarations such as the variable "light." It's set as a byte, which is defining values from 0 to 255. Yet in the code, light never strays beyond a 0 or 1. It could be declared as a bit and save considerable RAM. led is declared as a byte variable, but never used in the program. It can be removed.

    humanoido
Sign In or Register to comment.