CCS C Software and Maintenance Offers
FAQFAQ   FAQForum Help   FAQOfficial CCS Support   SearchSearch  RegisterRegister 

ProfileProfile   Log in to check your private messagesLog in to check your private messages   Log inLog in 

CCS does not monitor this forum on a regular basis.

Please do not post bug reports on this forum. Send them to support@ccsinfo.com
there must be a better way
Goto page Previous  1, 2
 
Post new topic   Reply to topic    CCS Forum Index -> General CCS C Discussion
View previous topic :: View next topic  
Author Message
Ttelmah
Guest
PostPosted: Tue Jan 03, 2006 4:50 am     Reply with quote
The 'other good way', is to buffer the serial transmission as well, using an interrupt driven routine. Then your interrupt code, transfers the data into the output buffer, and data will start sending right away, without delaying the rest of the code (printf is fairly slow for some functions, so there may still be problems).

Best Wishes
ckielstra



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

View user's profile Send private message

PostPosted: Tue Jan 03, 2006 5:08 am     Reply with quote
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

View user's profile Send private message Send e-mail

PostPosted: Tue Jan 03, 2006 5:48 am     Reply with quote
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)

View user's profile Send private message Send e-mail Visit poster's website

Rom usage cure - Arrays
PostPosted: Wed Jan 04, 2006 11:01 am     Reply with quote
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

View user's profile Send private message Send e-mail

PostPosted: Wed Jan 04, 2006 4:04 pm     Reply with quote
hillcraft, you are a champ Smile 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

View user's profile Send private message Send e-mail

PostPosted: Mon Jan 09, 2006 6:13 am     Reply with quote
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)

View user's profile Send private message Send e-mail Visit poster's website

A couple of things
PostPosted: Tue Jan 10, 2006 10:51 am     Reply with quote
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:   
Post new topic   Reply to topic    CCS Forum Index -> General CCS C Discussion All times are GMT - 6 Hours
Goto page Previous  1, 2
Page 2 of 2

 
Jump to:  
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


Powered by phpBB © 2001, 2005 phpBB Group