CCS :: View topic - there must be a better way

FAQ

Forum Help

Official CCS Support

Profile Log in to check your private messages Log in

CCS does not monitor this forum on a regular basis.

Please do not post bug reports on this forum. Send them to CCS Technical Support

there must be a better way
Goto page Previous 1, 2

CCS Forum Index -> General CCS C Discussion

View previous topic :: View next topic

Author

Message

ckielstra

Joined: 18 Mar 2004
Posts: 3680
Location: The Netherlands

Posted: Tue Jan 03, 2006 5:08 am

Markdem wrote:

This, however, does not fix the out of rom problem.
Does anyone have any ideas on how i can send some kind fo
configuration file to configure the on and off timers??

Did you have a look at the list file (*.lst) ? By looking at this file you can
see which functions require a lot of ROM space.

I don't have your ds1307.c and 24LC256.c files so I took the ds1302.c
and I removed the calls to the eeprom. Compiled in v3.226 I then get
about 50% ROM usage, is this similar to your results?

From the list file you will learn:
- the printf lines take a lot of memory space, do you really need all of them?
- the lines with assignments to a variable using getc are small and not
really a target for optimization.
- the combination of gets/atoi calls are large and very similar, try
replacing this by a new single function call.
- calls to read and write the eeprom are inline functions and take a lot of
memory. I was able to save 662 bytes in your program by creating
simple wrapper functions.

Code:

//----------------------------------------------------
// A wrapper for the standard macro in order to save some memory space.
//----------------------------------------------------
void my_write_eeprom(int8 Address, int8 Value)
{
write_eeprom(Address, Value);
}

//-----------------------------------------------------
// A wrapper for the standard macro in order to save some memory space.
//-----------------------------------------------------
int8 my_read_eeprom(int8 Address)
{
return read_eeprom(Address);
}

Markdem

Joined: 24 Jun 2005
Posts: 206

Posted: Tue Jan 03, 2006 5:48 am

Hi all, as sombody that has done very little c programing, i am very happy with the following code that i made up. What do you think. It is been called when i set a flag in my ISR.

Code:

void writeuservars()                                        //writes user set vars to eeprom
{
   for(i=0; i<=22; i++)
      {
         disable_interrupts(INT_RDA);
         printf("0\n");
         gets(datain);
         buff = atoi(datain);
         write_eeprom(i,buff);
         enable_interrupts(INT_RDA);
      }
}

the code above replaced all of the eeprom writing i had before.
My program now build ok, and i am only using 52% of the rom Smile

Is this a good way of doing this?

I am now going to work on fixing the rest of the ISR

Thanks again for the help

Mark

hillcraft

Joined: 22 Sep 2003
Posts: 101
Location: Cape Town (South africa)

Rom usage cure - Arrays

Posted: Wed Jan 04, 2006 11:01 am

You should be using arrays to deal with your data.

Code:

#define MAX_LIGHTBANKS 5

int lightbank_on_m[MAX_LIGHTBANKS];
int lightbank_off_mf[MAX_LIGHTBANKS];
int lightbank_on_h[MAX_LIGHTBANKS];
int lightbank_off_hf[MAX_LIGHTBANKS];
int lightbank_stat[MAX_LIGHTBANKS];
int lightbank_force[MAX_LIGHTBANKS];

Now all the other code is much simpler to work with and also takes up less space.

This code:

Code:

printf("%U\n",lightbank1_stat);
printf("%U\n",lightbank2_stat);
printf("%U\n",lightbank3_stat);
printf("%U\n",lightbank4_stat);
printf("%U\n",lightbank5_stat);

Can be replaced with:

Code:

for (i=0;i<MAX_LIGHTBANKS;i++)
printf(" %U ",lightbank_stat[i]);

This code:

Code:

lightbank1_force = 50; //set vars
lightbank2_force = 50;
lightbank3_force = 50;
lightbank4_force = 50;
lightbank5_force = 50;
lightbank1_stat = 0;
lightbank2_stat = 0;
lightbank3_stat = 0;
lightbank4_stat = 0;
lightbank5_stat = 0;

can be replaced with:

Code:

for (i=0;i<MAX_LIGHTBANKS;i++) {
lightbank_force[i] = 50;
lightbank_stat[i] = 0;
}

This code:

Code:

printf("%02u:%02u\n",lightbank1_on_h,lightbank1_on_m);
printf("%02u:%02u\n",lightbank1_off_h,lightbank1_off_m);
printf("%02u:%02u\n",lightbank2_on_h,lightbank2_on_m);
printf("%02u:%02u\n",lightbank2_off_h,lightbank2_off_m);
printf("%02u:%02u\n",lightbank3_on_h,lightbank3_on_m);
printf("%02u:%02u\n",lightbank3_off_h,lightbank3_off_m);
printf("%02u:%02u\n",lightbank4_on_h,lightbank4_on_m);
printf("%02u:%02u\n",lightbank4_off_h,lightbank4_off_m);
printf("%02u:%02u\n",lightbank5_on_h,lightbank5_on_m);
printf("%02u:%02u\n",lightbank5_off_h,lightbank5_off_m);

Can be replaced with:

Code:

for (i=0;i<MAX_LIGHTBANKS;i++) {
printf("%02u:%02u\n",lightbank_on_h[i],lightbank_on_m[i]);
printf("%02u:%02u\n",lightbank_off_h[i],lightbank_off_m[i]);
}

You can even go as far as sequentially reading and writing your EEPROM

Don't do this:

Code:

#define addlightbank1_on_h 0x03
#define addlightbank1_off_h 0x04
#define addlightbank1_on_m 0x05
#define addlightbank1_off_m 0x06
#define addlightbank2_on_h 0x07
#define addlightbank2_off_h 0x08
#define addlightbank2_on_m 0x09
#define addlightbank2_off_m 0x10
#define addlightbank3_on_h 0x11
#define addlightbank3_off_h 0x12
#define addlightbank3_on_m 0x13
#define addlightbank3_off_m 0x14
#define addlightbank4_on_h 0x15
#define addlightbank4_off_h 0x16
#define addlightbank4_on_m 0x17
#define addlightbank4_off_m 0x18
#define addlightbank5_on_h 0x19
#define addlightbank5_off_h 0x20
#define addlightbank5_on_m 0x21
#define addlightbank5_off_m 0x22

Declare your EEPROM addresses as follows:

Code:

#define #define addlightbank_on_h 0x03 // this is the 1st address of on_h
#define #define addlightbank_off_h addlightbank_on_h+5 // this is the 1st address of off_h
#define #define addlightbank_on_m addlightbank_off_h+5 // this is the 1st address of on_m
#define #define addlightbank_off_m addlightbank_on_m+5 // this is the 1st address of off_m

Now, this allows you to do the following:

Code:

void readuservars() {
//reads user set vars from eeprom
int i;

maxtemp = read_eeprom(addmaxtemp);
maxhoodtemp = read_eeprom(addmaxhoodtemp);
maxph = read_eeprom(addmaxph);

for (i=0;i<MAX_LIGHTBANKS;i++) {
lightbank_on_m[i] = read_eeprom(addlightbank_on_m+i);
lightbank_off_m[i] = read_eeprom(addlightbank_off_m+i);;
lightbank_on_h[i] = read_eeprom(addlightbank_on_h+i);
lightbank_off_h[i] = read_eeprom(addlightbank_off_h+i);;
}
}

void writeuservars() {
//wites user set vars to eeprom
int i;

disable_interrupts(INT_RDA);
write_eeprom(addmaxtemp, maxtemp);
write_eeprom(addmaxhoodtemp, maxhoodtemp);
write_eeprom(addmaxph, maxph);

for (i=0;i<MAX_LIGHTBANKS;i++) {
write_eeprom(addlightbank_on_m+i,lightbank_on_m[i]);
write_eeprom(addlightbank_off_m+i,lightbank_off_m[i]);
write_eeprom(addlightbank_on_h+i,lightbank_on_h[i]);
write_eeprom(addlightbank_off_h+i,lightbank_off_h[i]);
}
enable_interrupts(INT_RDA);
}

You can get your data from the VB app in a loop in exactly the same way.

You must also keep in mind that your light banks will no be numbered from 0-4

The bottom line is this: If you need to use lots of variables that are very similar in nature and can be accessed sequentially, then the cleanest way to do so is to use arrays. Iterating through arrays chops the code size right down.

Laughing

Markdem

Joined: 24 Jun 2005
Posts: 206


Posted: Wed Jan 04, 2006 4:04 pm

hillcraft, you are a champ thank you so much for the code examples. I will rewrite my code today and post the rom useage Thank you all for your help

Markdem

Joined: 24 Jun 2005
Posts: 206

Posted: Mon Jan 09, 2006 6:13 am

Hi all again. I am back on my project again, dame work getting in the way Smile

Anyways, all is working purfect, but this i cant work out.

I need to change the output pin to reflect which light bank i am controlling, eg, each time the for loop is started, i need the output pin to go up by one

Can anybode help??

Thanks all

Code:

void lightwork()
{
   ds1307_get_time(hrs,min,sec);

      for (i=0;i<MAX_LIGHTBANKS;i++)
         {
            if(lightbank_force[i] < 50)
               {
                  if(lightbank_force[i] == 48)
                     {
                        output_low(light1);
                        lightbank_stat[i] = 0;
                     }
                  else
                     {
                        output_high(light1);
                        lightbank_stat[i] = 1;
                     }
               }

            else
               {
                  if (hrs >= lightbank_on_h[i] && hrs <= lightbank_off_h[i] && min >= lightbank_on_m[i] && min < lightbank_off_m[i])
                     {
                        output_high(light1);
                        lightbank_stat[i] = 1;
                     }
                  else
                     {
                        output_low(light1);
                        lightbank_stat[i] = 0;
                     }
               }
         }
   }

the defines look like this

Code:

#define LIGHT1       PIN_D0                                                    //global defines
#define LIGHT2       PIN_D1
#define LIGHT3       PIN_D2
#define LIGHT4       PIN_D3
#define LIGHT5       PIN_D4

hillcraft

Joined: 22 Sep 2003
Posts: 101
Location: Cape Town (South africa)

A couple of things

Posted: Tue Jan 10, 2006 10:51 am

You can make your life a lot less cryptic

I suspect that the following can be made simpler:

Code:

if(lightbank_force[i] == 48)
{
output_low(light1);
lightbank_stat[i] = 0;
}
else
{
output_high(light1);
lightbank_stat[i] = 1;

in the following way:

Code:

#define D_OFF 48 // this allows you to use the ASCII codes throughout the program
#define D_ON 49

if(lightbank_force[i] == D_OFF)
{
output_low(light1);
lightbank_stat[i] = D_OFF;
}
else
{
output_high(light1);
lightbank_stat[i] = D_ON;

Now you don't have to wonder about the status of the lightbank.

I don't think that you can pass a variable to:

output_high() and output_low()

The easiest way would be to write a function and pass it 2 variables. One to determine the output state, and the other to determine the lightbank.

Then all you do in this function is to create an if statement for the on-off state of the lightbank, and a case within each if to set the appropriate light bank. You can also mode the setting of the lightbank_stat array to the function:

Code:

if(lightbank_force[i] == D_OFF)
output_lightbank(i,D_OFF) // remember that the lightbank is offset to 0 so using i will be fine
else
output_lightbank(i,D_ON);

The function would look something like this:

Code:

void output_lightbank(int BankNo, int BankState) {

lightbank_stat[BankNo] = BankState;

if (BankState==D_ON) {
switch (BankNo) {
case 0:
output_high(LIGHT1)
break;
case 1:
output_high(LIGHT2)
break;
case 2:
output_high(LIGHT3)
break;
case 3:
output_high(LIGHT4)
break;
case 4:
output_high(LIGHT5)
}
}
else (BankState==D_OFF) {
switch (BankNo) {
case 0:
output_low(LIGHT1)
break;
case 1:
output_low(LIGHT2)
break;
case 2:
output_low(LIGHT3)
break;
case 3:
output_low(LIGHT4)
break;
case 4:
output_low(LIGHT5)
}
}
}

Display posts from previous:

	CCS Forum Index -> General CCS C Discussion	All times are GMT - 6 Hours Goto page Previous 1, 2
Page 2 of 2

You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum