This is some elegant-looking code, but could I have done this better?

This is a visual representation of 8 blocks of the eeprom and different "keys" that need to be written into them. Varying from the shortest I will have to the longest. The actual numbers oil change but the number of digits and how they are split into each of the eight memory blocks is visible.

0 255
1 255
2 255
3 255
4 255
5 255
6 255
7 255

11 22 33 44 55 66 77 88
111 22 33 44 55 66 77 88
111 222 33 44 55 66 77 88
111 222 333 44 55 66 77 88
111 222 333 444 55 66 77 88
111 222 333 444 555 66 77 88
111 222 333 444 555 666 77 88
111 222 333 444 555 666 777 88
111 222 333 444 555 666 777 888

This is the code to pharse them into blocks acceptable by eeprom. Could this have been done a better way?

I understand that I am technically losing a bit of memory space here. But the data format is difficult to work with at my skill level and doing it this way keeps the data in a uniform format as it always uses the same number of blocks which makes it much easier to code. Sometimes 8 blocks will be removed random places in the eeprom and I need to be able to find spots of eight blocks so that the eeprom will not become fragmented, which will be extremely difficult to work with.

int address = FindFirstAvailableEEPROMSetOf8(); //if returns 32767, handle block list being full

    //write uid to first available set of 8
    int block0;
    int block1;
    int block2;
    int block3;
    int block4;
    int block5;
    int block6;
    int block7;
    
    bankey

    if(bankey.length() == 16){
        block0 = bankey.substring(0,1);
        block1 = bankey.substring(2,3);
        block2 = bankey.substring(4,5);
        block3 = bankey.substring(6,7);
        block4 = bankey.substring(8,9);
        block5 = bankey.substring(10,11);
        block6 = bankey.substring(12,13);
        block7 = bankey.substring(14,15);
    }else if(bankey.length() == 17){
        block0 = bankey.substring(0,2);
        block1 = bankey.substring(3,4);
        block2 = bankey.substring(5,6);
        block3 = bankey.substring(7,8);
        block4 = bankey.substring(9,10);
        block5 = bankey.substring(11,12);
        block6 = bankey.substring(13,14);
        block7 = bankey.substring(15,16);
    }else if(bankey.length() == 18){
        block0 = bankey.substring(0,2);
        block1 = bankey.substring(3,5);
        block2 = bankey.substring(6,7);
        block3 = bankey.substring(8,9);
        block4 = bankey.substring(10,11);
        block5 = bankey.substring(12,13);
        block6 = bankey.substring(14,15);
        block7 = bankey.substring(16,17);
    }else if(bankey.length() == 19){
        block0 = bankey.substring(0,2);
        block1 = bankey.substring(3,5);
        block2 = bankey.substring(6,8);
        block3 = bankey.substring(9,10);
        block4 = bankey.substring(11,12);
        block5 = bankey.substring(13,14);
        block6 = bankey.substring(15,16);
        block7 = bankey.substring(17,18);
    }else if(bankey.length() == 20){
        block0 = bankey.substring(0,2);
        block1 = bankey.substring(3,5);
        block2 = bankey.substring(6,8);
        block3 = bankey.substring(9,11);
        block4 = bankey.substring(12,13);
        block5 = bankey.substring(14,15);
        block6 = bankey.substring(16,17);
        block7 = bankey.substring(8,19);
    }else if(bankey.length() == 21){
        block0 = bankey.substring(0,2);
        block1 = bankey.substring(3,5);
        block2 = bankey.substring(6,8);
        block3 = bankey.substring(9,11);
        block4 = bankey.substring(12,14);
        block5 = bankey.substring(15,16);
        block6 = bankey.substring(17,18);
        block7 = bankey.substring(19,20);
    }else if(bankey.length() == 22){
        block0 = bankey.substring(0,2);
        block1 = bankey.substring(3,5);
        block2 = bankey.substring(6,8);
        block3 = bankey.substring(9,11);
        block4 = bankey.substring(12,14);
        block5 = bankey.substring(15,17);
        block6 = bankey.substring(18,19);
        block7 = bankey.substring(20,21);
    }else if(bankey.length() == 23){
        block0 = bankey.substring(0,2);
        block1 = bankey.substring(3,5);
        block2 = bankey.substring(6,8);
        block3 = bankey.substring(9,11);
        block4 = bankey.substring(12,14);
        block5 = bankey.substring(15,17);
        block6 = bankey.substring(18,20);
        block7 = bankey.substring(21,22);
    }else if(bankey.length() == 24){
        block0 = bankey.substring(0,2);
        block1 = bankey.substring(3,5);
        block2 = bankey.substring(6,8);
        block3 = bankey.substring(9,11);
        block4 = bankey.substring(12,14);
        block5 = bankey.substring(15,17);
        block6 = bankey.substring(18,20);
        block7 = bankey.substring(21,23);
    }

    EEPROM.write(EEPROMIndex, block0);
    EEPROM.write(EEPROMIndex + 1, block1);
    EEPROM.write(EEPROMIndex + 2, block2);
    EEPROM.write(EEPROMIndex + 3, block3);
    EEPROM.write(EEPROMIndex + 4, block4);
    EEPROM.write(EEPROMIndex + 5, block5);
    EEPROM.write(EEPROMIndex + 6, block6);
    EEPROM.write(EEPROMIndex + 7, block7);
    return;

I get extra points because I wrote all of this on my phone. LOL

It looks more repetitive than elegant. I can't figure out what it's doing. What is bankey?

birddseedd: I get extra points because I wrote all of this on my phone. LOL

No, you don't. It would if it would be a function that we could drop in an empty sketch and it would compile :)

And every time you use String (capital S) we will deduct a point 8) Heavy use of String manipulation (specifically concatenations) might eventually result in unexpected behaviour of your code.

Every time that you're inclined to number the same variable (block0..block7), use arrays. It will simplify your code significantly.

If this is related to your RFID project, why do you have text? UIDs are byte arrays and you can store them directly in your EEPROM. Where does that text come from?

    int block0;
    int block1;
    int block2;
    int block3;
    int block4;
    int block5;

Any time you start using variable names like xxx1, xxx2, xxx3 - it probably means you should have used an array.

I think you try to solve a homebrewn problem.

Your bankey String gets probably filled by concatenation of the decimal representation of 8 bytes of a uid?

You can not reverse that operation, if some of the bytes are below 100 and you did not account for that. The different length of bankey you try to decode points in that direction.

Just use the uid and .put it to the EEPROM.

EEPROM.write(EEPROMIndex, block0);
    EEPROM.write(EEPROMIndex + 1, block1);
    EEPROM.write(EEPROMIndex + 2, block2);
    EEPROM.write(EEPROMIndex + 3, block3);
    EEPROM.write(EEPROMIndex + 4, block4);
    EEPROM.write(EEPROMIndex + 5, block5);
    EEPROM.write(EEPROMIndex + 6, block6);
    EEPROM.write(EEPROMIndex + 7, block7);

Is rubbish.
If you have an array of your block variable then you can write this as:-

for(int i = 0; i <8;i++){
     EEPROM.write(EEPROMIndex + i, block[i]);
}

As others have said using Strings ( capital S ) is criminally insane.

Grumpy_Mike:
If you have an array of your block variable then you can write this as:-

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

EEPROM.write(EEPROMIndex + i, block[i]);
}

Oops. It’s an array of ints and will loose the high part of the values
you increment the address by one for each step.

EEPROM.put(EEPROMIndex, block);

Looks better to me.

ChrisTenone, Yes its a little bit of a bruit force way of doing it. i'm new. i can clean it up as i learn more. mainly how to work with byte data.

Sterretje, Yea, the whole thing seems botched now that i try and compile it. :( no points. an array would have been better but i eliminated the variables completely. but i still can't convert the data. yes its for the RFID. I still can't figure out how to work with byte data. this would be easier if i could figure it out but every time i try to do anything with it i get complaints that its not the right data type.

westfw, yes

whandall, i believe you are correct, its a decimal version of the byte data. i'm still trying to figure out how to not have to do that.

Grumpy mike, in your for loop, "block*" becomes "block1, block2, block3," and so on because of the "[]"? if so that's a great thing to know.* Whandall, * "EEPROM.put(EEPROMIndex, block);"* _ stairs blankly at code_

I might have to learn how to just work with byte data. nothing i try works. *note some lines are removed to stay below 9k characters

invalid cast from type ‘String’ to type ‘char’

    if(bankey.length() == 16){
        EEPROM.write(EEPROMIndex, (char)bankey.substring(0,1));
        EEPROM.write(EEPROMIndex + 1, (char)bankey.substring(2,3));
        EEPROM.write(EEPROMIndex + 2, (char)bankey.substring(4,5));
        EEPROM.write(EEPROMIndex + 3, (char)bankey.substring(6,7));
        EEPROM.write(EEPROMIndex + 4, (char)bankey.substring(8,9));
        EEPROM.write(EEPROMIndex + 5, (char)bankey.substring(10,11));
        EEPROM.write(EEPROMIndex + 6, (char)bankey.substring(12,13));
        EEPROM.write(EEPROMIndex + 7, (char)bankey.substring(14,15));
    }else if(bankey.length() == 17){
        EEPROM.write(EEPROMIndex, (char)bankey.substring(0,2));
        EEPROM.write(EEPROMIndex + 7, (char)bankey.substring(15,16));
    }else if(bankey.length() == 18){
        EEPROM.write(EEPROMIndex, (char)bankey.substring(0,2));
        EEPROM.write(EEPROMIndex + 7, (char)bankey.substring(16,17));
    }else if(bankey.length() == 19){
        EEPROM.write(EEPROMIndex, (char)bankey.substring(0,2));
        EEPROM.write(EEPROMIndex + 7, (char)bankey.substring(17,18));
    }else if(bankey.length() == 20){
        EEPROM.write(EEPROMIndex, (char)bankey.substring(0,2));
        EEPROM.write(EEPROMIndex + 7, (char)bankey.substring(18,19));
    }else if(bankey.length() == 21){
        EEPROM.write(EEPROMIndex, (char)bankey.substring(0,2));
        EEPROM.write(EEPROMIndex + 7, (char)bankey.substring(19,20));
    }else if(bankey.length() == 22){
        EEPROM.write(EEPROMIndex, (char)bankey.substring(0,2));
        EEPROM.write(EEPROMIndex + 7, (char)bankey.substring(20,21));
    }else if(bankey.length() == 23){
        EEPROM.write(EEPROMIndex, (char)bankey.substring(0,2));
        EEPROM.write(EEPROMIndex + 7, (char)bankey.substring(21,22));
    }else if(bankey.length() == 24){
        EEPROM.write(EEPROMIndex, (char)bankey.substring(0,2));
        EEPROM.write(EEPROMIndex + 7, (char)bankey.substring(21,23));
    }

‘class String’ has no member named ‘subint’

invalid cast from type ‘String’ to type ‘int’

    if(bankey.length() == 16){
        EEPROM.write(EEPROMIndex, (int)bankey.substring(0,1));
        EEPROM.write(EEPROMIndex + 7, (int)bankey.substring(14,15));
    }else if(bankey.length() == 17){
        EEPROM.write(EEPROMIndex, (int)bankey.substring(0,2));
        EEPROM.write(EEPROMIndex + 7, (int)bankey.substring(15,16));
    }else if(bankey.length() == 18){
        EEPROM.write(EEPROMIndex, (int)bankey.substring(0,2));
        EEPROM.write(EEPROMIndex + 7, (int)bankey.substring(16,17));
    }else if(bankey.length() == 19){
        EEPROM.write(EEPROMIndex, (int)bankey.substring(0,2));
        EEPROM.write(EEPROMIndex + 7, (int)bankey.substring(17,18));
    }else if(bankey.length() == 20){
        EEPROM.write(EEPROMIndex, (int)bankey.substring(0,2));
        EEPROM.write(EEPROMIndex + 7, (int)bankey.substring(18,19));
    }else if(bankey.length() == 21){
        EEPROM.write(EEPROMIndex, (int)bankey.substring(0,2));
        EEPROM.write(EEPROMIndex + 7, (int)bankey.substring(19,20));
    }else if(bankey.length() == 22){
        EEPROM.write(EEPROMIndex, (int)bankey.substring(0,2));
        EEPROM.write(EEPROMIndex + 7, (int)bankey.substring(20,21));
    }else if(bankey.length() == 23){
        EEPROM.write(EEPROMIndex, (int)bankey.substring(0,2));
        EEPROM.write(EEPROMIndex + 7, (int)bankey.substring(21,22));
    }else if(bankey.length() == 24){
        EEPROM.write(EEPROMIndex, (int)bankey.substring(0,2));
        EEPROM.write(EEPROMIndex + 7, (int)bankey.substring(21,23));
    }

invalid cast from type ‘String’ to type ‘byte {aka unsigned char}’

    if(bankey.length() == 16){
        EEPROM.write(EEPROMIndex, (byte)bankey.substring(0,1));
        EEPROM.write(EEPROMIndex + 7, (byte)bankey.substring(14,15));
    }else if(bankey.length() == 17){
        EEPROM.write(EEPROMIndex, (byte)bankey.substring(0,2));
        EEPROM.write(EEPROMIndex + 7, (byte)bankey.substring(15,16));
    }else if(bankey.length() == 18){
        EEPROM.write(EEPROMIndex, (byte)bankey.substring(0,2));
        EEPROM.write(EEPROMIndex + 7, (byte)bankey.substring(16,17));
    }else if(bankey.length() == 19){
        EEPROM.write(EEPROMIndex, (byte)bankey.substring(0,2));
        EEPROM.write(EEPROMIndex + 7, (byte)bankey.substring(17,18));
    }else if(bankey.length() == 20){
        EEPROM.write(EEPROMIndex, (byte)bankey.substring(0,2));
        EEPROM.write(EEPROMIndex + 7, (byte)bankey.substring(18,19));
    }else if(bankey.length() == 21){
        EEPROM.write(EEPROMIndex, (byte)bankey.substring(0,2));
        EEPROM.write(EEPROMIndex + 7, (byte)bankey.substring(19,20));
    }else if(bankey.length() == 22){
        EEPROM.write(EEPROMIndex, (byte)bankey.substring(0,2));
        EEPROM.write(EEPROMIndex + 7, (byte)bankey.substring(20,21));
    }else if(bankey.length() == 23){
        EEPROM.write(EEPROMIndex, (byte)bankey.substring(0,2));
        EEPROM.write(EEPROMIndex + 7, (byte)bankey.substring(21,22));
    }else if(bankey.length() == 24){
        EEPROM.write(EEPROMIndex, (byte)bankey.substring(0,2));
        EEPROM.write(EEPROMIndex + 7, (byte)bankey.substring(21,23));
    }

no matching function for call to ‘EEPROMClass::write(int&, String)’

    if(bankey.length() == 16){
        EEPROM.write(EEPROMIndex, (String)bankey.substring(0,1));
        EEPROM.write(EEPROMIndex + 7, (String)bankey.substring(14,15));
    }else if(bankey.length() == 17){
        EEPROM.write(EEPROMIndex, (String)bankey.substring(0,2));
        EEPROM.write(EEPROMIndex + 7, (String)bankey.substring(15,16));
    }else if(bankey.length() == 18){
        EEPROM.write(EEPROMIndex, (String)bankey.substring(0,2));
        EEPROM.write(EEPROMIndex + 7, (String)bankey.substring(16,17));
    }else if(bankey.length() == 19){
        EEPROM.write(EEPROMIndex, (String)bankey.substring(0,2));
        EEPROM.write(EEPROMIndex + 7, (String)bankey.substring(17,18));
    }else if(bankey.length() == 20){
        EEPROM.write(EEPROMIndex, (String)bankey.substring(0,2));
        EEPROM.write(EEPROMIndex + 7, (String)bankey.substring(18,19));
    }else if(bankey.length() == 21){
        EEPROM.write(EEPROMIndex, (String)bankey.substring(0,2));
        EEPROM.write(EEPROMIndex + 7, (String)bankey.substring(19,20));
    }else if(bankey.length() == 22){
        EEPROM.write(EEPROMIndex, (String)bankey.substring(0,2));
        EEPROM.write(EEPROMIndex + 7, (String)bankey.substring(20,21));
    }else if(bankey.length() == 23){
        EEPROM.write(EEPROMIndex, (String)bankey.substring(0,2));
        EEPROM.write(EEPROMIndex + 7, (String)bankey.substring(21,22));
    }else if(bankey.length() == 24){
        EEPROM.write(EEPROMIndex, (String)bankey.substring(0,2));
        EEPROM.write(EEPROMIndex + 7, (String)bankey.substring(21,23));
    }

Show at least the part of your program that is defining and generating bankey
and give some examples for bankey content.

  String  bankey;
  if (txt == "100101979911610511897116101000000") {

    Serial.println("deactivate card used");

    //lightled(2); //flash blue, was for development
    //get key  from block 3
    readBlock(BanKeyBlock, readbackblock);//bankeyblock is an in == 3
    
    for (int j = 0; j < 16; j++){

    Serial.write(readbackblock[j]); //Serial.write() transmits the ASCII numbers as human readable characters to serial monitor

    bankey = bankey + (byte) readbackblock[j]; //adds the acii character to the string. the data ends up being "1121129999000000000000" It needs to be converted to "ppcc000000000000"

  }

//function

int readBlock(int blockNumber, byte arrayAddress[]) {

  int largestModulo4Number = blockNumber / 4 * 4;

  int trailerBlock = largestModulo4Number + 3; //determine trailer block for the sector


  /*****************************************authentication of the desired block for access***********************************************************/

  byte status = mfrc522.PCD_Authenticate(MFRC522::PICC_CMD_MF_AUTH_KEY_A, trailerBlock, & key, & (mfrc522.uid));

  //byte PCD_Authenticate(byte command, byte blockAddr, MIFARE_Key *key, Uid *uid);

  //this method is used to authenticate a certain block for writing or reading

  //command: See enumerations above -> PICC_CMD_MF_AUTH_KEY_A = 0x60 (=1100000),    // this command performs authentication with Key A

  //blockAddr is the number of the block from 0 to 15.

  //MIFARE_Key *key is a pointer to the MIFARE_Key struct defined above, this struct needs to be defined for each block. New cards have all A/B= FF FF FF FF FF FF

  //Uid *uid is a pointer to the UID struct that contains the user ID of the card.

  if (status != MFRC522::STATUS_OK) {

    Serial.print("PCD_Authenticate() failed (read): ");

    Serial.println(mfrc522.GetStatusCodeName(status));

    return 3; //return "3" as error message

  }

  //it appears the authentication needs to be made before every block read/write within a specific sector.

  //If a different sector is being authenticated access to the previous one is lost.


  /*****************************************reading a block***********************************************************/


  byte buffersize = 18; //we need to define a variable with the read buffer size, since the MIFARE_Read method below needs a pointer to the variable that contains the size... 

  status = mfrc522.MIFARE_Read(blockNumber, arrayAddress, & buffersize); //&buffersize is a pointer to the buffersize variable; MIFARE_Read requires a pointer instead of just a number

  if (status != MFRC522::STATUS_OK) {

    Serial.print("MIFARE_read() failed: ");

    Serial.println(mfrc522.GetStatusCodeName(status));

    return 4; //return "4" as error message

  }

  Serial.println("block was read");

}

You should drop all that converting/concatenating/splitting/deconverting, it will only work for very specific uids.

See an example how you can not figure out what the original values have been:

byte pattern[][16] = {
  {  1, 1, 11, },
  {  11, 1, 1, },
  {  1, 1, 11, },
};

void setup() {
  Serial.begin(250000);
  for (byte j = 0; j < sizeof(pattern) / sizeof(pattern[0]); j++) {
    Serial.print(j + 1);
    Serial.print(F(". = "));
    makeAndPrintBanKey(pattern[j]);
  }
}
void makeAndPrintBanKey(byte* fromPattern) {
  String bankey;

  for (int j = 0; j < 16; j++) {
    bankey = bankey + (byte) fromPattern[j];
  }
  Serial.println(bankey);
}
void loop() {}
1. = 11110000000000000
2. = 11110000000000000
3. = 11110000000000000

That's why i have 9 different versions and i check the length. the code you put, something like that would be much better, once i figure out how to work with binary data. Ill get there and rewrite the app to be a lot cleaner and not screw around with converting data.

Your method of conversion is not suited for the application you have.

Just copy around the 16 byte uid (memcpy), compare (memcmp) and put the whole 16 bytes to the EEPROM.

Are we there yet?

lastchancename: Are we there yet?

I got sick and went to bed. I got my method working with the exception that I seem to be losing leading zeros.

So if that cant be fixed ill have to figure out how to work with raw byte data and rewrite some of the app.

Although even when I'm working directly with the byte data type, I still don't understand how it won't delete leading zeros. I'm not trying to be a noob

NEW THREAD TITLE

Could I have done this better?

The better way is to work with byte data. Ill get it figured out

Could I have done this better?

nah. Everyone knows that you'll get a lot more response with a message that says "look how great my code it" than you will from a message that asks for help... Cunningham's Law

Re: This is some elegant-looking code, but could I have done this better?

I did ask for a better way. And have gotten some very good input. I have admitted that my code was kind of a brute force method and not the best way of doing it by far.