 |
 |
| View previous topic :: View next topic |
| Author |
Message |
tcruise7771
Joined: 12 Apr 2013
Posts: 24
|
| Question about storing a variable into pic eeprom. |
 Posted: Sun Jun 09, 2013 2:24 pm |
 |
|
Hello,
I have PIC18F4550 (48 MHz) and PIC16F877A( 20MHz). I Have different codes running on both of them but basically they both depend on a variable that is used as a multiplier.
| Code: |
int16 multiplifier=1600;
at some point in the void main() {
avgV = adc_values * multiplifier
}
|
Now this multiplifier is something to be calibrated based on 5 different sensors and since every sensor is manufactured with different typical values this multiplifier changes from just a bit to like 10 times .
Now i able to change that with a press of a button
| Code: |
int but_A = input(PIN_D0); // Pin has 5V , when button is pressed it goes to 0V
if (!but_A)
{ multiplifier= multiplifier+10;
disp57Int16(x,y, multiplifier); // custom printf function for 128x64 display, x,y are the positions
delay_ms(1000);
}
|
Now this calibration will be kind of problematic cos it will have to be done every time the PIC is started.
Now i first thought of using write_program_eeprom(adress, 16bitvalue) , but that is limited only to being done 1000 times and them the PIC is bricked.
So the only thing i can do is have some code like that
| Code: |
int8 multiplifier;
int but_A = input(PIN_D0); // Pin has 5V , when button is pressed it goes to 0V
int but_B = input(PIN_D1);
void main()
{
multiplifier=read_EEPROM(0);
while(true)
{
// some long code, my program.. and then... and then
{
while (!but_A)
{
multiplifier++;
if (multiplifier > 254)
multiplifier = 1;
disp57Int16(10,10, multiplifier); // custom printf function for 128x64 display, x,y=pixel position
delay_ms(2000);
offdisp57Int16(10,10, multiplifier); //custom printf to clear the displayed value
delay_ms(500);
}
if (!but_B)
{
write_eeprom(0, multiplifier);
disp57Int16(10,10, multiplifier); // to print what have been written
delay_ms(5000); // long delay so it will not be written couple of times
}
}
}
}
|
I haven't tested yet the code yet on the hardware( i will do that tomorrow ) , but it compiles fine .
From what i think it will do is when it starts the void main() it will set the multiplifier to whatever the read_EEPROM(0); value is .
Then it will enter the while true loop.
If i have Buton A pressed it will increment multiplifier++ till the button is pressed ( 254 is precaution so there will be no overflow as the eeprom only allows int8 )
Then if i am happy with the result by pressing button B it will store whatever the multiplifier value is to the eeprom .
Now i have those questions : writing with write_eeprom() should be able to be done at least 100 000 times until the PIC eeprom is bricked, or i am wrong ? How many times the write_eeprom() can be used ? read_eeprom() should have no limit or at least way much more than 100 000 times?
if i want to have 16 bit variable in the eeprom I would have to write and read two int8 values to the eeprom (varhigh and varlow) and use them as MSB and LSB for the 16bit
| Code: |
int16 multiplifier;
// I would have to write two int8 values to the eeprom (varhigh and varlow).
int8 varhigh,varlow;
void main()
{
varhigh= read_eeprom(0);
varlow= read_eeprom(10);
multiplifier = make16(varhigh,varlow); // making 11111111 11111111 = 16bit
while (TRUE)
{
// some code for the button A to change multiplifier value like in the code above
disp57Int16(10,10, multiplifier); // to print the value on the display
// some code for button B to split multiplifier which is int16
// and put it in two int8 representing MSB and LSB
// How i can do that ? Is there any command ?
// Can i do the following ???
varhigh=(int16) multiplifier>>8; // this will bitshift to the right 8 positions
// making 1111 1111 1111 1111 to 0000 0000 1111 1111 , and since varhigh is int8
// varhigh will have value of 1111 1111 . This way i can get the MSB
// Now to get the LSB
int16 tempmult;
tempmult=(int16) multiplifier<<8; // now this will bitshift to the left with 8 positions
// 1111 1111 1111 1111 will become 1111 1111 0000 0000 because it is casted as int16
// so it will NOT become 1111 1111 1111 1111 0000 0000
varlow= (int16) tempmult>>8; // will bitshift 1111 1111 0000 0000 to 00000000 1111 1111
// and since varlow is int8 only 1111 1111 be written to varlow and this way i can get the LSB
// IS there any better way to do that ?
write_eeprom(0, varhigh); // to write MSB
write_eeprom(10, varlow); // to write LSB
}
}
|
Now this code compiles fine also and i will test it tomorrow and see what i get .
But i wanted more advanced users like Ttelmah and PCM Programmer to give me their opinions and advices on what i am doing wrong or i can do better |
|
 |
asmboy
Joined: 20 Nov 2007
Posts: 2128
Location: albany ny
|
|
| Posted: Sun Jun 09, 2013 2:41 pm |
|
|
| Code: |
unsigned int16 chksum;
chksum=0xC55C;
write_eeprom(0, varhigh);
chksum +=varhigh;
write_eeprom(1, varlow);
// to write LSB
chksum +=varlow;
write_eeprom(2, make8(chksum,0));// write check word in next 2 addrs
write_eeprom(3, make8(chksum,1));
|
now when you read BACK the data , you read and add to the chksum
and compare to what you stored in add 2,3
there is a one chance in 2^16 that you have bad data -
if the checksum is GOOD
and it gives great protection from accidentally treating random junk in EEPROM as if it was good stored data. |
|
|
tcruise7771
Joined: 12 Apr 2013
Posts: 24
|
|
| Posted: Sun Jun 09, 2013 3:46 pm |
|
|
| asmboy wrote: | | Code: |
unsigned int16 chksum;
chksum=0xC55C;
write_eeprom(0, varhigh);
chksum +=varhigh;
write_eeprom(1, varlow);
// to write LSB
chksum +=varlow;
write_eeprom(2, make8(chksum,0));// write check word in next 2 addrs
write_eeprom(3, make8(chksum,1));
|
|
You mean to add
| Code: |
unsigned int16 tempchksum;
varhigh= read_eeprom(0); // MSB 255
chksum +=varhigh; // 50548
varlow= read_eeprom(0); // LSB 255
chksum +=varlow; // 50803
tempchksum= make16((read_eeprom(2),read_eeprom(3);
if (chksum !=tempchksum)
{ glcd_text57(10, 35, text_error, 1, ON); // print error checksum missmatch message
}
|
I am sorry if i sound noob but by compare you mean something like the above ?
Or in the if (chksum !=tempchksum) {
start to write the varhigh and low again untill i get the chksum!=tempchksum. Should i also write the MSB and LSB of the chksum again ?
}
Another question also : i havent thought of make8()
but from what i see it would do exactly this
| Code: |
varhigh =make8(multiplifier,0); // would be equal to
varhigh=(int16) multiplifier>>8;
and
varlow=make8(multiplifier,1); would be equal to doing the following three lines
int16 tempmult;
tempmult=(int16) multiplifier<<8;
varlow= (int16) tempmult>>8; // to get the LSB |
Also if i dont do that chksum , what is the chance for an error. Since the 16bit value will always be shown on the display . For example if it started as 1500 and the user presses the button till it goes to 5000. Then he presses button B to store it . After that the value that is stored can be red again and displayed so the user will see if there is bad data ? Since this calibration will be done like only once in a month and is done manually by the user it wouldnt be a problem if it is done twice to get the values right ?
Also once a value is written corectly in the eeprom, will it always stay stored and red back correctly ? |
|
|
asmboy
Joined: 20 Nov 2007
Posts: 2128
Location: albany ny
|
|
| Posted: Sun Jun 09, 2013 8:04 pm |
|
|
good initiative you showed and almost perfect coding too.
but one more line needed.... You forgot to init the
checksum on read back.
| Code: |
chksum=0xC55C; // initialize the same as when written
varhigh= read_eeprom(0); // MSB 255
chksum +=varhigh; // 50548
varlow= read_eeprom(0); // LSB 255
chksum +=varlow; // 50803
if (chksum !=make16((read_eeprom(2),read_eeprom(3))
{ glcd_text57(10, 35, text_error, 1, ON)} ;
// YES pull the handle - BAD data failed test of checksum here - ; |
I added the requirement to init the checksum to the same zero pattern
that you missed in your coding - but that is it.
Does not matter if you use checksum or CRC for a check word -
to test validity there is still a 1 in 65536 chance of a random value satisfying the checksum.
BUT since after erasure or with a new chip - the value of an unwritten checkword in EEPROM will be 0xFFFF - you are insured against having your program recognize 'junk' data as a correctly written cal value.
This is a method i have used for years with blocks of calibration data
in EEPROM - and note: you can calculate that checksum over MANY memory addresses - not just the 2 in the example here-
and still only use 2 bytes of EEPROM for the check value.
( not just the 2 bytes in the primitive example.)
AND ---
if more desiring of high security - make the check word be a 32 bit int -
and roll the checksumming of MANY vars at a long series of EEPROM addresses and write a 32bit check - you won't go wrong.
And 4 bytes of check data will insure integrity of the whole damned EEPROM area with high confidence too.
edit: integrity in this context means certainty that data read from eeprom is valid- not that your MAIN() logic is ok.  |
|
|
tcruise7771
Joined: 12 Apr 2013
Posts: 24
|
|
| Posted: Sun Jun 09, 2013 11:43 pm |
|
|
asmboy i have initialised the checksum
| Code: |
unsigned int16 tempchksum
tempchksum= make16((read_eeprom(2),read_eeprom(3);
if (chksum !=tempchksum)
|
its just your code saves spafe for the tempchksum variable and will be processed faster
| Code: |
if (chksum !=make16((read_eeprom(2),read_eeprom(3))
|
Question i havent used EEPROM on this chips so far but they have been programmed with normal program data. Is the eeprom erased if i select in MPLAB - Programmer - Erase Flash Device ? I think it will only erase the program memory ? Or i have to make a small function with other button in my program code to erase the eeprom button c = write_eeprom(0,0x00) and write_eeprom(0,0x00)
You havent answered for my question for if the following commands are qual
| Code: |
varhigh =make8(multiplifier,0); // would be equal to
varhigh=(int16) multiplifier>>8;
and
varlow=make8(multiplifier,1); would be equal to doing the following three lines
int16 tempmult;
tempmult=(int16) multiplifier<<8;
varlow= (int16) tempmult>>8; // to get the LSB
|
. |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19515
|
|
| Posted: Mon Jun 10, 2013 1:34 am |
|
|
1) Hurrah!. Somebody actually thinking....
2) A full erase on a chip will erase the EEPROM. This is why most programmers have an option something like 'erase only as needed', or an option to 'save EEPROM'. If you have code protection enabled, then a full erase has to be used. If the EEPROM is not read protected, then it can be read, and written back (the second option). If the programmer does not do this, then you can do it manually. If code protection is not used, then page erases can be used, and the EEPROM can be left un-erased, if the programmer supports this.
3) You can save a little on the lives used, by reading first. If the byte in the EEPROM matches the 'new' value, then don't perform the write. Small, but can save a few 'lives'.
4) Provided this calibration is only changing infrequently, this is an almost 'ideal' job for the EEPROM. If you think about it even the 'worst case' write life of the EEPROM, if a byte was changed every hour, would give over 10 years life. 
5) There is a separate 'life' issue, which is that slowly, when other values are changed in the EEPROM, values will get corrupted. So if (for instance), you had two data blocks 20 bytes long, stored the first, and then kept changing all the bytes in the second ever hour, the values in the first could start to decay after only perhaps five years. However unless you are doing a _lot_ of writes to other bytes in the EEPROM, this one is unlikely to arise.
Best Wishes |
|
|
ckielstra
Joined: 18 Mar 2004
Posts: 3680
Location: The Netherlands
|
|
| Posted: Mon Jun 10, 2013 2:22 am |
|
|
No, the two pieces of code are not equal.
From the CCS manual on make8(): | Quote: | | Extracts the byte at offset from var. Same as: i8 = (((var >> (offset*8)) & 0xff) except it is done with a single byte move. |
There are a few important conceptual things that worry me: | Code: | | varhigh=(int16) multiplifier>>8; |
1) Cast is a very powerful instruction where you as a programmer tell the compiler to 'forget' the internal type rules and use your new rule. Sometimes this is the only way to make the program work as intended, but best practice is to use the cast instruction as little as possible. As an example. imagine in the future you change the variable from int8 to int32, then you have to check your whole program at all locations where a cast instruction is used. Easy to miss one line and create a hard to find bug.
2) Which part do you want to cast to int16?
From your code I can not see what you intended to do, adding a few extra '()' characters will make it clear to other programmers (like me) what your intention is. Also, not all compilers are the same so I like to make it extra clear to the compiler what my intention is. | Code: | varhigh = (int16) multiplifier>>8; // unclear what part will be cast to int16
Much clearer is:
varhigh = (int16) (multiplifier>>8); // The result after the calculation is changed to int16
varhigh = ((int16) multiplifier)>>8; // multiplifier is converted to int16 before performing the shift |
3) Why the int16?
multiplifier already is an int16 so here no cast is needed and the result is put into an int8, so again no int16 needed. In fact, casting the result to an int8 would make more sense (but will be done by the compiler automatically).
| Code: | int16 tempmult;
tempmult=(int16) multiplifier<<8;
varlow= (int16) tempmult>>8; // to get the LSB |
Yes, this will get you the LSB but inefficient and is not equal to: | Code: | | varlow = make8(multiplifier,1); |
This line will give you the MSB. To get the LSB you would have to use: | Code: | varlow = make8(multiplifier,0);
or:
varlow = multiplifier & 0xFF;
or:
varlow = (int8) multiplifier;
or even:
varlow = multiplifier; // not preferred option unless you add some good code comments. |
|
|
|
tcruise7771
Joined: 12 Apr 2013
Posts: 24
|
|
| Posted: Mon Jun 10, 2013 3:38 am |
|
|
Ttelmah,
thanks for the detailed information. I was very confused on when BAD DATA happens in EEPROM. I see i shouldn't worry that much about it since writing to the EEPROM will only be done for the calibration itself. Since calibration will be done only like once in a month or only couple of times in lifetime. The only thing done more frequently is reading the EEPROM once every time the program starts, since it will only start like maximum 5 times a day all should be ok. I have #fuses NOPROTECT, then the code is not protected right ? How i can i protect only my program ( the .hex file) without protecting the EEPROM ? Can this be done ?
Edit i just seen within the valid fuses for 18F4550 :
PROTECT - protect code from reads
NOCPD - data EEPROM not protected
CPD - data EEPROM protected
So i guess i need within the fuses PROTECT, NOCPD , right ?
ckielstra,
You are absolutely right, the codes you gave would give the same same result my code gives but more efficient and fast. It was very late when i thought of my code and i can see clearly i don't need to cast with (int16), but at that time i was sleepy and just wanted to make sure i will get the MSB and LSB. By equal I meant the same result. Thought your code will run times faster and also you are correct that casting should only be done if its needed as if you forget about it, it can mess up alot.
Thanks for the debug |
|
|
|
|
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
|
FAQ 
Search
Register
Profile 
Log in to check your private messages 
Log in
