 |
 |
| View previous topic :: View next topic |
| Author |
Message |
aodj
Joined: 20 Dec 2006
Posts: 40
Location: Reading, UK
|
| I2C writing errors |
 Posted: Wed Aug 08, 2007 2:19 am |
 |
|
I'm attempting to create a backup routine to store values from and then restore to my EEPROM, by using a 24LC01B external chip.
However, I'm getting erroneous 0xA0 and 0xA1 (the start and stop bits) being written to the memory, corrupting the data.
I currently have the following in place:
| Code: | void fCopyFrom(int8 v1)
{
int8 v2;
while(ext_eeprom_ready() == 1) // loops while waiting for 24LC01B to appear ready
{
}
if(ext_eeprom_ready() == 0)
{
v2 = read_ext_eeprom(v1);
fDelay(5); // simple delay routine
write_ee(v1, v2); // simple eeprom write wrapper
fDelay(2);
}
}
void fCopyTo(int8 v1)
{
int8 v2;
while(ext_eeprom_ready() == 1)
{
}
if(ext_eeprom_ready() == 0)
{
v2 = read_ee8(v1); // read_eeprom() wrapper
fDelay(2);
write_ext_eeprom(v1, v2);
fDelay(5);
}
}
void fBackup()
{
int8 v1, v2;
if(input(iDongle) == 1) // iDongle == write protect line on 24LC01B
{
bDP1 = 1;
for(v1 = 0;v1 < 64;v1++)
{
fCopyFrom(v1);
fDelay(5);
}
fLoadDefault(); // reloads defaults from EEPROM
fPointer(); // reassigns pointers
fClear(); // clears erroneous button pushes
bDP1 = 0; // sets decimal place LED
bSave = 0;
}
else // Write
{
bDP1 = 1;
for(v2 = 0;v2 < 64;v2++)
{
fCopyTo(v2);
fDelay(5);
}
bDP1 = 0;
bSave = 0;
}
} |
I'm at a loss for why I'm getting errors, so any help would be appreciated. I'm using compiler v4.016 and my PIC is a 16F917. |
|
 |
ckielstra
Joined: 18 Mar 2004
Posts: 3680
Location: The Netherlands
|
|
| Posted: Wed Aug 08, 2007 3:52 am |
|
|
From the function names I can see you are using one of the CCS provided drivers for the external eeprom.
- Which one?
- What is your compiler version? (so we can compare the driver file from that version)
In fCopyFrom: | Code: | | while(ext_eeprom_ready() == 1) // loops while waiting for 24LC01B to appear ready |
This doesn't make sense and most likely is an error. The comment makes sense but the code is doing exactly the opposite, waiting until the eeprom is _not_ ready.
Then I don't understand the next line | Code: | | if(ext_eeprom_ready() == 0) |
You just waited for the eeprom to become ready (or the other way around) and now you are testing again? You only got out of the loop when ext_eeprom_ready() returned 0, so testing the same value again looks at least superfluous.
Why are you doing the call to ext_eeprom_ready() anyway? The ext_eeprom read and srite functions are performing that test for you internally, there is no need for you to test it again.
I want to give you a design hint as well. It took me too much time to understand you code because of the names you assign to your variables and functions. For example naming a variable v1 or v2 is not giving any hint as to what the purpose of that variable is. Similar from the function names CopyTo and CopyFrom, the source and destination of the copy can not be guessed from the function name. Better suggestion is CopyInternalToExternalEeprom().
Another example of poor naming is the lineWhat the ... is happening here? Luckily a few line lower you added some comment | Code: | | bDP1 = 0; // sets decimal place LED |
In my code this would have looked like | Code: | | DecimalPlaceLed = ON; |
Much easier to understand, no comment is required. |
|
|
aodj
Joined: 20 Dec 2006
Posts: 40
Location: Reading, UK
|
|
| Posted: Wed Aug 08, 2007 4:16 am |
|
|
I'm using compiler version 4.016 and the 2401.c that comes with it, unmodified.
The reason I have a the ext_eeprom_ready tests is so that if the chip isn't ready to receive, it would just loop indefinately until it was ready at which point it would break the loop, test the state (a double test I guess, but as far as I know, there is no else statement for a while loop) and then perform the embedded code.
Looking at it now, the subroutines do check the state of ext_eeprom_ready, so it is superfluous. I have tried it without the lines subsequently and get no better code performance.
As for the functiona and variable names, they were like that when I received them. I've spent months attempting to reconstruct this code from someone elses files and methods, and I'm used to the way they've written them now. Was a pain to get to grips with in the beginning though. :( |
|
|
ckielstra
Joined: 18 Mar 2004
Posts: 3680
Location: The Netherlands
|
|
| Posted: Wed Aug 08, 2007 4:39 am |
|
|
Having to do modifications to someone else's code is always a difficult job. In this situation you are really unlucky as it looks like the code was written by a beginner.
I don't have v4.016 so I can not check the driver supplied with that version. Is there a special reason you want to use a v4.0xx compiler version? All releases between 4.001 and 4.030 are to be considered alpha releases, full of bugs and even the most basic programs did not work reliable. The last official stable release is v3.249 (still available for download when I last checked) and it seems like the most recent releases are getting stable as well (v4.040 and up).
Maybe your EEPROM problem has nothing to do with the v4.016 compiler you are using but that version is definately not good enough for a production release. Please try again with another software version like v3.249 or one of the recent releases. |
|
|
aodj
Joined: 20 Dec 2006
Posts: 40
Location: Reading, UK
|
|
| Posted: Wed Aug 08, 2007 5:04 am |
|
|
| ckielstra wrote: | | Having to do modifications to someone else's code is always a difficult job. In this situation you are really unlucky as it looks like the code was written by a beginner. |
Well the previous coder was straight out of University, as am I. Just the way it is.
| ckielstra wrote: | I don't have v4.016 so I can not check the driver supplied with that version. Is there a special reason you want to use a v4.0xx compiler version? All releases between 4.001 and 4.030 are to be considered alpha releases, full of bugs and even the most basic programs did not work reliable. The last official stable release is v3.249 (still available for download when I last checked) and it seems like the most recent releases are getting stable as well (v4.040 and up).
Maybe your EEPROM problem has nothing to do with the v4.016 compiler you are using but that version is definately not good enough for a production release. Please try again with another software version like v3.249 or one of the recent releases. |
My company doesn't want to pay for more compilers, when, as far as they can see, this one is fine. Until such a time as I can explicitely point to something and say "this is broken, I need the newest compiler to fix it" I'm stuck with v4.016 and it will simply be the version we use for the product, come rerelease. |
|
|
ckielstra
Joined: 18 Mar 2004
Posts: 3680
Location: The Netherlands
|
|
| Posted: Wed Aug 08, 2007 7:04 am |
|
|
| Quote: | | My company doesn't want to pay for more compilers, when, as far as they can see, this one is fine. Until such a time as I can explicitely point to something and say "this is broken, I need the newest compiler to fix it" I'm stuck with v4.016 and it will simply be the version we use for the product, come rerelease. |
Take my word for it, v4.016 is broken in many places (check the sticky thread on top of this forum). Save everybody a lot of time by using a proven stable version of the compiler. CCS has always two compiler versions ready for download; the newest but untested version, and an older version which is the last known stable version. When your company bought the compiler they should have downloaded both compiler versions. At the time of the v4.016 release the v3.0249 was (and still is) the last known stable version. If you don't have v3.249 contact CCS to send it to you.
Just for reference I compared the 2401.c file in my v4.038 against v3.249, they are identical.
In v4 the pointer arithmetic has changed to better comply with the C standards, many projects upgrading from a v3 compiler suffered problems here. Can you post your read_ee8() and write_ee() functions? |
|
|
aodj
Joined: 20 Dec 2006
Posts: 40
Location: Reading, UK
|
|
| Posted: Wed Aug 08, 2007 8:29 am |
|
|
| Code: | void write_ee(int8 v1, int8 v2) // Overloaded (int8 version)
{
write_eeprom(v1, v2); // location, data
fClear();
}
void write_ee(int8 v1, int16 v2) // Overloaded (int16 version)
{
write_ee(++v1, make8(v2, 0)); // location, data
write_ee(--v1, make8(v2, 1)); // location, data
fClear();
}
int8 read_ee8(int8 v1)
{
int8 v2;
v2 = read_eeprom(v1);
return v2;
}
int16 read_ee16(int8 v1)
{
int8 v2 = 0, v3 = 0;
int16 v4 = 0;
v2 = read_ee8(v1);
v3 = read_ee8(++v1);
v4 = make16(v2, v3);
return v4;
} |
|
|
|
ckielstra
Joined: 18 Mar 2004
Posts: 3680
Location: The Netherlands
|
|
| Posted: Wed Aug 08, 2007 9:03 am |
|
|
No pointers in your eeprom routines, so that isn't the problem.
Function overloading is new in v4, this makes me very suspicious. Try what happens when you remove the overload of the write_ee routines. |
|
|
Ttelmah
Guest
|
|
| Posted: Wed Aug 08, 2007 9:22 am |
|
|
My guess would be that overloading probably doesn't work properly in 4.016, and you are therefore writing two bytes, when you call it with a single byte.
Try without using the overloaded functions, and you may well find it'll work. The late 'teen' releases, generated workable code in most cases, if you did not use the new abilities (like overloading).
Seriously, the earliest compilers that started to actually produce working code for some of the 'new' features, were the .03x releases, and there are still problems in some parts.
Best Wishes |
|
|
aodj
Joined: 20 Dec 2006
Posts: 40
Location: Reading, UK
|
|
| Posted: Wed Aug 08, 2007 10:18 am |
|
|
| That seems to have solved the issue for the most part. I'm still getting the occasional erroneous reading, but no where near the amount I was getting before. |
|
|
|
|
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
