[quote=buddy.smith;567537]I was just suggesting the array method to show the possibility. You can do it the way you were doing it before.... (details below)
What's the -1 for? Why not just set*4, then add 0, 1, 2, 3?
Quote:
Originally Posted by htwiz2002
Code:
void Terminal_eepromreadset(unsigned char set, unsigned char *data1, unsigned char *data2, unsigned char *data3, unsigned char *data4)
{
//Consolidate this on verify that it works!!!
unsigned int address = set * 4 -1;
*data1 = EEPROM_Read(address + 1);
*data2 = EEPROM_Read(address + 2);
*data3 = EEPROM_Read(address + 3);
*data4 = EEPROM_Read(address + 4);
}
This looks right. I'd recommend changing 'data1' to pData1, and so forth. This lets you know that it's a pointer. I sometimes prefer to put the * next to 'char' as well, but that's something that C programmers argue about constantly
I'd work on the formatting here. Move the 'break' to a line by itself.
Also, why are you using global variables here instead of declaring them inside this function?
Also, perhaps you should name your case....
Code:
#define VERSION_SET (0)
....
...
case VERSION_SET:
....
Again i'd improve the formatting:
Code:
default:
return 999;
break;
I hope this doesn't come off as critical. I'm trying to be constructive and help you come up with code that is clear to the human.
A programmer's job is to write documentation that happens to compile....
Suggestions: Use whitespace, it's free. Avoid global variables like the plague.
If you must use globals, stick a 'g' in front of the name, again so that you know it's a global.
Make all your globals 'static'. This means that they won't conflict with another global in a different file. That can cause huge headaches.
ttyl,
--buddy
|
!!! sorry, i should have mentioned it's incomplete!

Anyways, I'll make some major changes....
With the "break" thing I was trying to consolidate things so that I wouldn't need to scroll half a mile just to get from section to section, while (hopefully) improving readability (since most of the code that I combine to one line doesn't need (normally) to be changed or read much.
Also, my globals (generally) have the file's name prefixed so as to help prevent them being used (normally, again) outside of the file. Problems do arise when I try to access them o-o-f, but since I moved the handler code i haven't needed to (much).
Again, code in beta, far from finished. I was using the version globals for version checking and for the test set. They will become #define s soon enough, as well as many of the other global variables.