i2c SSD1306 bug with sending device address

[agento2]

file with the issue : lines 109, 130

In both the send command and send buffer methods the call to the i2c library looks like this

i2c_write_blocking(i2c_default, (SSD1306_I2C_ADDR & SSD1306_WRITE_MODE), buf, 2, false);

this issue is with the address parameter, (SSD1306_I2C_ADDR & SSD1306_WRITE_MODE), it should be just SSD1306_I2C_ADDR because the anding of SSD1306_WRITE_MODE which is 0xFE only works for some people using this example if their device address matches the 0x3C that is defaulted here. This is because that & FE is a no-op for addresses that are even, where as in my case the device address is 0x3D results in changing the address sent to 0x3C.
Another illustration for why that shouldn't be there is that if you were to change it to & SSD1306_READ_MODE it would be a no-op for all addresses and the example would still work.

My guess is this is a vestige from using a different i2c library that did not have both read and write methods but just send, even then the & doesn't seem like the right operator.

I can file a PR for this simple change or someone else can. I just hope it saves someone else the time i spent figuring out why my sparkfun made ssd1306 based Oled display with address 0x3D wouldn't do anything at all when running this example.

[agento2]

I opened a PR for this issue link

[urbasus]

7-bit addressing for i2c distinguishes read-write by the eight bit. From wikipedia:

My understanding for the code is the address constant is either the read or write address, and that the AND makes sure it becomes the write address. To make it the read address you'd OR it, not AND it. Does any of this change what you think of the issue? Maybe you already knew this and it's just me being slow again, haha.

I've seen more than one way of specifying the i2c-address. Sometimes RW-addresses are explicitly specified, sometimes the address part with room for the rw-bit, sometimes the address normalized to the lowest 7-bits. Keeping it clear and consistent what is being specified is the best way of doing it.

[agento2]

I'd read that yes. I think that setting that bit is handled by the i2c library when you call the write or read methods. I also think this approach is wrong even if it wasn't because the read / write bit shouldn't alter the address just use the unused bit, where as with the code as it is, it does (sometimes) alter it depending on the value of the address.

This is from the i2c.h in the pico-sdk
\param addr 7-bit address of device to write to

[urbasus]

I'd read that yes. I think that setting that bit is handled by the i2c library when you call the write or read methods. I also think this approach is wrong even if it wasn't because the read / write bit shouldn't alter the address just use the unused bit, where as with the code as it is, it does (sometimes) alter it depending on the value of the address.

This is from the i2c.h in the pico-sdk \param addr 7-bit address of device to write to

Ok. Based on your explanation, the docstring, and re-reading the original issue description I agree the AND is a mistake and should be removed.

[kilograham]

fix merged into develop