#169 PIC16: I2C, several bugs


Several bugs found in PIC16 I2C library.

-#define I2C_SLEW_OFF 0xc0
+#define I2C_SLEW_OFF 0x80
Slew rate control is only the 7th bit of SSPSTAT and 6th (SMbus) was also set.

- Macros semicolons in i2c.h changed to comma operators (http://en.wikipedia.org/wiki/C_preprocessor#Multiple_statements).

- I2C_IDLE() macro had semicolon at the end. Removed.

- i2c_write() now clears SSPCON1bits.WCOL before writing to SSPBUF.
If there was a write collision on a previous writing, you might think it happened at the current one. And it would also happen at the next ones.

- i2c_write() now does "I2C_IDLE()" instead of "while(I2C_DRDY())".
SSPSTATbits.BF is cleared by hardware after 8 bits have been output and SSPSTATbits.R_W is cleared by hardware after the ninth bit (ACK), at the same time that SSPIF would be set. So, I think that SSPSTATbits.R_W is the flag to check and I2C_IDLE() checks it. I2C_DRDY() does not.

Not it the patch: maybe, I2C_IDLE() should be taken out of i2c_write() so execution doesn't block if you use i2c interrupts. If you don't use interrupts, you would need to call I2C_IDLE() after calling i2c_write().

If there's any doubt please let me know.

Thank you very much


  • Diego Herranz

    Diego Herranz - 2011-09-23
  • Diego Herranz

    Diego Herranz - 2011-10-16

    Any possibility to address this before 3.1.0?

    Maybe I should have posted it in bugs and not in patches, I don't know. If so, I'm sorry.

    Thank you

  • Raphael Neider

    Raphael Neider - 2011-10-16

    Fixed in r6971.

    I find do { ... } while (0) more explicit than the comma-operator (easily mistaken for a semicolon), so I changed the macros to use the loop syntax. Should behave identically, though ... If not, please speak up.

    As for taking I2C_IDLE out of i2_writec, I am hesitant to changing the semantics of the library function from blocking to non-blocking. If non-blocking write is required/requested, one could add i2c_writec_nb() or something to the library.

    Thank you for the report and the reminder.

  • Raphael Neider

    Raphael Neider - 2011-10-16
    • status: open --> closed-accepted
  • Diego Herranz

    Diego Herranz - 2011-10-16

    I think that do{...}while(0) can also work. Anyway, I found it unnecessary on single statement macros.

    Thank you very much for your help.


Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:

No, thanks