I2CHost to I2CGuest EEPROM writes might be skipped if PC sends data too quickly

[blahlicus]

@BlackCapCoder I was looking through the source code and noticed that it was possible for the host to drop bytes if the PC is sending data to the host quicker than the guest could ask for it.

Currently, ModI2CGuest pings the host every 50ms (as defined by PULL_RATE), every time that happens, a 28 byte package is sent from the host to the guest. If the PC sends packages at a rate quicker than 50ms, then packages are dropped in between due to this code:

          if (EEPROMPacketSize > 0)
          {
            EEPROMPacket[EEPROMPacketIndex] = (byte)Serial.read();
            EEPROMPacketIndex++;
            EEPROMPacketSize--;
          }
          if (EEPROMPacketSize <= 0) <---------problem starts from this line
          {
            pendingEEPROMUpdate = true;
            Comms.mode = 0;
          }

Whilst EEPROMUpdate is pending for the ping from I2CGuest, any further received packages would be dumped because EEPROMPacketSize is 0 and it would reset to mode 0.

I see two solutions to resolve this and I could implement it if you don't want to work on this, but I could really use your insight on this:

Solution 1: Change Comms.mode 6 so that bytes from the PC isn't removed from the buffer until the package was sent from host to guest, this would involve changing the code snippet above to this:

          if (EEPROMPacketSize <= 0 && pendingEEPROMUpdate == false)
          {
            pendingEEPROMUpdate = true;
            Comms.mode = 0;
          }

We will also have to add a Comms.mode =0; to here in the onReceive function:

  if (pendingEEPROMUpdate)
  {
    SetSubEEPROM();
    EEPROMPacketIndex = 2;
    SerialLoaderByteStatus = 0;
    pendingEEPROMUpdate = false;
    Comms.mode =0; <----------here
  }

Which would prevent Comms.mode from resetting to 0 until the package was completely received by the guest, thus prevent Comms from dumping bytes to mode 0.

This solution is easy to implement but it would restrict the bandwitch of editing EEPROM from host to guest to 560 bytes per second, thus, updating the 5120 EEPROM on the guest would take around ~9 seconds which is quite long.

Solution 2: Temporary disable the master/slave switching and have the master be set to the host for the duration of the transfer.

Comms.mode 6 would need to be changed (or we could make a mode 9) where it would have to define the total number of bytes of all the packages at the start as a short, then split it up into two bytes and send it over to the guest in one go.

Currently a package looks like this and sending 5120 bytes to the guest involves sending several of these packages to the guest:

byte0: start address byte A
byte1: start address byte B
byte2: package size
byte3: data
byte4: data
...

byte0 and byte1 are combined to form a short, byte2 only needs to be a byte because the max size for each package is 28 only.

I could create a Comms.mode 9 that has the following structure instead:

byte0: start address byte A
byte1: start address byte B
byte2: package size byte A
byte3: package size byte B
byte4: data
byte5: data
...

Then only the first package would contain the starting address and package size whilst the trailing packages would only contain data, this would be similar to how Comms.mode 8 works for editing the host's EEPROM.

The idea is to lock the guest from typing or updating other info so that all bytes received by the guest during that time would be correct data to be written to EEPROM.

The above should take less than a second to write the entire EEPROM on the guest side so it should be OK to temporarily disable the master/slave switching.

Do you have any insight on how we might be able to deal with this problem?

[BlackCapCoder]

Yeah, we definitely want solution 2; I can't imagine a use case where I would want a full EEPROM update while typing- I'd much rather have fast uploading than have the ability to type while doing so!

Suppose we implemented a passthrough mode where the host could request to be the master for a duration of n bytes. Then it might still be possible for the host to drop a few packages from the computer while we are still waiting for the guest's PULL_RATE timer to run out. This could be resolved with your solution 1, before we get to activate passthrough.

[BlackCapCoder]

Are you working on this? Otherwise I have time to look at this this weekend.

[blahlicus]

No, I had to deal with my masters' midterm this week so I didn't do anything, I had planned to do it over the weekend.

I did some brainstorming and thought that doing solution 1 and temporarily setting PULL_RATE to 1 to be the easiest to implement and quick enough for our purposes, this also doesn't interrupt keystrokes on the guest side.

Please do take a look if you can.