|
|
View previous topic :: View next topic |
Author |
Message |
hadeelqasaimeh
Joined: 05 Jan 2006 Posts: 105
|
save string in eeprom |
Posted: Tue Feb 05, 2008 7:05 pm |
|
|
i have a string contains phone number with 12 digit
i want to save it in the eeprom (address 0 to 11), any body have idea?
i try to do this:
read data:
Code: |
for ( i=0;i<12;i++)
keys[i] = read_eeprom(i);
|
where keys[i] is int, ok this work
now to write data:
Code: |
for(i=0;i<12;i++)
{
if (TX[i]== '9' )
write_eeprom(i,9);
else
if (TX[i]== '2')
write_eeprom(i,2);
else
if (TX[i]== '3')
write_eeprom(i,3);
else
if (TX[i]== '4')
write_eeprom(i,4);
else
if (TX[i]== '5')
write_eeprom(i,5);
else
if (TX[i]== '6')
write_eeprom(i,6);
else
if (TX[i]== '7')
write_eeprom(i,7);
else
if (TX[i]== '8')
write_eeprom(i,8);
else
if (TX[i]== '1')
write_eeprom(i,1);
else
if (TX[i]== '0')
write_eeprom(i,0);
} |
this work sometimes ,and do not other times
thank you |
|
|
D-Kens
Joined: 09 May 2005 Posts: 35 Location: Toulouse (France)
|
|
Posted: Wed Feb 06, 2008 2:03 am |
|
|
Ouch, was a ugly if() condition...
I guess TX[i] is a char, else you wouldn't do this operation. If so, then you can make your code clearer by remembering that you can transform a character from 0 to 9 to its value just by substracting the value of character '0' (which should be 48).
Code: | for (i=0 ; i<12 ; i++) { write_eeprom(i,(TX[i]-'0')) ; } |
Well, else it should work, according to me, if you keep in mind that you enter a string and that you then convert it to integers when pasting it to keys[i]...
Oh, by the way, do you really have 12 digits in your phone number ? Your code doesn't care about symbols ('.' or '-'), which could lead to weird results if you use 'em in your number. |
|
|
hadeelqasaimeh
Joined: 05 Jan 2006 Posts: 105
|
|
Posted: Wed Feb 06, 2008 8:44 pm |
|
|
thank you
i try it,it work just once!!actually this just a segment from the code, code supposed to receive data ,then get the string which include the number and save it in eeprom, sometimes it need this number so it will read it from eeprom again....but what occur that when i run the code first time it receive string correct and save it,when get another string it analyze correct but dont save the new phone number , if i reset it work again just once.
here is my whole code:
tx code: Code: |
#include <16f877a.h>
#fuses xt,NOWDT,NOLVP
#use delay(clock=4000000) //one instruction=1us
#use rs232(baud=9600, xmit=PIN_C6, rcv=PIN_C7,ERRORS)
int8 value;
void main() {
while(1) {
delay_ms(15000);
printf("*2#111111111111&\r\n");//rank1=30,rank2=36
delay_ms(15000);
printf("*2#222222222222&\r\n");//rank1=30,rank2=36
}
}//MAIN
|
rx code take the string check the command between * and # if 2 then save number in between # and &, i test this and work
rx code: Code: |
#include <16f877a.h>
#include <string.h>
#fuses xt,NOWDT,NOLVP
#use delay(clock=4000000)
#use rs232(baud=9600, xmit=PIN_C6, rcv=PIN_C7)
static char RX[78] ,MX[10] ,TX[12];
static char S1[3],S2[3],S3[3],S4[3],S5[3] ,S6[3];
static int1 jj ,gsm,report;
static int8 i,n,f ;
static int8 rank1,rank2 ,keys[12];
#int_rda
void serial_isr() {
RX[i++]=getch();
jj=1;
}
void main() {
enable_interrupts(GLOBAL);
enable_interrupts(INT_RDA);
while(TRUE)
{
if (jj==1) {
if(input(PIN_B0)) {
report=1;
gsm=0; }
else
if(input(PIN_B1)) {
report=0;
gsm=1;
}
delay_ms(100);
if((report==1) && (gsm == 0)) {
//read number from eeprom
//and send the recieved data to mobile
for ( i=0;i<12;i++)
keys[i] = read_eeprom(i);
printf("%u",keys[0]);
printf("%u",keys[1]);
printf("%u",keys[2]);
printf("%u",keys[3]);
printf("%u",keys[4]);
printf("%u",keys[5]);
printf("%u",keys[6]);
printf("%u",keys[7]);
printf("%u",keys[8]);
printf("%u",keys[9]);
printf("%u",keys[10]);
printf("%u",keys[11]);
printf("\r\n");
}
else
if ((gsm==1) && (report == 0)) {
/////////////read command and do it///////////////////////
n=0;
rank1=strcspn(RX, S1);//*
rank2=strcspn(RX, S2);//#
for(i=(rank1+1);i<(rank2);i++) {
MX[n]=RX[i];
n++;
}//for
n=0;
rank1=strcspn(RX, S2);//#
rank2=strcspn(RX, S5);//&
for(i=(rank1+1);i<(rank2);i++) {
TX[n]=RX[i];
n++;
}//for
strcpy(S6,"2");
if(!strcmp(MX, S6)) {
//get number and save it
for(i=0;i<12;i++)
{
write_eeprom(i,(TX[i] - '0'));
}
}//if gsm ==1
jj=0;
i=0;
gsm=0;
report=0;
}//if (jj==1)
}//while
}//MAIN
|
what do you think?[/code] |
|
|
ckielstra
Joined: 18 Mar 2004 Posts: 3680 Location: The Netherlands
|
|
Posted: Wed Feb 06, 2008 10:00 pm |
|
|
Quote: | what do you think? | Sorry to say so, but I get a headache from reading your code.
Some general suggestions:
- Fix the layout of your code. The indentation is now changing many times and there are a lot of unneeded empty lines making reading difficult.
- Try to avoid global variables. Sometimes globals have good use but in your code most can be changed into local variables which make for easier reading and better optimized code.
- Do not use the same variables in your main and interrupt routine unless you really mean so. Now variable 'i' will get corrupted.
- Try to use meaningful variable names. What is 'jj'?
- Initialise all variables at program start. Now many variables have random values and you are lucky it is even running the first time.
- S[1], S[2] and S[5] are used but never get a value.
- Add more and meaningful comments. For example at the end of the two lines for-loop you have the comment to mark the end of the loop. The loop is so small I can see this terminator belongs to the for loop, no need to place a comment there. What I want to know is why you do have a for-loop! For example add a text like 'sort telephone numbers in ascending order'.
I've read your program at least 3 times and I have no clue as to what your program is trying to do so I can't say anything about the other errors. |
|
|
D-Kens
Joined: 09 May 2005 Posts: 35 Location: Toulouse (France)
|
|
Posted: Thu Feb 07, 2008 2:30 am |
|
|
I agree with ckielstra : you don't give a value to S1, S2 and S5 anywhere in your code... If you try to print to screen the value of rank1 and rank2, you might get a surprise. You should also have conditions about what to do if your strcspn() returns an error : you might have received noisy signal, or some parasite, and not a valid message that you want to store in memory...
Ah, and use the ERRORS fuse also in the receiver code : I see you use it in the transmitter's definition of the RS232, why not in the receiver ?
Edit : I just had a closer look at your code... I assume "jj" is a flag to indicate you received a string from your transmitter. Then you should only set it to '1' after receiving a full instruction (which means when you read a '&' in your code), and not on every interrupt. Else you will set it on every character received on the RS232, and the rest of your main() will proceed incomplete messages... |
|
|
hadeelqasaimeh
Joined: 05 Jan 2006 Posts: 105
|
|
Posted: Thu Feb 07, 2008 7:47 am |
|
|
ok ,im not surprised that you cant understand my code that every one have his own way to write and understand codes,any way there is some missing lines from this code,i try to simplify it there is lines such as:
and others, about strcspn() its work correct, im sure, because use this segment in many real codes and it still work up to now.
yes D-Kens, jj is flag to indicate receiving string.
i will explain , code do nothing if there is no interrupt , when interrupt occur the buffer of 78 char will get, back to main it will check flag and find equal to one so analyze this string to get phone number and save it in eeprom, here is my problem
i think 16f877a cant do such lnog code!!!so itry to use 18f458 but it dont work at all so i go back to 16f877a
now i will try to add EERORS as D-Kens advise me
uh, also i will try to change i to other integer
ckielstra ,D-Kens thank you |
|
|
Wayne_
Joined: 10 Oct 2007 Posts: 681
|
|
Posted: Thu Feb 07, 2008 8:24 am |
|
|
I have taken the code and given it a quick tidy (acording to the way I program).
I have not yet tried to work out what the problem is but I have added code which should fix a few things.
See what you think.
Edited.
Code: |
#include <16f877a.h>
#include <string.h>
#fuses xt,NOWDT,NOLVP
#use delay(clock=4000000)
#use rs232(baud=9600, xmit=PIN_C6, rcv=PIN_C7)
static int1 jj;
static char RX[78];
static int8 i;
#int_rda
void serial_isr() {
RX[i++]=getch();
jj=true; // char recieved
}
void main() {
static char S1[3];
static int8 rank1,rank2 ,keys[12];
static char MX[10] ,TX[12];
static int1 gsm,report;
static int8 n,f ;
i = FALSE;
jj = FALSE;
enable_interrupts(GLOBAL);
enable_interrupts(INT_RDA);
while(TRUE) {
if (jj) {
report = input(PIN_B0);
gsm = input(PIN_B1);
delay_ms(100);
if(report && !gsm) {
//read number from eeprom
//and send the recieved data to mobile
for ( i=0;i<12;i++) {
printf("%u",read_eeprom(i));
}
printf("\r\n");
} else if (gsm && !report) {
/////////////read command and do it///////////////////////
strcpy(S1, "*");
rank1=strcspn(RX, S1); //*
strcpy(S1, "#");
rank2=strcspn(RX, S1); //#
n=0;
for(i = (rank1+1); i < (rank2); i++) {
MX[n++]=RX[i];
} //for
rank1 = rank2; //#
strcpy(S1, "&");
rank2 = strcspn(RX, S1);//&
n=0;
for(i=(rank1+1);i<(rank2);i++) {
TX[n++]=RX[i];
}//for
strcpy(S1,"2");
if(!strcmp(MX, S1)) {
//get number and save it
for(i=0;i<12;i++) {
write_eeprom(i,(TX[i] - '0'));
}
}//if gsm ==1
jj=FALSE;
i=0;
}//if (jj)
}//while
}//MAIN
|
|
|
|
ckielstra
Joined: 18 Mar 2004 Posts: 3680 Location: The Netherlands
|
|
Posted: Thu Feb 07, 2008 10:22 am |
|
|
Quote: | i think 16f877a cant do such lnog code!!!so itry to use 18f458 but it dont work at all so i go back to 16f877a Crying or Very sad | An expensive lesson I recently learned again: During development when you have the choice between a small and a larger processor _always_ choose the large one. When development is finished and you are going to sell millions of chips, only then look on the Microchip website for the cheapest processor just adequate for your job.
Code: | n=0;
strcpy(S1, "*");
rank1=strcspn(RX, S1); //*
strcpy(S1, "#");
rank2=strcspn(RX, S1); //#
for(i=(rank1+1);i<(rank2);i++) {
MX[n]=RX[i];
n++;
}//for | I finally figured out what you are doing here, you are extracting the command string which starts with a * and ends with a #. I didn't understand because in your first post the initialisation of S1 and S2 was missing. The use of strcspn() is very unusual and inefficient if you are searching for just 1 character.
Here is an more optimized and easier to read version`: Code: | // Extract the command string from received data.
// The command starts with '*' and ends with '# '.
rank1=strchr(RX, "*");
rank2=strchr(RX, "#");
if ((rank1 == 0) || (rank2 == 0))
{
// Error, command not found.
// TODO: Add error handling code.
}
else
{
// Command found, now extract
memcpy(MX, &RX[rank1+1], rank2 - rank1);
} |
Note that the command copied to the array RX is not terminated by a 0, i.e. it is not a string now. This conflicts a few lines down in your code where you are using strcmp() which does expect a zero terminated string! |
|
|
hadeelqasaimeh
Joined: 05 Jan 2006 Posts: 105
|
|
Posted: Thu Feb 07, 2008 6:59 pm |
|
|
ckielstra wrote: | .
- Do not use the same variables in your main and interrupt routine unless you really mean so. Now variable 'i' will get corrupted.
|
thank you changing 'i' to another new variable solve the problem,, also i add ERRORS to fuses of Rs232.
Wayne_ thank you , your code so tidy, i learn a lesson from you. |
|
|
Wayne_
Joined: 10 Oct 2007 Posts: 681
|
|
Posted: Fri Feb 08, 2008 2:30 am |
|
|
No problem and ckielstra code is worth considering as well. Especially using the strchr functions. |
|
|
|
|
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
|