0

I've been chipping away at a project for a long time now and I've found a serious bug.

Basically the user has an 8x8 grid, and each cell in this grid has a number of properties. I want to be able to save all of these properties as a 'profile' within the EEPROM, and previously this worked OK but now seems to have a few bugs.

The most prominent of which is that when I save a grid to a profile (Say profile 3), and then load the next profile (profile 4, in this case) the far right column of profile 3 appears in the far left column of profile 4. LikeWise, saving a filled in grid to profile 1, and then saving an empty grid to profile 2 causes the RIGHT hand column of profile 1 to be cleared.

It used to work great in a previous version but there was completely different hardware in place supporting the GUI making it impossible to load older versions of the code and test how it was working then.

Here is the code where I save / load profiles


#include <EEPROM.h>
#include"EEPROMAnything.h"
const int profileSize = 224; // Actually everything is only half the size you thought (WTF but ok)


void saveProfile(int profileNum)                               // saves the complete current configuration of the system to memory under a profile for later restoration 
{
  tft.fillScreen(BGCOL);// Clear the screen. 
  tft.setTextSize(2); 
  setLine0();
  tft.print("Saving...");
  setLine1(); 
  tft.print(sizeof(saveConfiguration)/8); // Original
  tft.print(" Bytes");
  setLine2();
  tft.print("please wait");
//  tft.sendBuffer();

  for (int ROMRow = 0; ROMRow< 8; ROMRow++)    // Populate the ROM Structure
  {
    for (int ROMCol = 0; ROMCol < 8; ROMCol++)
    {
      saveConfiguration.ROMcellType[ROMRow][ROMCol]      = cellType[ROMRow][ROMCol];
      saveConfiguration.ROMindexVal[ROMRow][ROMCol]      = indexVal[ROMRow][ROMCol];      
      saveConfiguration.ROMMIDIVariable[ROMRow][ROMCol]  = MIDIVariable[ROMRow][ROMCol];
      saveConfiguration.ROMcellChannel[ROMRow][ROMCol]   = cellChannel[ROMRow][ROMCol];
      saveConfiguration.ROMorientation[ROMRow][ROMCol]   = orientation[ROMRow][ROMCol];
      saveConfiguration.ROMfaderGroundX[ROMRow][ROMCol]  = faderGroundX[ROMRow][ROMCol];
      saveConfiguration.ROMfaderGroundY[ROMRow][ROMCol]  = faderGroundY[ROMRow][ROMCol];
    }    
  }    

  EEPROM_writeAnything((profileNum*profileSize), saveConfiguration);

  delay(500);
  menuState = 1; // Go back to main menu
  menuLocatorY = 0; // Otherwise text will not show on next screen;
  updateScreen();
}
//{EEPROM_writeAnything(profileNum*profileSize, configuration);}




void loadProfile(int profileNum)                              // saves the complete current configuration of the system to memory under a profile for later restoration 
{
  tft.fillScreen(BGCOL);// Clear the screen. 
  tft.setTextSize(2); 
  setLine0();
  tft.print("Loading...");
  setLine1(); 
  tft.print(sizeof(saveConfiguration)/8);
  tft.print(" Bytes.");
  setLine2();
  tft.print("please wait");  

  initGrid(); // Clear any information regarding the current grid. Shouldn't really be neccesary but makes code safer.

  EEPROM_readAnything(profileNum*profileSize,saveConfiguration);
  for (int ROMRow = 0; ROMRow< 8; ROMRow++)    // Populate the ROM Structure
  {
    for (int ROMCol = 0; ROMCol < 8; ROMCol++)
    {
      cellType[ROMRow][ROMCol]      = saveConfiguration.ROMcellType[ROMRow][ROMCol];
      indexVal[ROMRow][ROMCol]      = saveConfiguration.ROMindexVal[ROMRow][ROMCol];      
      MIDIVariable[ROMRow][ROMCol]  = saveConfiguration.ROMMIDIVariable[ROMRow][ROMCol];
      cellChannel[ROMRow][ROMCol]   = saveConfiguration.ROMcellChannel[ROMRow][ROMCol];
      orientation[ROMRow][ROMCol]   = saveConfiguration.ROMorientation[ROMRow][ROMCol];
      faderGroundX[ROMRow][ROMCol]  = saveConfiguration.ROMfaderGroundX[ROMRow][ROMCol];
      faderGroundY[ROMRow][ROMCol]  = saveConfiguration.ROMfaderGroundY[ROMRow][ROMCol];

      /*
      if (cellType[ROMRow][ROMCol] == 3)
      {
        customValue = orientation[ROMRow][ROMCol];
        customMessage =   indexVal[ROMRow][ROMCol];
        MIDIVariable[ROMRow][ROMCol] = 0;   // Sets read fader value to 0 initially,   the customvalue variable is used to control physical orientation in this function
        customChannel = cellChannel[ROMRow][ROMCol];    // The MIDI channel the given cell is assigned to
        initType3(ROMRow,ROMCol);
      }
      */

    }    
  }   

    // Here we go back to the grid view
    delay(500); // Just so user can see message
    menuState = 2; // Go back to Grid View
    menuLocatorX = 0; 
    menuLocatorY = 0; // Otherwise text will not show on next screen;
    updateScreen();
}
//{EEPROM_readAnything((profileNum*profileSize), configuration);}


Here is the declaration of the struct I am using

//-------------------------------EEPROM STRUCTURE -------------------------------------------
struct config_t   // I think this will allow EEPROM saving made easy
{
  int ROMcellType[8][8];  // [Row][Column] States the type of input assigned to the cell.  Top left = 0,0 Row, Column    // Celltype 9 will be a reservation cell for the faders.
  int ROMindexVal[8][8];       // States the note value / Command  assigned to the cell
  int ROMMIDIVariable[8][8];   // Used to retain desired velocity / Current value of continuous input.
  int ROMcellChannel[8][8];    // The MIDI channel the given cell is assigned to
  int ROMorientation[8][8]; // Keeps track of the direction of orientation of large modules
  int ROMfaderGroundX[8][8]; // Keeps track of where the 'root' of the fader is
  int ROMfaderGroundY[8][8]; // Keeps track of where the 'root' of the fader is
} saveConfiguration;
//--------------------------------------------------------------------------------------------

IndexVal, cellType, etc are all global variables (I know, I know, global variable bad)

And last of all, the header file with my base level EEPROM functions

#include <EEPROM.h>
#include <Arduino.h>  // for type definitions

template <class T> int EEPROM_writeAnything(int ee, const T& value)
{
    const byte* p = (const byte*)(const void*)&value;
    unsigned int i;
    for (i = 0; i < sizeof(value); i++)
          EEPROM.write(ee++, *p++);
    return i;
}

template <class T> int EEPROM_readAnything(int ee, T& value)
{
    byte* p = (byte*)(void*)&value;
    unsigned int i;
    for (i = 0; i < sizeof(value); i++)
          *p++ = EEPROM.read(ee++);
    return i;
}

It's been a while since I wrote this code so I'm a little lost in it myself. I should probably add that if I save to profile 1, mess around testing the other profiles, then load profile 1 again it's always correct.

Also, the part where it prints sizeof(saveConfiguration) does return 224, and I've also removed the "/8" just to check that there was no rounding error. If I double the value of profileSize it simple halves the number of profiles I have available by spacing them out by the size of the struct in memory.

Please ignore the (if cellType == 3) section, this is used for a type of object that uses 3 consecutive cells and is a whole can of worms in itself. For the sake of this test I'm just using celltypes that are modules that use a single cell.

I feel like I really need to make some progress with this soon for the sake of my own mental wellbeing.

Anyone able to help?

Further info for the commenters: The board I am using is a PJRC teensy 3.5.

The /8 was to display the number of Integers being saved. I think when I originally wrote this code I mistakenly thought that the device was 8 bit, and estimated the profile size to be 448. I believe the EEPROM is 16 bits wide?

  • What board are you using? And why do you think you would want to divide the size by 8...? – Majenko Jan 27 at 13:25
  • EEPROMAnything can be replaced by EEPROM.put and EEPROM.get. – Jot Jan 27 at 13:32
  • I'll add that info. 2 Secs. – GigaJoules Jan 27 at 13:33
  • Ok so replacing with .put and .get works, but I get the exact same issue. – GigaJoules Jan 27 at 13:58
1

Depending if you are on an 8 bit or 32 bit Arduino your profile is either 896 or 1792 bytes in size. Not 224. (Why do you think /8 is an option?)

I am guessing that you are on a 32-bit Arduino since 1792/8 = 224.

1792 is more EEPROM than any normal Arduino has, anyway. Even if you were on an 8-bit Arduino and the profile size was 896 there would only be room for one in the EEPROM.

I think you need to re-think your strategy somewhat to either massively reduce the amount of data you are saving, or change where you are storing the data to.

  • So the teensy 3.5 is a 32 uController with 4096 bytes of EEPROM. As it stands, I could probably encode the X and Y position into a single variable as neither value can be > 8. I am considering including a larger external EEPROM in the final design but for now the first few profiles should still work OK, even if the profile size is worst case scenario, right? I can save stuff to the other profiles OK but I get the issue where the far right and left columns overwrite the a column inthe next / last profile respectively – GigaJoules Jan 27 at 13:45
  • I think the /8 comes from the code originally being written on an arduino mega? – GigaJoules Jan 27 at 13:50
  • I can see no place ever for a /8 regardless of what the code was written for. The size is the size. If you try working with an eighth of the size then yes things are going to get messed up. – Majenko Jan 27 at 13:56
  • 1
    If you are storing values < 256 then there is no need to use int - it's 4x larger than you need. Use uint8_t. – Majenko Jan 27 at 13:57
  • Yeah good shout, this is definitely an artifact from previously working with a smaller Ucontroller. – GigaJoules Jan 27 at 14:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.