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 CCS Technical Support

How to improve this example ?

 
Post new topic   Reply to topic    CCS Forum Index -> General CCS C Discussion
View previous topic :: View next topic  
Author Message
piripic



Joined: 15 Jan 2008
Posts: 25

View user's profile Send private message

How to improve this example ?
PostPosted: Tue Jan 29, 2008 1:35 pm     Reply with quote

Hi, I am new on this great forum... first of all thanks to all for their valuable post. Sorry for my poor english, I am italian ..and I have help from a translator....
I would like to know if you think I can improve this small sample program.
If anyone has any idea or example I would be happy...thanks.


Code:
// This is an example about the use of timer interrupt to drive a 7 segment led
// display without flickering. The negative aspect is the waste of resources...
// It is possible to write numbers from 0 to 9 and letters from A to F (in ascii);
// also display of the DOT it is implemented by sending the relative ASCII character.
// Port_B is connected to the seven segment display in this order:
// B0 = A; B1 = B; B2 = C; B3 = D; B4 = E; B5 = F; B6 = G; B7 = DOT
// The lower nibble of Port_C is connected to the displays common chatode

// In this example, the waste of resources is also caused by calculating the
// voltage and use of the printf function.

#include <16F877.h>
#device adc=10
#fuses HS,NOWDT,NOPROTECT,BROWNOUT,NOLVP
#use delay(clock=8000000)
#use fast_io(B)
#use fast_io(C)

#include <ctype.h>

#define DIGIT 4  // Defines the number of digits used

                    //    0   1    2    3    4    5    6    7   8    9    A    B    C    D    E    F
BYTE CONST D_MAP[16] = {0x3f,0x6,0x5b,0x4f,0x66,0x6d,0x7d,0x7,0x7f,0x6f,0x77,0x7c,0x39,0x5e,0x79,0x71};

int8 i, j, Common, DigitArray[DIGIT], DigitArray2[DIGIT];  // too much global variables

#INT_TIMER2
void multiplexing_isr() {  // Switch the array value to Port_B at a Timer2 interrupt rate
   OUTPUT_C(0);   // to avoid some noise
   OUTPUT_B(D_MAP[DigitArray[i] & 0xf] | (DigitArray[i] & 0x80));  //if flag: dot=ON
   OUTPUT_C(Common & 0xf);
   Common<<=1;  i++;     // shift out the common signal to Port_C
   if(i > DIGIT-1) {
      i = 0;  Common = 1;  // restore the value (roll over)
   }
}


void array_putc(char c) {  //This function is used to assign values and filter the characters not viewable
  switch(c) {
     case '.':  // if dot I signal it by a flag on previous digit (in array)
        if(j < DIGIT-1)  DigitArray2[j+1] |= 0x80;
        break;
     default:
        if(isdigit(c)) {DigitArray2[j] = c - '0';  if(j)  j--;}
        else if(isxdigit(c)) {
           if(islower(c))  DigitArray2[j] = c - 'a' + 0xa;
           if(isupper(c))  DigitArray2[j] = c - 'A' + 0xa;
           if(j)  j--;
        }
  }
}


void array_copy(void) {  // Function for copy value from "prebuffer" to buffer
   int8 i;
   for(i=0;i<=3;++i) {
      DigitArray[i] = DigitArray2[i];
   }
}


void main() {            //   >>>>>> EXAMPLE <<<<<<
   int16 adc_value;
   float volts;

   i = 0; Common = 1;
   set_tris_B(0);      // all pin = output
   set_tris_C(0xf0);   // lower nibble = output
   setup_timer_2(T2_DIV_BY_16, 100, 15);  //Try yourself what is the best value
   set_timer2(0);
   enable_interrupts(INT_TIMER2);
   enable_interrupts(GLOBAL);

   setup_adc_ports(AN0);
   setup_adc(ADC_CLOCK_DIV_8);
   set_adc_channel(0);  delay_us(10);

   while(1) {
      adc_value = read_adc();
      volts = (float)(adc_value * 5) / 1023.0;
      j = DIGIT-1;  // Put index on MSB before any "loop call" of array_putc
      printf(array_putc,"%1.2f",volts);  // Printf formatting works well on this
      array_copy();  //copy from "prebuffer" to buffer (double buffer to avoid dot flikering)
      delay_ms(1);
   }
}
piripic



Joined: 15 Jan 2008
Posts: 25

View user's profile Send private message

is it really so bad ?
PostPosted: Wed Jan 30, 2008 9:41 am     Reply with quote

Maybe I should rewrite from scratch....

Some advice ?
Neutone



Joined: 08 Sep 2003
Posts: 839
Location: Houston

View user's profile Send private message

Re: is it really so bad ?
PostPosted: Wed Jan 30, 2008 11:48 am     Reply with quote

piripic wrote:
Maybe I should rewrite from scratch....

Some advice ?


What aspect were you wishing to improve. It looks like it works.
SET



Joined: 15 Nov 2005
Posts: 161
Location: Glasgow, UK

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

PostPosted: Wed Jan 30, 2008 3:16 pm     Reply with quote

Quote:
Code:
volts = (float)(adc_value * 5) / 1023.0;


I suppose this could be 'improved' by :

Code:
int32 volts;
volts = (adc_value << 2) + adc_value; // mult by 5
volts = volts >> 10; // div by 1024, not 1023.. close enough?


And the comment about 'too many globals' - yes better style is to have i and j as locals only.
Ttelmah
Guest







PostPosted: Wed Jan 30, 2008 3:44 pm     Reply with quote

Actually, division by 1024, is the _correct_ value for the PIC, not 1023!.
This is down to the internal structure of the ADC's. If you read Microchip's own application notes on the ADC's (AN546 in particular), you will find that the design used in the PIC, changes to reading the full '1023' value, one 'level' lower than most ADC desgns, so the full scale (5v), would potentially actually be a reading of 1024 (which the chip can never return). This means that the correct conversion needs to use /1024, rather than /1023. Effectively there are 1025 'levels', with the top two returning the same value. So using the rotation (or simply dividing by 1024, the compiler is smart enough to automatically substitute a rotation for this), is not only quicker, but better!....

Best Wishes
piripic



Joined: 15 Jan 2008
Posts: 25

View user's profile Send private message

PostPosted: Thu Jan 31, 2008 1:07 am     Reply with quote

I thank you for your advice....I try to change it as you said


Claudio
piripic



Joined: 15 Jan 2008
Posts: 25

View user's profile Send private message

PostPosted: Wed Feb 20, 2008 6:45 am     Reply with quote

Hi, I have a question... How can I pass to a function a pointer to a structure or union ? I have try in this manner but it don't work; and how can I access my array in a union via (by) pointer ?

Code:
//----------------------------------

union {
  int8 ee_image[20];          // EEPROM image, accessible by array or by struct member
                              //Example:  eeimage.ee_image[0]  ;  eeimage.bb.channel_number
  struct e_image {
    int8  channel_number;       // Selected channel number (0 to 7)
    int8  system_code;          // system code
    int8  device_address;       // the address of this device
    int8  target_address;       // target device address
    int8  digip_address;        // digipeater device address
    int8  baud_dte;             // dte baud rate
    int8  baud_radio;           // radio baud rate
    int16 preamble_cycle;       // number of preamble cycle (power save related)
//    int8  channel_flag;       //
    int8  broadcast:        1;  // broadcasting mode (the packet is for all radio modem)
    int8  ack_req:          1;  // requested acknowledge
    int8  echo_req:         1;  // echo mode
    int8  nak_req:          1;  // nak (not acknowledged) to dte requested
    int8  addr_dte:         1;  // address of the sender to dte
    int8  reverse_addr:     1;  // use reversed address for packet response
    int8  addr_from_dte:    1;  // address from dte (not from configuration)
    int8  use_cts:          1;  // use cts signal instead of rt485 (rs485 send)
    int16 buffer_lenght;        // the lenght of the data buffer
    int8  time_dtx;             // time between dte data stop and packet transmission
    int8  repetition_number;    // number of repetition if not acknowledge
    int8  pwsavoff;             // power save off time
    int8  pwsava;               // power save reactivation time
    int16 checksum;             // the checksum of the above data
  } bb;
} eeimage;

//---------------------------------------
// The function...

int16 checksum_ee_image(eeimage *ptr, int8 lenght) {
   int16 result;
   do {
        result += ptr->ee_image[0];     // <--- How do this ?
   } while (++ptr <= lenght);

   return result;
}

//-----------------------------------------
// The function call....

     temp16 = checksum_ee_image(&eeimage.ee_image[0],&eeimage.ee_image[0]+sizeof(eeimage-3));



Thanks in advance,
Claudio
piripic



Joined: 15 Jan 2008
Posts: 25

View user's profile Send private message

PostPosted: Wed Feb 20, 2008 8:01 am     Reply with quote

My error, I have not put the typedef before the union. I did not understand how to pass the union address to the function; I have to do it or it is "automatic" ?
Sorry but I am new in C ...thanks

Code:
typedef union {       // <--- typedef added
  int8 ee_image[20];          // EEPROM image, accessible by array or by struct member
                              //Example:  eeimage.ee_image[0]  ;  eeimage.bb.channel_number
  struct e_image {
    int8  channel_number;       // Selected channel number (0 to 7)
    int8  system_code;          // system code
    int8  device_address;       // the address of this device
    int8  target_address;       // target device address
    int8  digip_address;        // digipeater device address
    int8  baud_dte;             // dte baud rate
    int8  baud_radio;           // radio baud rate
    int16 preamble_cycle;       // number of preamble cycle (power save related)
//    int8  channel_flag;       //
    int8  broadcast:        1;  // broadcasting mode (the packet is for all radio modem)
    int8  ack_req:          1;  // requested acknowledge
    int8  echo_req:         1;  // echo mode
    int8  nak_req:          1;  // nak (not acknowledged) to dte requested
    int8  addr_dte:         1;  // address of the sender to dte
    int8  reverse_addr:     1;  // use reversed address for packet response
    int8  addr_from_dte:    1;  // address from dte (not from configuration)
    int8  use_cts:          1;  // use cts signal instead of rt485 (rs485 send)
    int16 buffer_lenght;        // the lenght of the data buffer
    int8  time_dtx;             // time between dte data stop and packet transmission
    int8  repetition_number;    // number of repetition if not acknowledge
    int8  pwsavoff;             // power save off time
    int8  pwsava;               // power save reactivation time
    int16 checksum;             // the checksum of the above data
  } bb;
} eeimage;


//--------------------------
int16 checksum_ee_image(eeimage *ptr, int8 lenght) {
   int16 result;
   do {
         result += ptr->ee_image[0];   // <--- is this correct ?
   } while(++ptr <= lenght);

   return result;
}



//----------- function call --------------- I don't know how to do this
     temp16 = checksum_ee_image(&eeimage.ee_image[0],&eeimage.ee_image[0]+sizeof(eeimage-3));

// how to pass the union address to the function ?



edit: If the union/struct is global can I access this "eeimage.ee_image[0]" everywhere ?
piripic



Joined: 15 Jan 2008
Posts: 25

View user's profile Send private message

PostPosted: Wed Feb 20, 2008 1:41 pm     Reply with quote

Finally, after 3 hours, I understand how to use union and struct (I hope).
Attaching my test.... if there 'a better way to do it, tell me, thanks.
I try tomorrow with pointer and function call

Claudio

Code:
#include <18F24K20.h>
#device adc=8
#FUSES NOWDT, HS, NOPROTECT, BROWNOUT, BORV27, NOPUT, NOCPD, STVREN
#FUSES NODEBUG, NOLVP, NOWRT, NOWRTD, IESO, FCMEN, NOPBADEN, WRTC
#FUSES NOWRTB, NOEBTR, NOEBTRB, NOCPB, LPT1OSC, MCLR
#use delay(clock=4000000)
#use rs232(baud=9600,parity=N,xmit=PIN_C6,rcv=PIN_C7,bits=8,errors)

union aa {
 int8 ee_image[6];
 struct {
  int8  d1;
  int8  d2;
  int8  d3;
  int8  d4;
  int16 d5;
 } bb;
};

union aa cc;
union aa *cc_ptr;
int8 i;

void main() {
   cc_ptr = &cc;
   cc.ee_image[0] = 240;
   cc.bb.d2 = 33;
   cc_ptr->ee_image[2] = 10;
   cc_ptr->bb.d4 = 255;
   cc_ptr->bb.d5 = 584;

   for(i = 0; i < 6; i++)
      printf("%u ", cc.ee_image[i]);
   while(1);
}
SET



Joined: 15 Nov 2005
Posts: 161
Location: Glasgow, UK

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

PostPosted: Thu Feb 21, 2008 6:44 am     Reply with quote

Quote:
Code:
int16 checksum_ee_image(eeimage *ptr, int8 lenght) {
   int16 result;
   do {
         result += ptr->ee_image[0];   // <--- is this correct ?
   } while(++ptr <= lenght);

   return result;
}


I dont think this will do what you need - 'ptr' will advance by the length of what it points to, but you want to advance by just 1 byte.

Something more like -

Code:
int16 checksum_ee_image(eeimage *ptr, int8 lenght) {
   int16 result;
   int8 *p_data = ptr->ee_image[0];   

   do {
         result += *ptr;
   } while(++ptr <= lenght);

   return result;
}
piripic



Joined: 15 Jan 2008
Posts: 25

View user's profile Send private message

PostPosted: Thu Feb 21, 2008 1:39 pm     Reply with quote

Thanks SET, you saved me a sure headache :-)
I have always programmed the PIC in assembly; it is hard for me to learn c
But .. Slowly ...

Claudio
piripic



Joined: 15 Jan 2008
Posts: 25

View user's profile Send private message

New question
PostPosted: Fri Feb 22, 2008 1:48 pm     Reply with quote

Hi, I have a doubt:
As shown in the example I have an array and a structure both within a union (for ease of access). As you see, the size is not the same for both.
In this case, index 0 of the array and the first member of the struct (header) are always in match or not (same address?) ? Thanks in advance,

Claudio


Code:
#define MAXPROTO 18   // Protocol space
#define MAXDATA 256   // Data space
#define MAXCHECK 3    // Checksum space

typedef union _protocol {
 int8  p_array[MAXPROTO+MAXDATA+MAXCHECK];
 struct proto {
  int8  header;          // packet header
  int8  proto_lenght;    // lenght of the protocol (from 10 to 18 bytes)
  int8  cosys;           // system code
  int8  frame_cnt;       // frame counter (for packet repetitions when ack fail)
//  int8  flag_pack;       // flag of the packet, see below
  int8  free1:       1;  // free bit flag
  int8  free2:       1;  // free bit flag
  int8  free3:       1;  // free bit flag
  int8  free4:       1;  // free bit flag
  int8  echo:        1;  // echo packet
  int8  broadcast:   1;  // broadcast (multicast) packet
  int8  ack_req:     1;  // packet with acknowledge request
  int8  req_resp:    1;  // request/response flag
  int8  digip_ptr;       // digipeater pointer/counter
  int16 data_lenght;     // lenght of the data
  int8  sender_addr;     // the address of the sender
  int8  dig_tar1;        // may be the target or the digipeater
  int8  dig_tar2;        // may be the target or the digipeater
  int8  dig_tar3;        // may be the target or the digipeater
  int8  dig_tar4;        // may be the target or the digipeater
  int8  dig_tar5;        // may be the target or the digipeater
  int8  dig_tar6;        // may be the target or the digipeater
  int8  dig_tar7;        // may be the target or the digipeater
  int8  dig_tar8;        // may be the target or the digipeater
  int8  dig_tar9;        // may be the target
 } b;
};
union _protocol protocol;
union _protocol *proto_ptr;
// EXAMPLE:  proto_ptr->p_array[0] ;  protocol.b.header
SET



Joined: 15 Nov 2005
Posts: 161
Location: Glasgow, UK

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

PostPosted: Fri Feb 22, 2008 1:54 pm     Reply with quote

I think the compiler will align the struct first member to be at the same address as the first element of the array.
Display posts from previous:   
Post new topic   Reply to topic    CCS Forum Index -> General CCS C Discussion All times are GMT - 6 Hours
Page 1 of 1

 
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