Forum Rule: Always post complete source code & details to reproduce any issue!
Results 1 to 8 of 8

Thread: Wire library incorrectly reading last 2 bytes in page

  1. #1

    Wire library incorrectly reading last 2 bytes in page

    Hi guys, I've been using the i2c_t3 library for a while with no issue, but recently had to switch to Wire because of memory limitations. For some reason, upon switching libraries, the last two bytes of a page write/read operation return incorrectly.

    I'm not sure why exactly. Here's my write code:

    Code:
    void DataLooper::writeCommands(bool shouldWrite){
      int daddr = 0x50;
      //tracks page
      int numBytesWritten = 0;
      for(int buttonNumber = 0; buttonNumber < NUM_BUTTONS; buttonNumber++){
        for(int commandNum = 0; commandNum < NUMBER_USER_COMMANDS; commandNum++){
            //startbyte = number of bytes for all previous buttons commands, the number of bytes of all previous commands on current button, the number of bytes for all the commands in previous presets, a 32 byte offset for global config commands
            int startByte = (buttonNumber * NUMBER_USER_COMMANDS * BYTES_PER_COMMAND) +  commandNum * BYTES_PER_COMMAND + (State::presetsWritten * NUM_BUTTONS * NUMBER_USER_COMMANDS * BYTES_PER_COMMAND ) + GLOBAL_CONFIG_PAGE_OFFSET ;
            if(State::presetsWritten != 0){
              //each preset occupys 144 bytes of memory (12 buttons, 2 commands per button, 6 bytes per command), leaving 16 bytes left over after every preset. This advances the start byte to the next memory page.
              //while this is inefficient, it allows for faster operation and still 49 presets.
              startByte += (16 * State::presetsWritten);
            }
            for(int cmdByte = 0; cmdByte < BYTES_PER_COMMAND; cmdByte++){
              //grab the byte that we're going to write
              int newByte = buttons[buttonNumber].commands[commandNum].ee_storage.asBytes[cmdByte];
              //append the address
              int eeaddress = cmdByte + startByte;
    
              if(!shouldWrite){
                int checkByte = readByte(eeaddress);
                //Serial.print("stored byte: ");
                //Serial.print(checkByte);
                //Serial.print(" command byte: ");
                //Serial.println(newByte);
                if(checkByte != newByte){
                  writeCommands(true);
                  return;
                }
              }else{
                //if this is the start of a page
                if(numBytesWritten == 0){
                  Serial.println("new page");
                  Wire.beginTransmission(daddr);
                  Wire.write((int)(eeaddress >> 8)); // MSB
                  Wire.write((int)(eeaddress & 0xFF)); // LSB
                }
                Serial.print("writing byte:");
                Serial.print(newByte);
                Serial.print(" at: ");
                Serial.println(eeaddress);
                Wire.write(newByte);
                numBytesWritten+=1;
                if(numBytesWritten == 32){
                  numBytesWritten = 0;
                  Wire.endTransmission();
                  Serial.println("page written");
                  delay(100);
                }    
            }
          }
        }
      }
      //ends partial page writes at the end of preset
    
      if(shouldWrite && numBytesWritten != 0){
        for(int x = 0; x < 3; x++){
          Wire.write(State::presetColor.asBytes[x]);
          delay(100);
          numBytesWritten += 1;
          Serial.print("writing color byte: ");
          Serial.println(State::presetColor.asBytes[x]);
          Serial.print(" at: ");
          Serial.println(numBytesWritten);
    
        }
        Wire.endTransmission();
        delay(100);
      } else{
        int colorStartByte = (160 * State::presetsWritten) + GLOBAL_CONFIG_PAGE_OFFSET + 144;
        for(int x = 0 ; x < 3 ; x++){
          if(readByte(colorStartByte + x) != State::presetColor.asBytes[x]){
            Wire.beginTransmission(daddr);
            Wire.write((int)(colorStartByte + x >> 8)); // MSB
            Wire.write((int)(colorStartByte + x & 0xFF)); // LSB
            Wire.write(State::presetColor.asBytes[x]);
            delay(100);
            Wire.endTransmission();
          }
        }
      }
    }

  2. #2
    Senior Member+ defragster's Avatar
    Join Date
    Feb 2015
    Posts
    9,967
    Two other new posts about memory and arrays? Is this all related to the same issue?

    This snippet is the only code posted - but doesn't show the memory allocated for the arrays[] in use - and the other two posts at least one refers to crashing with array size increase?

    Reading data array needs valid pointer to properly allocated/sized space - if 2 or 3 of these posts are one problem then that could explain not finding expected data?

  3. #3
    I don't think it's the same issue; even when I'm within the memory limitations, this issue persists. For some reason swapping out the i2c_t3 library with Wire causes the issue without changing anything about the arrays. I actually managed to solve the problem by using v1 of i2c_t3, which is about 1/4 of the size, but it's still a curiosity. Here's the array creation:

    In the button class:

    DLCommand commands[NUMBER_COMMANDS];
    The DLCommand class
    Code:
    class DLCommand {
    
    	public:
                    DLCommand();
                    DLCommand(uint64_t command, DLled * _led, unsigned char _buttonNum, DLObserver * _dataLooper);
                    DLObserver * dataLooper;
                    DLled* led;
                    unsigned char * state;
                    bool toggle = false;
                    void sendSysEx(); 
                    void execute();
                    void checkLed(bool onOff);
                    void checkDLCommands();
                    ee_storage_typ ee_storage;
    };
    The ee_storage_typ class:
    Code:
    typedef union
    {
    		commands_typ commands;
    		uint8_t asBytes[BYTES_PER_COMMAND];
    		uint64_t the_big_blob;
    } ee_storage_typ;

  4. #4
    Senior Member+ defragster's Avatar
    Join Date
    Feb 2015
    Posts
    9,967
    My guess would be there is a low level problem that is made worse with the change - it is an extension of the same problem.

    Not looked at the third thread … seems it was another build system question?

    But it seems there is likely one underlying problem that needs to be fixed. Go to the simplest repro case where the problem exists and examine and debug that problem there. And keep the focus and discussion on a single area at a time - in a single thread. There is no problem with threads expanding to get to a solution - but distributing across multiple threads makes focus harder for help to be given.

    Use the desired libraries known to work and zero in on the first reproducible observed bad behavior to resolve that issue.

    For another person outside your code to examine this it would take a review of all the code to locate the suspect areas.

    Indications as noted … on the other thread … are array/pointer use resulting in data outside memory allocated for that purpose.

    It seems there is a union holding an array of bytes that is placed in an array. the i2c code is given a write pointer to some portion of that in addition to whatever sketch manipulations are used.

    The loss of the two bytes suggests those bytes are being placed outside the expected area where they are vulnerable to overwrite or where they are overwriting something else.

  5. #5
    Senior Member+ defragster's Avatar
    Join Date
    Feb 2015
    Posts
    9,967
    To make a repro case and observe expected or faulty behavior:

    Make the simplest sketch possible and execute the i2c command and validate the returned data.

    With a working library to a working device using the proper command the returned data should be as expected: The proper length and expected values stored in the correct memory locations.

  6. #6
    The third thread is PIO specific. Their latest build nearly doubles my memory usage calculation, but if I revert to an old build, everything seems to work fine. I'm not sure if that's my issue or theirs. Good suggestions...also point taken regarding multiple threads. I didn't want my question to be disjointed as there are multiple issues, but I get that it is confusing as we move forward in the discussion. If you're open to looking at the code, I'm happy to send you a PM with a link to a private bitbucket repo. I've been racking my brain on this one and can't seem to figure it out and am kind of out of people to ask.

  7. #7
    Senior Member+ defragster's Avatar
    Join Date
    Feb 2015
    Posts
    9,967
    3rd thread sounds PIO specific - happy to ignore that one

    Perhaps:: Starting with p#5 simple sketch would let you confirm the i2c device to operate as expected.

    Not sure what the device is and likely don't have one. But when it works there then make an array of the data receive as planned perhaps and do a set of 5-10 reads to array elements and verify they work.

    If that works then the i2c device and library are working - and that shows the problem in the existing code.
    >> If that fails then that is a simpler sketch to examine and understand the device and behavior.

  8. #8
    Quote Originally Posted by defragster View Post
    3rd thread sounds PIO specific - happy to ignore that one

    Perhaps:: Starting with p#5 simple sketch would let you confirm the i2c device to operate as expected.

    Not sure what the device is and likely don't have one. But when it works there then make an array of the data receive as planned perhaps and do a set of 5-10 reads to array elements and verify they work.

    If that works then the i2c device and library are working - and that shows the problem in the existing code.
    >> If that fails then that is a simpler sketch to examine and understand the device and behavior.
    Sounds good...I'll keep you posted.

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •