|
|
View previous topic :: View next topic |
Author |
Message |
piripic
Joined: 15 Jan 2008 Posts: 25
|
How to improve this example ? |
Posted: Tue Jan 29, 2008 1:35 pm |
|
|
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
|
is it really so bad ? |
Posted: Wed Jan 30, 2008 9:41 am |
|
|
Maybe I should rewrite from scratch....
Some advice ? |
|
|
Neutone
Joined: 08 Sep 2003 Posts: 839 Location: Houston
|
Re: is it really so bad ? |
Posted: Wed Jan 30, 2008 11:48 am |
|
|
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
|
|
Posted: Wed Jan 30, 2008 3:16 pm |
|
|
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
|
|
Posted: Wed Jan 30, 2008 3:44 pm |
|
|
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
|
|
Posted: Thu Jan 31, 2008 1:07 am |
|
|
I thank you for your advice....I try to change it as you said
Claudio |
|
|
piripic
Joined: 15 Jan 2008 Posts: 25
|
|
Posted: Wed Feb 20, 2008 6:45 am |
|
|
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
|
|
Posted: Wed Feb 20, 2008 8:01 am |
|
|
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
|
|
Posted: Wed Feb 20, 2008 1:41 pm |
|
|
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
|
|
Posted: Thu Feb 21, 2008 6:44 am |
|
|
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
|
|
Posted: Thu Feb 21, 2008 1:39 pm |
|
|
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
|
New question |
Posted: Fri Feb 22, 2008 1:48 pm |
|
|
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
|
|
Posted: Fri Feb 22, 2008 1:54 pm |
|
|
I think the compiler will align the struct first member to be at the same address as the first element of the array. |
|
|
|
|
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
|