lexus2k / ssd1306

Driver for SSD1306, SSD1331, SSD1351, IL9163, ILI9341, ST7735, PCD8544, Nokia 5110 displays running on Arduino/ESP32/Linux (Rasperry) platforms
MIT License
655 stars 125 forks source link

ssd1306_i2c_twi.c #93

Closed haarer closed 4 years ago

haarer commented 4 years ago

There is a possible mistake in the i2c code, there is a semicolon after an if clause and a following break statement, which may make the i2c retry fail.

I have no i2c display to test it, but i believe this wont work as expected, it will break the loop on the first iter , and not go for the full MAX_RETRIES

also GCC 9.2 complains and this says it all.

Compiling .pio/build/pro16MHzatmega328/libaf3/ssd1306/intf/spi/ssd1306_spi_avr.o
.pio/libdeps/pro16MHzatmega328/ssd1306/src/intf/i2c/ssd1306_i2c_twi.c: In function 'ssd1306_twi_start':
.pio/libdeps/pro16MHzatmega328/ssd1306/src/intf/i2c/ssd1306_i2c_twi.c:49:9: warning: this 'if' clause does not guard... [-Wmisleading-indentation]
   49 |         if (!--iters);
      |         ^~
.pio/libdeps/pro16MHzatmega328/ssd1306/src/intf/i2c/ssd1306_i2c_twi.c:50:9: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if'
   50 |         {
      |         ^

The code

static uint8_t ssd1306_twi_start(void)
{
    uint8_t twst;
    uint8_t iters = MAX_RETRIES;
    do
    {
        TWCR = (1<<TWINT) | (1<<TWSTA) | (1<<TWEN);
        while ( (TWCR & (1<<TWINT)) == 0 );
        twst = TWSR & 0xF8;
        if (!--iters);
        {
            break;
        }
    } while (twst == TW_MT_ARB_LOST);
    if ((twst != TW_START) && (twst != TW_REP_START))
    {
        return twst;
    }
    return 0;
}

To Reproduce Well - i cant .

Expected behavior remove the semicolon

Screenshots n.a.

Please complete the following information:

lexus2k commented 4 years ago

Thank you for reporting the issue. Yes, this is misprint.