Go Down

Topic: Arduino Uno + 24LC256 + struct in an Array = corruption ??? (Read 241 times) previous topic - next topic

ianjeffery

Hi All,

Please help.

I have a struct that holds stepper motor positions ( address, front, back ).

Code: [Select]
typedef struct { int16_t address; int16_t front; int16_t back; } StepperConfig;

front and back are certain positions around "clock face" , such as f=100, b=1900 - for example.

I then have an array that holds a number of these positions .

Code: [Select]

#define INDEXCOUNT 16
StepperConfig config[INDEXCOUNT];


I have then used this web site to write the array , and some headers stuff to the external i2c eeprom

http://apexlogic.net/code-bank/arduino/eeprom-i2c-write-anything/

and this is the eeprom

https://www.ebay.co.uk/itm/AT24C256-Memory-Module-Serial-EEPROM-I2C-Interface-Data-Storage-Module-Arduino/232685714630

Here is the full code before i describe the problem....


eepromi2c.h
Code: [Select]

#include <Arduino.h>
#include <Wire.h>
#define DEVICE 0x50 //this is the device ID from the datasheet of the 24LC256

//in the normal write anything the eeaddress is incrimented after the writing of each byte. The Wire library does this behind the scenes.

template <class T> int eeWrite(int ee, const T& value)
{
 const byte* p = (const byte*)(const void*)&value;
 int i;
 Wire.beginTransmission(DEVICE);
 Wire.write((int)(ee >> 8)); // MSB
 Wire.write((int)(ee & 0xFF)); // LSB
 for (i = 0; i < sizeof(value); i++)
 Wire.write(*p++);
 Wire.endTransmission();
 return i;
}

template <class T> int eeRead(int ee, T& value)
{
 byte* p = (byte*)(void*)&value;
 int i;
 Wire.beginTransmission(DEVICE);
 Wire.write((int)(ee >> 8)); // MSB
 Wire.write((int)(ee & 0xFF)); // LSB
 Wire.endTransmission();
 Wire.requestFrom(DEVICE, sizeof(value));
 for (i = 0; i < sizeof(value); i++)
 if (Wire.available())
 *p++ = Wire.read();
 return i;
}


and the main unit

ForumHelpEEPROM.ino

Code: [Select]


#include "eepromi2c.h"
#include <Wire.h>

#define EEPROM_CONFIG_START_ADDRESS 32
#define INDEXCOUNT 16
#define paddingBytes 16

// define the structure
typedef struct { int16_t address; int16_t front; int16_t back; } StepperConfig;

StepperConfig data;

StepperConfig config[INDEXCOUNT];

void ConfigureDecoder()
{
 // this is home
 config[0].address = 200;
 config[0].front = 906;
 config[0].back = 2506;

 config[1].address = 201;
 config[1].front = 1700;
 config[1].back = 100;

 config[2].address = 202;
 config[2].front = 1900;
 config[2].back = 300;

 config[3].address = 203;
 config[3].front = 2100;
 config[3].back = 500;

 config[4].address = 204;
 config[4].front = 2300;
 config[4].back = 700;

 config[5].address = 205;
 config[5].front = 2500;
 config[5].back = 900;

 config[6].address = 206;
 config[6].front = 2700;
 config[6].back = 1100;

 config[7].address = 207;
 config[7].front = 2900;
 config[7].back = 1300;

 config[8].address = 208;
 config[8].front = 3100;
 config[8].back = 1500;

 config[9].address = 209;
 config[9].front = 100;
 config[9].back = 1700;

 config[10].address = 210;
 config[10].front = 300;
 config[10].back = 1900;

 config[11].address = 211;
 config[11].front = 500;
 config[11].back = 2100;

 // these are the nudge addresses
 config[12].address = 300;
 config[13].address = 301;
 config[14].address = 302;
 config[15].address = 303;
}


void writeEEPromHeader(byte startingAddress)
{
 char eepromHeader[12] = "CONFIG_1.0";

 byte numberOfIndexes = INDEXCOUNT;
 byte homeIndex = 2;
 int16_t steps = 25600;
 int16_t speed = 400;

 int eeHeaderAddress = 0;

 Serial.print(F("eepromHeader : "));
 Serial.println(eepromHeader);

 Serial.print("Size of eepromHeader : ");
 Serial.println(sizeof(eepromHeader));

 eeHeaderAddress = eeHeaderAddress + eeWrite(eeHeaderAddress, eepromHeader);
 delay(30);
 Serial.print(F("Next Address : "));
 Serial.println(eeHeaderAddress);

 Serial.print(F("EEProm Address numberOfIndexes: "));
 Serial.println(eeHeaderAddress);
 eeHeaderAddress = eeHeaderAddress + eeWrite(eeHeaderAddress, numberOfIndexes);
 delay(30);

 Serial.print(F("EEProm Address homeIndex: "));
 Serial.println(eeHeaderAddress);
 eeHeaderAddress = eeHeaderAddress + eeWrite(eeHeaderAddress, homeIndex);
 delay(30);

 Serial.print(F("EEProm Address steps: "));
 Serial.println(eeHeaderAddress);
 eeHeaderAddress = eeHeaderAddress + eeWrite(eeHeaderAddress, steps);
 delay(30);

 Serial.print(F("EEProm Address speed: "));
 Serial.println(eeHeaderAddress);
 eeHeaderAddress = eeHeaderAddress + eeWrite(eeHeaderAddress, speed);
 delay(30);

 Serial.println(F(""));
}

void readEEPromHeader(byte startingAddress)
{
 char eepromHeader[12] = "";

 byte numberOfIndexes = 0;
 byte homeIndex = 0;
 int16_t steps = 0;
 int16_t speed = 0;

 int eeHeaderAddress = 0;

 eeHeaderAddress = eeHeaderAddress + eeRead(eeHeaderAddress, eepromHeader);
 delay(30);

 Serial.print(F("eepromHeader : "));
 Serial.println(eepromHeader);

 eeHeaderAddress = eeHeaderAddress + eeRead(eeHeaderAddress, numberOfIndexes);
 delay(30);
 Serial.print(F("numberOfIndexes: "));
 Serial.println(numberOfIndexes);

 eeHeaderAddress = eeHeaderAddress + eeRead(eeHeaderAddress, homeIndex);
 delay(30);
 Serial.print(F("homeIndex: "));
 Serial.println(homeIndex);

 eeHeaderAddress = eeHeaderAddress + eeRead(eeHeaderAddress, steps);
 delay(30);
 Serial.print(F("steps: "));
 Serial.println(steps);

 eeHeaderAddress = eeHeaderAddress + eeRead(eeHeaderAddress, speed);
 delay(30);
 Serial.print(F("speed: "));
 Serial.println(speed);
}

void setup()
{
 Wire.begin();
 Serial.begin(9600);

 ConfigureDecoder();
 writeEEPromHeader(0);

 uint16_t eeAddress = EEPROM_CONFIG_START_ADDRESS;

 for (int i = 0; i < INDEXCOUNT; i++)
 {
 data = config[i];
 Serial.print("Writting [");
 Serial.print(i);
 Serial.println("] Decoder information.");

 Serial.print("Current EEPROM Location : ");
 Serial.println(eeAddress);

 // this returns the next memory location
 eeAddress = eeAddress + eeWrite(eeAddress, data) + paddingBytes;

 Serial.print("Size of Data : ");
 Serial.println(sizeof(data));

 Serial.print("Next EEPROM Location : ");
 Serial.println(eeAddress);

 Serial.println(F(""));
 delay(30);
 }

 delay(30);
}

// Add the main program code into the continuous loop() function
void loop()
{
  readEEPromHeader(0);

 uint16_t eeAddress = EEPROM_CONFIG_START_ADDRESS;

 for (int i = 0; i < INDEXCOUNT; i++)
 {

 Serial.print("Reading [");
 Serial.print(i);
 Serial.println("] Decoder information.");

 eeAddress = eeAddress + eeRead(eeAddress, config[i]) + paddingBytes;
 delay(30);

 Serial.println("Complete.");

 Serial.print("Decoder Information : ");
 Serial.println(i);
 Serial.print("Address : ");
 Serial.println(config[i].address);
 Serial.print("Front : ");
 Serial.println(config[i].front);
 Serial.print("Back : ");
 Serial.println(config[i].back);
 Serial.println("----------------------");
 }

 delay(10000);
}



now, if i set the "paddingBytes" define to 0 , I expect the code to write the header information at the start of the eeprom memory, then from address 32 on wards, store the config structs from the array, one  after an other sequentially.

HOWEVER, upon reading it back, the header is corrupted, and not all the config structs are written correctly - its as if there is corruption.

here is the log for when paddingBytes = 0

eepromHeader : CONFIG_1.0
Size of eepromHeader : 12
Next Address : 12
EEProm Address numberOfIndexes: 12
EEProm Address homeIndex: 13
EEProm Address steps: 14
EEProm Address speed: 16

Writting 0 Decoder information.
Current EEPROM Location : 32
Size of Data : 6
Next EEPROM Location : 38



etc etc

and when reading it back

eepromHeader : � �IG_1.0
numberOfIndexes: 16
homeIndex: 2
steps: 25600
speed: 400
Reading 0 Decoder information.
Complete.
Decoder Information : 0
Address : 200
Front : 906
Back : 2506
----------------------

etc etc

----------------------
Reading 5 Decoder information.
Complete.
Decoder Information : 5
Address : 205
Front : -1
Back : -1
----------------------


you can see the eepromHeader = � �IG_1.0

which is corrupted. This is when each struct is saved right next to each other, according to the next address returned from the eeprom saving code ( not mine ).


NOW, if i set the padding to 16 ( which means i add in 16 bytes of space between the config structs ) I get better results, but not always 100% correct

I have read that the compiler adds hidden padding between the fields in the struct, but i could not find a definitive answer...

Can anyone help please. Should I add padding, if so, how much ( i dont want to guess, i want to be precise ). Am i doing this right?

PaulS

What happens if you JUST write and read the header, without writing/reading any other data?

If that doesn't work, you need to look at how you are writing the header.

I don't understand the need for fixed spacing between the blocks of data. Padding, to me, means making each block take the same amount of space.
The art of getting good answers lies in asking good questions.

ianjeffery

Hi Paul,

If its just the header - no problems - no corruption.

if i write 1, 2, 3, 4 or 5 config array elements generally no issue, its when i start writing more than 6 i start to run in to issues.

As far as i can see, i'm not doing anything different to the examples, but i have not seen many with an array of struct.

I was adding the padding incase this was some how effecting the writing. to be honest, im at a loss....

could it me the timing between writes -> as its an i2c device do i need to pause more between write commands ?

PaulS

Quote
I was adding the padding incase this was some how effecting the writing. to be honest, im at a loss....
Since the padding didn't help, take it out.

I'm a real fan of the += operator. To me,
Code: [Select]
eeHeaderAddress += eeWrite(eeHeaderAddress, homeIndex);
is much easier to understand than
Code: [Select]
eeHeaderAddress = eeHeaderAddress + eeWrite(eeHeaderAddress, homeIndex);

I'm also a huge fan of curly braces after EVERY if and for statement, and of proper indentation.
Code: [Select]
for (i = 0; i < sizeof(value); i++)
 if (Wire.available())
 *p++ = Wire.read();

That just takes too long to figure out.

 
Code: [Select]
for (i = 0; i < sizeof(value); i++)
 {
    if (Wire.available())
    {
       *p++ = Wire.read();
    }
 }

Much easier to understand, and much easier to see the problem.

Suppose that the device doesn't respond nearly instantaneously to the request for data. What will be in the array pointed to be p when the for loop is done? NOT the n bytes you requested.

A for loop is a bad idea, here, because you don't know how long it will take for the required amount of data to arrive.

In pseudo code, I'd do something like:
Code: [Select]
askForData; // Use requestFrom...
set the amount of data read to 0;
while the required amount of data hasn't arrived:
   if there is something to read:
      read it, store it, and increment the pointer
      increment the amount of data read;
   end if
end while


Code: [Select]
const byte* p = (const byte*)(const void*)&value;
The double cast is useless. You want to treat value as an array of bytes, not an array of voids.

The art of getting good answers lies in asking good questions.

ianjeffery

Thanks Paul,

just for clarification, the eepromi2c.h unit was not written by myself, but i agree with what you have said.


I have been studying this thread - https://forum.arduino.cc/index.php?topic=405098.15

which seems to be a similar issue i have,

but what i don't understand is why the corruption of the very first part of the eeprom

PaulS

You are using code that has possible bugs, but you seem to be reluctant to change it because you didn't write it. Instead, you'll concentrate on looking for bugs in code you did write.

Is it possible that the bug is NOT in your code?

There is no harm in making the code you didn't write bulletproof, so you KNOW that that is not where the problem is.
The art of getting good answers lies in asking good questions.

cattledog

Quote
If its just the header - no problems - no corruption.
if i write 1, 2, 3, 4 or 5 config array elements generally no issue, its when i start writing more than 6 i start to run in to issues.
I believe that you are running into page boundary issues, as well as wire library buffer size issues, on the write, which are not properly handled by the library you are using. To me, this does not look like writing individual bytes which have no page issues, but rather is generating a block write with no consideration of page boundaries or wire library buffer size issues.

Code: [Select]
template <class T> int eeWrite(int ee, const T& value)
{
const byte* p = (const byte*)(const void*)&value;
int i;
Wire.beginTransmission(DEVICE);
Wire.write((int)(ee >> 8)); // MSB
Wire.write((int)(ee & 0xFF)); // LSB
for (i = 0; i < sizeof(value); i++)
Wire.write(*p++); //creates a block write sent with Wire.endTransmission
Wire.endTransmission();
return i;
}


i would strongly suggest that you use Jack Christensen's external i2c eeprom library-- extEEPROM.h

It is available from the library manager of the ide and is rock solid.

Go Up