??? 07/20/06 13:14 Read: times Msg Score: +1 +1 Good Answer/Helpful |
#120670 - Some comments Responding to: ???'s previous message |
Payam, There's a few things that worry me - the main loop does nothing, all the work is done in ISRs. You really want to minimise the number of interrupt sources and I think the code could be arranged so that it didn't need so many interrupt sources by having the main loop do most of the work. The outputting to the shift registers doesn't need to be done in a ISR - loading the output register could be done in an ISR to maintain timing. Your serial ISR relies on DPTR having the correct address. It may work fine in your current code, but modifications in the future may cause weird side effects. The way your data is laid out is not clear from your code. Again, this makes modifications difficult in the future. Wherever possible, I try to make my code as generic as possible - that is making sure I don't rely on other routines behaving in a certain way. Sure, sometimes when we're running out of clock cycles we need to make shortcuts. I've learnt these things from many years of having to add features to code - a little more thinking up front can save hours later on and minimise weird defects. If you over engineer, you can always trim it back if necessary. Using flags makes for short code but can get confusing once you have more than a few flags to control operations. It can be easier to use finite state machines where you have a state value rather than a handful of flags. It also makes the code easier to read and maintain. See what the other guys say and if they agree/disagree with my comments. I'm sure there'll be a bit of discussion. You can then to all these comments and see if you can take a new look at your code and see places where you could change it to make it easier to understand/change etc. The first step is to get code that works! You have got to this step. Then you can evaluate it and see how you can make it better. Congratulations! |