Go Down

Topic: Wire library (TWI) (Read 2 times) previous topic - next topic

Collin80

The library is hard coded for a 32 byte buffer and 100KHz transfer mode. I want to write 64 byte EEPROM pages on a 24AA256 in one shot but I can't do that with a 32 byte buffer. Also, it'd be nice to be able to go to 400KHz fast mode (which that EEPROM chip does support). I could fork the library into my source code and not use the Arduino library but I'd rather the official library be changed instead if it can be helped. Shouldn't these things be configurable?

robtillaart

#1
Mar 08, 2013, 07:07 pm Last Edit: Mar 08, 2013, 07:18 pm by robtillaart Reason: 1
Good idea,
- I propose post the code here so it can be peer reviewed and tested,
- and lets discuss the (new) interface and how to keep it backwards compatible (important imho)
- if it is stable it can be pushed to googlecode / git / whereever

I'm currently using I2C for 5 devices (ao PCF8574) and if it could be 4x as fast would be great!

- update -
a quick look shows the 1.0.3 version has the 400Khz and even 1Mhz numbers in place. So the most important point is the interface.

The begin method has a slave address as interface so I think the constructor should have the main param

Code: [Select]
TwoWire(int speed=100, uint8_t buffersize=32)  // max bufsize 255 ? or do we need an uint16_t for that? (MEGA / DUE?)
{
  if (speed == 1000) _speed = 1000;   // _speed in KHz
  else if (speed == 400) _speed = 400; 
  else _speed = 100;

  _bufsize = buffersize;
}

?

Rob Tillaart

Nederlandse sectie - http://arduino.cc/forum/index.php/board,77.0.html -
(Please do not PM for private consultancy)

Nick Gammon

One problem with this request is that, in the existing library at least, the buffer is copied 5 times.

Wire.cpp:

Code: [Select]

uint8_t TwoWire::rxBuffer[BUFFER_LENGTH];
uint8_t TwoWire::rxBufferIndex = 0;
uint8_t TwoWire::rxBufferLength = 0;

uint8_t TwoWire::txAddress = 0;
uint8_t TwoWire::txBuffer[BUFFER_LENGTH];


Twi.c:

Code: [Select]

static uint8_t twi_masterBuffer[TWI_BUFFER_LENGTH];
static volatile uint8_t twi_masterBufferIndex;
static volatile uint8_t twi_masterBufferLength;

static uint8_t twi_txBuffer[TWI_BUFFER_LENGTH];
static volatile uint8_t twi_txBufferIndex;
static volatile uint8_t twi_txBufferLength;

static uint8_t twi_rxBuffer[TWI_BUFFER_LENGTH];


So a 32-byte buffer is already taking up 160 bytes (out of the 2048 you have on a Uno). To double that would take 320 bytes.

You only have to edit two defines to changes its length to suit you.

It would be nice if they had done a malloc to allocate the buffers in the first place, and taken the buffer length as an argument, but no, that wasn't done, and it might be a bit late now.




Quote

Code: [Select]

TwoWire(int speed=100, uint8_t buffersize=32)  // max bufsize 255 ? or do we need an uint16_t for that? (MEGA / DUE?)



Your proposal might work with the defaults taken like that. Personally I think 255 is probably enough, once you start using int arithmetic the code gets larger.
Please post technical questions on the forum, not by personal message. Thanks!

More info:
http://www.gammon.com.au/electronics

Collin80

And, I'm sorry... I should have mentioned that I'm using the Due. It is a lot more RAM and DMA so I'd really want to not only do 400Khz and use large buffers but also use DMA to send and receive. On the Due with it's 100KB of RAM it would be no problem to have 128 byte buffers * 5 which is still less than a kilobyte and thus less than 1% of the RAM.

The TWI library on the Due is already a bit different from the 1.0.3 AVR version. Should I have left this discussion in the Due forums?

robtillaart

@AdderD
If it affects primary the DUE, I think the DUE section would be better. Still for the UNO's under us this is interesting too ;)

@Nick
Never reviewed the details of the wire/twi code but if I see this I really think a rewrite could save a byte or two ;)
1st thought is why not reuse buffers on the 2 levels?
2nd using malloc could still be proposed (as long as we not free the buffer, or is free() fixed?)
3rd do we really need a separate RX and TX buffer?

food for thought ...
Rob Tillaart

Nederlandse sectie - http://arduino.cc/forum/index.php/board,77.0.html -
(Please do not PM for private consultancy)

Nick Gammon


@AdderD
If it affects primary the DUE, I think the DUE section would be better. Still for the UNO's under us this is interesting too ;)

@Nick
Never reviewed the details of the wire/twi code but if I see this I really think a rewrite could save a byte or two ;)
1st thought is why not reuse buffers on the 2 levels?
2nd using malloc could still be proposed (as long as we not free the buffer, or is free() fixed?)
3rd do we really need a separate RX and TX buffer?

food for thought ...



Free is fixed now, I think, and in any case you wouldn't normally free it. After all the existing library doesn't because they are static buffers. ;)

Surely one of those 5 buffers could go, at least.

I can't comment on the Due library, it might be written completely differently.
Please post technical questions on the forum, not by personal message. Thanks!

More info:
http://www.gammon.com.au/electronics

LocalgHost

There seems to be 2 buffer declarations ( in wire.h and twi.h ) booth are hard set to 32 bytes
should wire.h not get its buffer size from twi.h ( in its utilities folder ) ?

- i made my buffer 8 bytes because i only send a few bytes to configure a IIC led driver.

but I am still not sure why there is a buffer declaration in both twi.h and wire.h if they are both used for the i2C communication.
.
  .
...

Nick Gammon

Me neither. Nor am I sure why you get 5 lots of buffers.

If you want to raise the issue:

https://github.com/arduino/Arduino/issues
Please post technical questions on the forum, not by personal message. Thanks!

More info:
http://www.gammon.com.au/electronics

Go Up