Skip to content

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

Closed
@mstranne

Description

@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

Activity

PaulZC

PaulZC commented on May 4, 2021

@PaulZC
Collaborator

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

mstranne commented on May 5, 2021

@mstranne
Author

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

PaulZC commented on May 5, 2021

@PaulZC
Collaborator

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

mstranne commented on May 5, 2021

@mstranne
Author

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

PaulZC commented on May 5, 2021

@PaulZC
Collaborator

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

mstranne commented on May 5, 2021

@mstranne
Author

That would be awesome!

thanks!
Best wishes,
Marco

changed the title [-]TwoWire I2C_BUFFER_LENGTH overflow in pushRawData[/-] [+][Feature Request] Please add stop as an optional argument for pushRawData[/+] on May 5, 2021
added a commit that references this issue on May 8, 2021
f081f75
PaulZC

PaulZC commented on May 8, 2021

@PaulZC
Collaborator

Added is this commit.
Enjoy!
Paul

added a commit that references this issue on May 8, 2021
linked a pull request that will close this issuev2.0.7 #36on May 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      Participants

      @PaulZC@mstranne

      Issue actions

        [Feature Request] Please add stop as an optional argument for pushRawData · Issue #30 · sparkfun/SparkFun_u-blox_GNSS_Arduino_Library