Testato / SoftwareWire

Creates a software I2C/TWI bus on every pins
GNU General Public License v3.0
148 stars 33 forks source link

Won't work on Arduino Pro mini with SSD1306 display? #19

Closed supersjimmie closed 3 years ago

supersjimmie commented 5 years ago

When connecting the display to the regular Wire pins, it works fine. When I replace the Wire with myWire from this library and connect it to the specified pins, it does not work.

#include <SoftwareWire.h>
SoftwareWire myWire( A2, A3, true, true); // sdaPin, sclPin, pullups, detectClockStretch
#include <Adafruit_GFX.h>
#include <Adafruit_SSD1306.h>
#define SCREEN_WIDTH 128 // OLED display width, in pixels
#define SCREEN_HEIGHT 64 // OLED display height, in pixels
#define OLED_RESET     -1 // Reset pin # (or -1 if sharing Arduino reset pin)
Adafruit_SSD1306 display(SCREEN_WIDTH, SCREEN_HEIGHT, &myWire, OLED_RESET);

void setup() {
  myWire.begin();

  if(!display.begin(SSD1306_SWITCHCAPVCC, 0x3C)) {
    Serial.println(F("SSD1306 allocation failed"));
    for(;;); // Don't proceed, loop forever
  }

  // Show initial display buffer contents on the screen --
  // the library initializes this with an Adafruit splash screen.
  display.display();
  // Clear the buffer
  display.clearDisplay();
  display.setTextColor(WHITE);        // Draw white text
  display.setCursor(0,0);             // Start at top-left corner
  display.setTextSize(1);
  display.print("Start");
  display.display();
}

I tried several pins, like the digital 2+3 or analog A2+A3.

Koepel commented 5 years ago

Thank you for reporting this issue. Which Arduino board do you use ? The display.begin() calls by default the Wire.begin(). Can you add the test with display.begin() as in the examples ?

Of all I2C sensors and modules, those displays are the least according to the I2C standard. It is not a surprise that it does not work. Adafruit has probably adjusted their code to make it work for their display modules.

The u8g2 library has both hardware I2C and software I2C which are specifically written for those displays.

If the SoftwareWire was 100% compatible with the Arduino (hardware) Wire library, then it should work. I don't have a Adafruit display module to test the SoftwareWire library. I will keep this open for now and mark it as a bug.

supersjimmie commented 5 years ago

I use an Arduino Pro mini (3.3v). It's not the display.begin() but the Adafruit_SSD1306 display(SCREEN_WIDTH, SCREEN_HEIGHT, &myWire, OLED_RESET); where the used wire is defined. (in this case it's set to &myWire)

The constructor is: Adafruit_SSD1306(uint8_t w, uint8_t h, TwoWire *twi=&Wire, int8_t rst_pin=-1, uint32_t clkDuring=400000UL, uint32_t clkAfter=100000UL);

EDIT: In addition, the u8g2 sw_i2c won't be really useful for me, since I not only need the SoftwareWire for this display but also for a BME280 and CCS811. (the hardware i2c needs to be used as slave for communication with another device)

Koepel commented 5 years ago

Sorry, but I have no quick fix. You can try another software I2C library that is compatible with TwoWire. If possible, you could try a display with SPI-alike signals. The displays with I2C interface are known to cause trouble on the I2C bus.

When you have the Arduino Pro Mini 3.3V as a I2C Slave and use SoftwareWire for the sensors, and the display with SPI-alike signals, then everything is straightforward. Be careful with libraries that turn off the interrupts for some time, that makes the I2C Slave part unreliable.

I meant something else. You have a myWire.begin(); in the setup() function, but the examples have a display.begin() in setup() to test if the display is connected. In the library, in display.begin() is a call to Wire.begin() which is called by default (although that can be turned off with a parameter).

If it fits, you can use the u8g2 with its own software I2C for the display and also SoftwareWire for the sensors. There are three options for the pins: seperate SDA and SCL, shared SCL, or shared SDA and SCL. I think it might be possible to share SDA and SCL and have both software I2C busses on the same pins.

supersjimmie commented 5 years ago

Sorry, but I have no quick fix. You can try another software I2C library that is compatible with TwoWire.

I will take a look at that too.

If possible, you could try a display with SPI-alike signals. The displays with I2C interface are known to cause trouble on the I2C bus.

SPI won't be an option, beside the display, BME280 and CCS811, other devices will also be connected. About all pins will be occupied. Sharing software-i2c for display/bme/ccs and using the hardware i2c as slave to communicate to another device (esp8266) will be the only possibility that fits.

I meant something else. You have a myWire.begin(); in the setup() function, but the examples have a display.begin() in setup() to test if the display is connected. In the library, in display.begin() is a call to Wire.begin() which is called by default (although that can be turned off with a parameter).

No, display.begin() does not call wire.begin. It the wire defined in 3rd parameter in Adafruit_SSD1306 display(SCREEN_WIDTH, SCREEN_HEIGHT, &myWire, OLED_RESET);, so in this case it will use myWire.

If it fits, you can use the u8g2 with its own software I2C for the display and also SoftwareWire for the sensors. There are three options for the pins: seperate SDA and SCL, shared SCL, or shared SDA and SCL. I think it might be possible to share SDA and SCL and have both software I2C busses on the same pins.

When I use u8g2, I still need another software wire for the bme280 and the ccs811, wich will still cost me more pins than I have (and complexity).

Koepel commented 5 years ago

display.begin() calls Wire.begin(): https://github.com/adafruit/Adafruit_SSD1306/blob/master/Adafruit_SSD1306.cpp#L477

And this is the display.begin() in the examples, which you don't have: https://github.com/adafruit/Adafruit_SSD1306/blob/master/examples/ssd1306_128x64_i2c/ssd1306_128x64_i2c.ino#L59

supersjimmie commented 5 years ago

Adafruit_SSD1306 display(SCREEN_WIDTH, SCREEN_HEIGHT, &myWire, OLED_RESET); Has the &myWire. This is then passed to https://github.com/adafruit/Adafruit_SSD1306/blob/master/Adafruit_SSD1306.cpp#L166-L171 with wire(twi ? twi : &Wire). The Line 477 that you mention, does not call Wire->begin but wire->begin (note the w). So this is a call to the myWire, so it's (a second call to) myWire.begin(). Hopefully a second call to myWire.begin() won't hurt.

And yes, in my begin post I forgot to past-in the display.begin, which I do have.

void setup() {
...
   myWire.begin();
...
  if(!display.begin(SSD1306_SWITCHCAPVCC, 0x3C)) {
    Serial.println(F("SSD1306 allocation failed"));
    for(;;); // Don't proceed, loop forever
  }
Koepel commented 5 years ago

Sorry for the confusion! With "Wire.begin()" I meant the .begin() function of a "Wire-alike" library. I meant to say that you may omit your myWire.begin(); when using display.begin(), because it will also be called from display.begin().

A second call to somethingWire.begin() won't hurt. Everything will be initialized once more. Even the official Arduino Wire.begin() function may be called more than once.

You don't have many options. You could try another TwoWire compatible software I2C library or try to use the u8g2 and SoftwareWire on the same pins.

supersjimmie commented 5 years ago

I tried the other software wire options from the list, but none was compatible enough to really do a 1:1 replace. And using the u8g2 might solve the issue for the display but won't work for the bme280 and the ccs811 at all. :(

Koepel commented 5 years ago

I'm having trouble finding a Adafruit I2C OLED of 128x64. Which display do you use? Is it this one in I2C mode www.adafruit.com/product/326 ?

supersjimmie commented 5 years ago

To be precise, I bought this one: https://www.aliexpress.com/item/0-96-inch-IIC-Serial-White-OLED-Display-Module-128X64-I2C-SSD1306-12864-LCD-Screen-Board/32878113301.html Which probably works the same.

Koepel commented 5 years ago

That is not a Adafruit OLED display. The Adafruit library is for the Adafruit displays only. Because those displays are not always according to the I2C standard, and they are not all the same, Adafruit has optimized their library for their displays only.

You had luck that it worked with the normal Arduino Wire library.

I have ordered that display in the link you gave and I will keep this bug open. It will take a month before I get it and I don't know if I have time to test it.

Koepel commented 5 years ago

Confirmed. The SoftwareWire does not put data on the bus, it bails out after the I2C address is send, even though the OLED display sends a ACK. It is very consistent and independant of voltage levels or speed or timing. The SoftwareWire correctly samples the ACK/NAK from a Slave just before the falling edge of the nineth clock pulse. I hope to look into it next week.

Koepel commented 5 years ago

I'm sorry, but I'm stuck. I need some help to fix this.

The Adafruit library uses a pointer to a TwoWire object. When the Adafruit library calls .begin or .write from the file "Adafruit_SSD1306.cpp", then the SoftwareWire functions are called. However, when calling the .beginTransmission or .endTransmission, the functions from the Arduino TwoWire hardware library are called !

As a result, the SoftwareWire gets a lot of .write calls, that explains the weird I2C bus data.

I have changed the TwoWire pointer from "wire" to "anyWire" in the Adafruit library to be sure to avoid a naming conflict. Here is a part of the changed command1 function:

anyWire->beginTransmission(i2caddr); // Arduino TwoWire hardware function is called !
anyWire->write(0x00);                // SoftwareWire library is called
anyWire->write(c);                   // SoftwareWire library is called
anyWire->endTransmission();          // Arduino TwoWire hardware function is called !

I tried changing the type of the parameters of the SoftwareWire, changing names or attributes and keywords. Nothing helps.

A Quick and Dirty fix is to force the code in Adafruit_SSD1306.cpp to use the SoftwareWire library. Change all the calls to myWire.beginTransmission, myWire.write, and so on. And add this at the top:

#include <SoftwareWire.h>
extern SoftwareWire myWire;

@supersjimmie I see that you also made an issue at Adafruit: https://github.com/adafruit/Adafruit_SSD1306/issues/133 You seem to be the first one to try this combination of libraries. I also noticed your question at the Dutch tweakers.net. You can find me sometimes at the Dutch arduinoforum.nl.

To be sure that the display is not the problem, I tried the U8g2 library (not the Adafruit U8g2 but the original U8g2). The U8g2 hardware I2C and the U8g2 software I2C work well, and also the U8g2 hardware functions when using the SoftwareWire library instead works fine.

supersjimmie commented 5 years ago

Sounds good. So your advise is to forget adafruit and move to the original U8g2 libraries? Any quick-tips about what you did to get that working?

Koepel commented 5 years ago

My goal is to fix this bug.

The U8g2 library is (too) big. The easiest solution for you is perhaps the Quick and Dirty fix and change the calls in the file "Adafruit_SSD1306.cpp".

These work for the U8g2 library:

U8G2_SSD1306_128X64_NONAME_F_HW_I2C u8g2(U8G2_R0, /* reset=*/ U8X8_PIN_NONE);
U8G2_SSD1306_128X64_NONAME_F_SW_I2C u8g2(U8G2_R0, /* clock=*/ SCL, /* data=*/ SDA, /* reset=*/ U8X8_PIN_NONE);   // All Boards without Reset of the Display

The SW_I2C option is a software I2C implementation of the U8g2 library. I think it can work together with the SoftwareWire library on the same pins, but I did not test that. I also tried the HW_I2C option and changed the file "U8x8lib.cpp" to use 'myWire' instead of 'Wire'. That worked, but the "GraphicsTest" example gave a low memory warning.

Thank you for reporting this bug and for you patience. Sorry for the confusion and this lengthy thread.

supersjimmie commented 5 years ago

I tried your solution to fix it in the Adafruit_SSD1306.cpp but I think I don't fully understand your steps. You mention anyWire and myWire, so I think you mean I should replace all with only myWire? But if I do that, the compiler complains about the non-existing myWire->... pieces.

Edit, I changed all wire-> pieces to myWire. pieces and added

#include <SoftwareWire.h>
extern SoftwareWire myWire;

to the beginning of the cpp file.

The .ino file starts with:

#include <SoftwareWire.h>
SoftwareWire myWire( A2, A3, true, true); // sdaPin, sclPin, pullups, detectClockStretch
#include <Adafruit_GFX.h>
#include <Adafruit_SSD1306.h>

and setup starts with:

  myWire.begin();
  if(!display.begin(SSD1306_SWITCHCAPVCC, 0x3C)) { // Address 0x3D for 128x64
    Serial.println(F("SSD1306 allocation failed"));
  }

THen when I try to send something to the display (now on A2/A3) in the same way that worked with the normal wire (on the normal A4/A5 pins), nothing happens on the display.

Koepel commented 5 years ago

The name "anyWire" was just a test, it is not needed. I have the changes as you describe, with a few differences. Adafruit sets the I2C speed to 400kHz, I set it to a normal 100kHz with:

Adafruit_SSD1306 display(SCREEN_WIDTH, SCREEN_HEIGHT, &myWire, OLED_RESET, 100000UL, 100000UL);

I use the two extra parameters for display.begin:

  myWire.begin();
  if (!display.begin(SSD1306_SWITCHCAPVCC, 0x3C, false, true))
  {
    Serial.println(F("SSD1306 allocation failed"));
  }

In the altered "Adafruit_SSD1306.cpp" file there are now 23 calls to myWire., four of them in #define and the rest the functions.

supersjimmie commented 5 years ago

After replace, I found these:

Search "myWire" (44 hits in 1 file)
(44 hits)
    Line 2: extern SoftwareWire myWire;
    Line 61:  #define WIRE_MAX BUFFER_LENGTH          ///< AVR or similar myWire lib
    Line 63:  #define WIRE_MAX (SERIAL_BUFFER_SIZE-1) ///< Newer myWire uses RingBuffer
    Line 72:  #define WIRE_WRITE myWire.write ///< myWire write function in recent Arduino lib
    Line 72:  #define WIRE_WRITE myWire.write ///< myWire write function in recent Arduino lib
    Line 74:  #define WIRE_WRITE myWire.send  ///< myWire write function in older Arduino lib
    Line 74:  #define WIRE_WRITE myWire.send  ///< myWire write function in older Arduino lib
    Line 90:  #define SETWIRECLOCK myWire.setClock(wireClk)    ///< Set before I2C transfer
    Line 91:  #define RESWIRECLOCK myWire.setClock(restoreClk) ///< Restore after I2C xfer
    Line 92: #else // setClock() is not present in older Arduino myWire lib (or WICED)
    Line 109: // beginning and end of I2C transfers (the myWire clock may be sped up before
    Line 114: // Check first if myWire, then hardware SPI, then soft SPI:
    Line 116:  if(myWire) {                 \
    Line 123:  } ///< myWire, SPI or bitbang transfer setup
    Line 125:  if(myWire) {                 \
    Line 132:  } ///< myWire, SPI or bitbang transfer end
    Line 143:             Pointer to an existing TwoWire instance (e.g. &myWire, the
    Line 150:             Speed (in Hz) for myWire transmissions in SSD1306 library calls.
    Line 159:             Speed (in Hz) for myWire transmissions following SSD1306 library
    Line 160:             calls. Defaults to 100000 (100 KHz), the default Arduino myWire
    Line 171:   Adafruit_GFX(w, h), spi(NULL), myWire(twi ? twi : &myWire), buffer(NULL),
    Line 171:   Adafruit_GFX(w, h), spi(NULL), myWire(twi ? twi : &myWire), buffer(NULL),
    Line 205:   int8_t cs_pin) : Adafruit_GFX(w, h), spi(NULL), myWire(NULL), buffer(NULL),
    Line 238:   Adafruit_GFX(w, h), spi(spi ? spi : &SPI), myWire(NULL), buffer(NULL),
    Line 273:   Adafruit_GFX(SSD1306_LCDWIDTH, SSD1306_LCDHEIGHT), spi(NULL), myWire(NULL),
    Line 301:   spi(&SPI), myWire(NULL), buffer(NULL), mosiPin(-1), clkPin(-1),
    Line 323:   Adafruit_GFX(SSD1306_LCDWIDTH, SSD1306_LCDHEIGHT), spi(NULL), myWire(&myWire),
    Line 323:   Adafruit_GFX(SSD1306_LCDWIDTH, SSD1306_LCDHEIGHT), spi(NULL), myWire(&myWire),
    Line 366:   if(myWire) { // I2C
    Line 367:     myWire.beginTransmission(i2caddr);
    Line 370:     myWire.endTransmission();
    Line 380:   if(myWire) { // I2C
    Line 381:     myWire.beginTransmission(i2caddr);
    Line 386:         myWire.endTransmission();
    Line 387:         myWire.beginTransmission(i2caddr);
    Line 394:     myWire.endTransmission();
    Line 472:   if(myWire) { // Using I2C
    Line 480:     if(periphBegin) myWire.begin();
    Line 492:       // SPI peripheral begin same as myWire check above.
    Line 911:   if(myWire) { // I2C
    Line 912:     myWire.beginTransmission(i2caddr);
    Line 917:         myWire.endTransmission();
    Line 918:         myWire.beginTransmission(i2caddr);
    Line 925:     myWire.endTransmission();

Including comments and the line that I added on top, that's 44.

So this line contains 2 of them, perhaps they conflict?

    Line 323:   Adafruit_GFX(SSD1306_LCDWIDTH, SSD1306_LCDHEIGHT), spi(NULL), myWire(&myWire),

It does work now, so I am only a bit concerned about that line 323, could that be done better or leave it like that?

Koepel commented 5 years ago

I'm glad the quick and dirty fix works for you. I hope the Pro Mini has still enough memory for other code.

Line 323 does not seem right. I prefer that you keep the original code for the constructors. The "wire" (all four lowercase) is a private variable of the Adafruit_SSD1306 class as you can see here. It is a pointer to a object. Normally it points to the Arduino Wire object, but we let it point to the myWire object. I did not change the constructors with the initialization of "wire" (all four lowercase) and not even the if-statements with "wire" (all four lowercase).

I have changed these function calls:

I did not change these "wire" (all four lowercase):

The if-statements do not matter, but I prefer that the constructors are kept original. I think you have the "wire" (all four lowercase) changed in about six constructors. Since only one constructor is used, you could remove the other five constructors.

dalegass commented 4 years ago

I'm pretty sure that the original problem of using SSD1306 (and other libraries) with SoftwareWire was due to missing and/or improperly declared prototype/methods (in Wire's utility/twi.h), resulting in hardware Wire's methods being called unexpectedly.

As Koepel noted, I similarly found that the wrong class' method (TwoWire's instead of SoftwareWire's) was being called for beginTransmission()--even at the assembler level it was clearly calling the wrong function and not attempting to use SoftwareWire's subclassed method.

Adding "virtual" to the beginTransmission() (and other methods overridden by SoftwareWire) in Wire's utility/twi.h seemed to solve that problem. "virtual" was specified for some of the methods, but not for all, and that appeared to cause some of the problems.

I believe there was also a problem with requestFrom(int addr, int size) matching the parent Wire's prototype, rather than SoftwareWire's requestFrom(int addr, int size, boolean sendStop=true). The default argument version in SoftwareWire isn't called when the sendStop parameter is left off, and instead the parent class Wire's two-argument version is favoured.

By adding a two-argument declaration/definition of requestFrom, and adding "virtual" to Wire's members that are overridden by SoftwareWire, things seem to be working for SSD1306 (and a TCS34725 color sensor library that was similarly failing with SoftwareWire, until I made those changes.)

I'll try to get together a specific changeset to fix things, but thought I'd post my findings in case I don't get around to it. :)

NOTE: There is likely a significant performance hit to fixing this properly. Using virtual method is expensive, especially with every single write() call looking up virtual methods. The earlier suggested solution of doing the search/replace and pointing to a static "SoftwareWire myWire" will probably result in faster performance. I'm hitting about 33 kbps tops on a 8 Mhz Atmega328p using this virtual method fix. (Having the SoftwareWire's multi-byte write() call i2c_write() directly helps this a bit.)

Testato commented 3 years ago

Original wire library will never declare Virtual the fubction that we need