Skip to content

Wire: Max TX buffer length #1562

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Nov 26, 2021
Merged

Wire: Max TX buffer length #1562

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
83c622e
Wire: remove useless variable
ABOSTM Nov 24, 2021
6b1c62e
Wire: move TX and RX buffer index and length to uint16_t
ABOSTM Nov 26, 2021
2e9ce90
Wire: limit TX buffer size to avoid consuming too much stack
ABOSTM Nov 24, 2021
2e73fd2
Wire: remove useless check on null pointer
ABOSTM Nov 26, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 48 additions & 66 deletions libraries/Wire/src/Wire.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,7 @@ void TwoWire::begin(uint8_t address, bool generalCall)
rxBufferAllocated = 0;
resetRxBuffer();

txBufferIndex = 0;
txBufferLength = 0;
txDataSize = 0;
txAddress = 0;
txBuffer = nullptr;
txBufferAllocated = 0;
Expand Down Expand Up @@ -125,48 +124,43 @@ uint8_t TwoWire::requestFrom(uint8_t address, uint8_t quantity, uint32_t iaddres

if (_i2c.isMaster == 1) {
allocateRxBuffer(quantity);
// error if no memory block available to allocate the buffer
if (rxBuffer == nullptr) {
setWriteError();
} else {

if (isize > 0) {
// send internal address; this mode allows sending a repeated start to access
// some devices' internal registers. This function is executed by the hardware
// TWI module on other processors (for example Due's TWI_IADR and TWI_MMR registers)
if (isize > 0) {
// send internal address; this mode allows sending a repeated start to access
// some devices' internal registers. This function is executed by the hardware
// TWI module on other processors (for example Due's TWI_IADR and TWI_MMR registers)

beginTransmission(address);
beginTransmission(address);

// the maximum size of internal address is 3 bytes
if (isize > 3) {
isize = 3;
}
// the maximum size of internal address is 3 bytes
if (isize > 3) {
isize = 3;
}

// write internal register address - most significant byte first
while (isize-- > 0) {
write((uint8_t)(iaddress >> (isize * 8)));
}
endTransmission(false);
// write internal register address - most significant byte first
while (isize-- > 0) {
write((uint8_t)(iaddress >> (isize * 8)));
}
endTransmission(false);
}

// perform blocking read into buffer
// perform blocking read into buffer
#if defined(I2C_OTHER_FRAME)
if (sendStop == 0) {
_i2c.handle.XferOptions = I2C_OTHER_FRAME ;
} else {
_i2c.handle.XferOptions = I2C_OTHER_AND_LAST_FRAME;
}
if (sendStop == 0) {
_i2c.handle.XferOptions = I2C_OTHER_FRAME ;
} else {
_i2c.handle.XferOptions = I2C_OTHER_AND_LAST_FRAME;
}
#endif

if (I2C_OK == i2c_master_read(&_i2c, address << 1, rxBuffer, quantity)) {
read = quantity;
}
if (I2C_OK == i2c_master_read(&_i2c, address << 1, rxBuffer, quantity)) {
read = quantity;
}

// set rx buffer iterator vars
rxBufferIndex = 0;
rxBufferLength = read;
// set rx buffer iterator vars
rxBufferIndex = 0;
rxBufferLength = read;

}
}
return read;
}
Expand Down Expand Up @@ -202,9 +196,8 @@ void TwoWire::beginTransmission(uint8_t address)
transmitting = 1;
// set address of targeted slave
txAddress = address << 1;
// reset tx buffer iterator vars
txBufferIndex = 0;
txBufferLength = 0;
// reset tx data size
txDataSize = 0;
}

void TwoWire::beginTransmission(int address)
Expand Down Expand Up @@ -242,7 +235,7 @@ uint8_t TwoWire::endTransmission(uint8_t sendStop)

if (_i2c.isMaster == 1) {
// transmit buffer (blocking)
switch (i2c_master_write(&_i2c, txAddress, txBuffer, txBufferLength)) {
switch (i2c_master_write(&_i2c, txAddress, txBuffer, txDataSize)) {
case I2C_OK :
ret = 0; // Success
break;
Expand All @@ -266,9 +259,8 @@ uint8_t TwoWire::endTransmission(uint8_t sendStop)
// reset Tx buffer
resetTxBuffer();

// reset tx buffer iterator vars
txBufferIndex = 0;
txBufferLength = 0;
// reset tx buffer data size
txDataSize = 0;

// indicate that we are done transmitting
transmitting = 0;
Expand All @@ -292,17 +284,13 @@ size_t TwoWire::write(uint8_t data)
size_t ret = 1;
if (transmitting) {
// in master transmitter mode
allocateTxBuffer(txBufferLength + 1);
// error if no memory block available to allocate the buffer
if (txBuffer == nullptr) {
setWriteError();
if (allocateTxBuffer(txDataSize + 1) == 0) {
ret = 0;
} else {
// put byte in tx buffer
txBuffer[txBufferIndex] = data;
++txBufferIndex;
txBuffer[txDataSize] = data;
// update amount in buffer
txBufferLength = txBufferIndex;
txDataSize++;
}
} else {
// in slave send mode
Expand All @@ -327,17 +315,14 @@ size_t TwoWire::write(const uint8_t *data, size_t quantity)

if (transmitting) {
// in master transmitter mode
allocateTxBuffer(txBufferLength + quantity);
// error if no memory block available to allocate the buffer
if (txBuffer == nullptr) {
setWriteError();
if (allocateTxBuffer(txDataSize + quantity) == 0) {
ret = 0;
} else {
// put bytes in tx buffer
memcpy(&(txBuffer[txBufferIndex]), data, quantity);
txBufferIndex = txBufferIndex + quantity;
memcpy(&(txBuffer[txDataSize]), data, quantity);

// update amount in buffer
txBufferLength = txBufferIndex;
txDataSize += quantity;
}
} else {
// in slave send mode
Expand Down Expand Up @@ -397,8 +382,7 @@ void TwoWire::flush(void)
rxBufferIndex = 0;
rxBufferLength = 0;
resetRxBuffer();
txBufferIndex = 0;
txBufferLength = 0;
txDataSize = 0;
resetTxBuffer();
}

Expand All @@ -416,12 +400,7 @@ void TwoWire::onReceiveService(i2c_t *obj)
// i know this drops data, but it allows for slight stupidity
// meaning, they may not have read all the master requestFrom() data yet
if (TW->rxBufferIndex >= TW->rxBufferLength) {

TW->allocateRxBuffer(numBytes);
// error if no memory block available to allocate the buffer
if (TW->rxBuffer == nullptr) {
Error_Handler();
}

// copy twi rx buffer into local read buffer
// this enables new reads to happen in parallel
Expand All @@ -442,10 +421,9 @@ void TwoWire::onRequestService(i2c_t *obj)

// don't bother if user hasn't registered a callback
if (TW->user_onRequest) {
// reset tx buffer iterator vars
// reset tx data size
// !!! this will kill any pending pre-master sendTo() activity
TW->txBufferIndex = 0;
TW->txBufferLength = 0;
TW->txDataSize = 0;
// alert user program
TW->user_onRequest();
}
Expand Down Expand Up @@ -485,9 +463,12 @@ void TwoWire::allocateRxBuffer(size_t length)
}
}

inline void TwoWire::allocateTxBuffer(size_t length)
inline size_t TwoWire::allocateTxBuffer(size_t length)
{
if (txBufferAllocated < length) {
size_t ret = length;
if (length > WIRE_MAX_TX_BUFF_LENGTH) {
ret = 0;
} else if (txBufferAllocated < length) {
// By default we allocate BUFFER_LENGTH bytes. It is the min size of the buffer.
if (length < BUFFER_LENGTH) {
length = BUFFER_LENGTH;
Expand All @@ -500,6 +481,7 @@ inline void TwoWire::allocateTxBuffer(size_t length)
_Error_Handler("No enough memory! (%i)\n", length);
}
}
return ret;
}

/**
Expand Down
19 changes: 12 additions & 7 deletions libraries/Wire/src/Wire.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,23 +28,28 @@ extern "C" {
#include "utility/twi.h"
}

// Minimal buffer length. Buffers length will be increased when needed,
// but TX buffer is limited to a maximum to avoid too much stack consumption
// Note: Buffer length and max buffer length are limited by uin16_t type
#define BUFFER_LENGTH 32
#if !defined(WIRE_MAX_TX_BUFF_LENGTH)
#define WIRE_MAX_TX_BUFF_LENGTH 1024U
#endif

// WIRE_HAS_END means Wire has end()
#define WIRE_HAS_END 1

class TwoWire : public Stream {
private:
uint8_t *rxBuffer;
uint8_t rxBufferAllocated;
uint8_t rxBufferIndex;
uint8_t rxBufferLength;
uint16_t rxBufferAllocated;
uint16_t rxBufferIndex;
uint16_t rxBufferLength;

uint8_t txAddress;
uint8_t *txBuffer;
uint8_t txBufferAllocated;
uint8_t txBufferIndex;
uint8_t txBufferLength;
uint16_t txBufferAllocated;
uint16_t txDataSize;

uint8_t transmitting;

Expand All @@ -57,7 +62,7 @@ class TwoWire : public Stream {
static void onReceiveService(i2c_t *);

void allocateRxBuffer(size_t length);
void allocateTxBuffer(size_t length);
size_t allocateTxBuffer(size_t length);

void resetRxBuffer(void);
void resetTxBuffer(void);
Expand Down