Pages: [1]   Go Down
Author Topic: Wire library (TWI)  (Read 2293 times)
0 Members and 1 Guest are viewing this topic.
Earth
Offline Offline
Sr. Member
****
Karma: 14
Posts: 330
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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?
Logged

Global Moderator
Netherlands
Offline Offline
Shannon Member
*****
Karma: 217
Posts: 13718
In theory there is no difference between theory and practice, however in practice there are many...
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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:
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;
}
?

« Last Edit: March 08, 2013, 01:18:57 pm by robtillaart » Logged

Rob Tillaart

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

Global Moderator
Offline Offline
Brattain Member
*****
Karma: 485
Posts: 18800
Lua rocks!
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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

Wire.cpp:

Code:
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:
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:
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.
Logged


Earth
Offline Offline
Sr. Member
****
Karma: 14
Posts: 330
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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?
Logged

Global Moderator
Netherlands
Offline Offline
Shannon Member
*****
Karma: 217
Posts: 13718
In theory there is no difference between theory and practice, however in practice there are many...
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

@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 smiley-wink

@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 smiley-wink
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 ...
Logged

Rob Tillaart

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

Global Moderator
Offline Offline
Brattain Member
*****
Karma: 485
Posts: 18800
Lua rocks!
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

@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 smiley-wink

@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 smiley-wink
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. smiley-wink

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

I can't comment on the Due library, it might be written completely differently.
Logged


Johannesburg South Africa
Offline Offline
Newbie
*
Karma: 0
Posts: 12
It can only be attributed to Human Error
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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.
Logged

 .
  .
...

Global Moderator
Offline Offline
Brattain Member
*****
Karma: 485
Posts: 18800
Lua rocks!
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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
Logged


Pages: [1]   Go Up
Jump to: