[Feature Request] Please add stop as an optional argument for pushRawData

[mstranne]

Hey,
I just switched from my self implemented version (still the old v1.18) to the "new" pushRawData.

Works out fine, however since I2C_BUFFER_LENGTH = 128 in the Wire lib, sending messages bigger than 128 byte leeds to buffer overflow when using _i2cPort->endTransmission(false) in the loop of the pushRawData function.

So some bigger rtcm messages where not send correctly in my case.
I changed to use _i2cPort->endTransmission(false) all 128 bytes and now its working fine.

Would be awesome if you could fix that. Nevertheless, great work!

Cheers,
Marco

[PaulZC]

Hi Marco (@mstranne ),

Unless I'm mistaken, pushRawData does already break long messages up into multiple Wire transmissions? The relevant section of code is here:

https://github.com/sparkfun/SparkFun_u-blox_GNSS_Arduino_Library/blob/master/src/SparkFun_u-blox_GNSS_Arduino_Library.cpp#L3407-L3436

You can set the i2cTransactionSize by calling setI2CTransactionSize

Best wishes,
Paul

[mstranne]

Hey,

Yes your code does spilt it up, however if you call _i2cPort->endTransmission(false) the Wire lib will not write the bytes on the i2c bus, but will just fill the write buffer of the TwoWire lib, which is 128 bytes and if this buffer is full, _i2cPort->write will not succeed after the fitst 128 bytes. (I2C_BUFFER_LENGTH is 128 in the TwoWire lib https://github.com/espressif/arduino-esp32/blob/7856de7a57420e494176c16c5138174fe2c1dad0/libraries/Wire/src/Wire.h#L34)

if you add :

if(bytesWritten == 0) {
  Serial.printf("some problem, error: %d ", _i2cPort->lastError());
}

directly after
size_t bytesWritten = _i2cPort->write(dataBytes, bytesToWrite); // Write the bytes

and send a message with more than 128 byte via pushRawData, you should get error 6, I2C_ERROR_MEMORY.

https://github.com/espressif/arduino-esp32/blob/7856de7a57420e494176c16c5138174fe2c1dad0/libraries/Wire/src/Wire.cpp#L231

best wishes,
Marco

[PaulZC]

Hi Marco,

Ah, OK. This is specific to the ESP32. You did not mention that before. I have not seen this issue on other platforms.

You said "I changed to use _i2cPort->endTransmission(false) all 128 bytes and now its working fine.".
Did you mean you changed line 3428 to if (_i2cPort->endTransmission() != 0) (i.e. removed the false )?

Thanks,
Paul

[mstranne]

oh I'm sorry, didnt think about that.

I did it like this:

        if(bytesWrittenTotal % 128 == 0) {
          if (_i2cPort->endTransmission() != 0) //128 byte reaced, send via i2c
            return (false);                     //Sensor did not ACK
        }
        else
          if (_i2cPort->endTransmission(false) != 0) //Send a restart command. Do not release bus.
            return (false);                          //Sensor did not ACK

however, here I would need to ensure i2cTransactionSize is set to 32/64 etc.

Cheers,
Marco

[PaulZC]

That's OK. I think there is a better way... I would add stop as an optional argument which defaults to false:

In the .h file:

boolean pushRawData(uint8_t *dataBytes, size_t numDataBytes, boolean stop = false);

Then, in the .cpp, change line 3428 to:

if (_i2cPort->endTransmission(stop) != 0)

For your ESP32 code, you can set i2cTransactionSize to 128 by calling setI2CTransactionSize(128);

You can then force pushRawData to send a stop, instead of a restart, every 128 bytes by adding , true); at the end of the call.

Let me know if this works for you.
Best wishes,
Paul

[mstranne]

That would be awesome!

thanks!
Best wishes,
Marco

[PaulZC]

Added is this commit.
Enjoy!
Paul