XIAO RP2040 failed @ Wire.write (I2C)

numeru55 commented 2 years ago

Discussed in https://github.com/earlephilhower/arduino-pico/discussions/591

^{Originally posted by **numeru55** May 23, 2022} Using both ``` rp2040:rp2040 2.0.2 2.0.2 Raspberry Pi Pico/RP2040 Seeeduino:rp2040 2.7.2 2.7.2 Seeed XIAO RP2040 ``` I2C scan is OK for both core: ``` Wire.beginTransmission(0x40); byte error = Wire.endTransmission(); ``` But `Wire.write()` is ignore only for earlephihower: ``` Wire.beginTransmission(0x40); Wire.write(byte(0)); byte error = Wire.endTransmission(); ``` Seeed core : address byte and 0 are transmitted normally earlephihower: nothing is transmitted on I2C line. Why?

josephduchesne commented 2 years ago

I'm also seeing this. Same versions of cores.

earlephilhower commented 2 years ago

I have no idea what Seeeduino has done to the core they have repackaged. Would either of you be able to get a diff of the file libraries/Wire/Wire.cpp?

numeru55 commented 2 years ago

Thank you for your comment. There are many difference, and seems that Seeed is based on mbed:

$ diff Seeeduino/hardware/rp2040/2.7.2/libraries/Wire/Wire.cpp rp2040/hardware/rp2040/2.0.2/libraries/Wire/src/Wire.cpp | head
2,3c2
<   Wire.cpp - wrapper over mbed I2C / I2CSlave
<   Part of Arduino - http://www.arduino.cc/
---
>     I2C Master/Slave library for the Raspberry Pi Pico RP2040
5c4
<   Copyright (c) 2018-2019 Arduino SA
---
>     Copyright (c) 2021 Earle F. Philhower, III <earlephilhower@yahoo.com>
7,20c6,21

earlephilhower commented 2 years ago

Thanks @numeru55. Looks like I will need to dig out the logic analyzer to dig into this.

Can you please post a complete small sketch, just so I know I'm running the same code as you? I don't have any XIAO boards, but the only difference would be the actual pins they chose for SDA/SCL and not any of the code being executed.

numeru55 commented 2 years ago

#include <Wire.h>

void setup()
{
  Wire.begin();

  Serial.begin(9600);
  while (!Serial);
  Serial.println("\nBooted");
}

void loop()
{
    Wire.beginTransmission(0x40);
    delay(10);
    Wire.write(byte(0));
    byte error = Wire.endTransmission();
  if (error==0) {
    Serial.println("OK");
  } else {
    Serial.println("Fail");
  }
  delay(1000);
}

$ arduino-cli compile --fqbn rp2040:rp2040:seeed_xiao_rp2040   INA219_test

(snip Japanese message)

$ arduino-cli upload -p /dev/cu.usbmodem14101 --fqbn rp2040:rp2040:seeed_xiao_rp2040   INA219_test
Resetting /dev/cu.usbmodem14101
Converting to uf2, output size: 139776, start address: 0x2000
Flashing /Volumes/RPI-RP2 (RPI-RP2)
Wrote 139776 bytes to /Volumes/RPI-RP2/NEW.UF2
$ arduino-cli monitor -p /dev/cu.usbmodem14101
Connected to /dev/cu.usbmodem14101! Press CTRL-C to exit.

Booted
Fail
Fail
^C

Pin out is:

                     USB
               +-------------+
P26 |  A0 | D0 |             |  5V
P27 |  A1 | D1 |             | GND
P28 |  A2 | D2 |             | 3V3
P29 |  A3 | D3 |             | D10 | MOSI | P3
 P6 | SDA | D4 |             |  D9 | MISO | P4
 P7 | SCL | D5 |             |  D8 |  SCK | P2
 P0 |  TX | D6 |             |  D7 |  CSn | P1 | RX
               +-------------+

I2C device is connected P6/P7 (@ RP2040 GPIO6/7) correctly.

If I comment-out // Wire.write(byte(0)); monitor will show OK.

I also check using logic analyzer. When the fail, means add Wire.write(byte(0), P6/P7 line is nothing pulse.

Seeed core is always OK. Very mysterious for me.

earlephilhower commented 2 years ago

If you Wire.write(1) does it work properly? It may be an issue with C++ overloads since \0 is end-of-string and would mean send nothing if the Print class overload caused it to think this was a String or something.

That seems a long shot, but I've heard from other people that I2C is working find and they've used it in production cases, so it is very mysterious for me, too!

earlephilhower commented 2 years ago

Oh and also thanks for the code so I can test myself.

One more thing, the delay() in your code is not doing what you think it is. Due to the hardware on the RP2040, the transmission (address + r/w bit, data, etc.) doesn't start until endTransmission. We need to buffer up everything before beginning the transaction. The delay just delays the start of xmission, and does not add a delay after the address+r/w byte is sent.

numeru55 commented 2 years ago

Thank you for your detailed explanation. I will test your comment later.

ZackFreedman commented 2 years ago

I'm having the same issue as OP - the zero-byte-transmission probe works, but all attempts to transmit one or more bytes fails with error 4.

I've done some investigating myself - the issue seems to be that i2c_write_blocking_until() is timing out (returning 255).

I've tested this with the same Seeedstudio Xiao RP2040 and a brand-new Raspberry Pi Pico, and the issue occurs on both.

earlephilhower commented 2 years ago

@ZackFreedman on your application, do you need a delay between the address byte and 1st data written byte (i.e. some slow device) (like in the sample code posted above)? At the default 100khz SCK I doubt it. but just checking.

Probe is special cased because AFAIK the HW doesn't support it, so I simulate using SW bit-banging.

ZackFreedman commented 2 years ago

Nope, I'm just trying to do standard I2C transactions. I'm using PCA9555 I/O expanders and an FT6336U touchscreen controller.

I tested this with slower clock rates, same effect. The circuit and i2c transactions work properly when targeting the Mbed core.

earlephilhower commented 2 years ago

As a sanity check, the loopback from I2C0->I2C1 test in the examples still works fine. I did find a bug in the read path when a timeout occurred which is now fixed. As far as I can see from an app-side I2C is not 100% broken. So, I'm still very puzzled.

The Arduino MBED core seems like it has many layers between the app and the hardware. At the lowest level it seems to use i2c_write_blocking() without any timeout (but they may implement the timeout at a higher level than the mbed porting i2c_write).

Do either of you have a logic analyzer able to capture the I2C bus on a failing transaction? I'm having trouble finding an I2C doodad in my junk box to connect to and try non-loopback testing.

ZackFreedman commented 2 years ago

I just tested on a brand-new Pi Pico, and you're right, the test worked. I ended up getting it to work on the Xiao RP2040 by doing the following:

On the breadboard:
- Connected D4 to D6
- Connected D5 to D7
In TalkingToMyself.ino:
- Changed line 17 to Wire1.setSDA(6);
- Changed line 18 to Wire1.setSCL(7);

I then went into my own test sketches, changed all instances of Wire to Wire1, set SDA and SCL to 6 and 7, and it worked. The problem was in the board definition.

I recommend bringing the Xiao RP2040 in line with Seeedstudio's first-party board support package and pinout:

Assign hardware interface i2c1 to Wire and i2c0 to Wire1
Define D0-D10 as aliases for associated physical GPIO pins
Assign Wire the default SDA and SCL of D4 and D5 (physical GPIO 6 and 7)

I'd do it myself, but I've never messed around with a board definition. I'll probably do more harm than good.

Thanks for promptly investigating. Your test was an important sanity check.

ZackFreedman commented 2 years ago

For convenience, here's Seeedstudio's pins_arduino.h included in their board support package:

#include <macros.h>
#include <stdint.h>

#ifndef __PINS_ARDUINO__
#define __PINS_ARDUINO__

#ifdef __cplusplus
extern "C" unsigned int PINCOUNT_fn();
#endif

// Pin count
// ----
#define PINS_COUNT           (PINCOUNT_fn())
#define NUM_DIGITAL_PINS     (30u)
#define NUM_ANALOG_INPUTS    (4u)
#define NUM_ANALOG_OUTPUTS   (0u)

extern PinName digitalPinToPinName(pin_size_t P);

// LEDs
// ----
#define PIN_LED     (25u)
#define LED_BUILTIN PIN_LED

// Digital pins
// ----
#define PIN_D0 (26u)  
#define PIN_D1 (27u)
#define PIN_D2 (28u)
#define PIN_D3 (29u)
#define PIN_D4 (6u)
#define PIN_D5 (7u)
#define PIN_D6 (0u)
#define PIN_D7 (1u)
#define PIN_D8 (2u)
#define PIN_D9 (4u)
#define PIN_D10 (3u)

static const uint8_t D0 = PIN_D0;
static const uint8_t D1 = PIN_D1;
static const uint8_t D2 = PIN_D2;
static const uint8_t D3 = PIN_D3;
static const uint8_t D4 = PIN_D4;
static const uint8_t D5 = PIN_D5;
static const uint8_t D6 = PIN_D6;
static const uint8_t D7 = PIN_D7;
static const uint8_t D8 = PIN_D8;
static const uint8_t D9 = PIN_D9;
static const uint8_t D10 = PIN_D10;

// Analog pins
// -----------
#define PIN_A0 (26u)
#define PIN_A1 (27u)
#define PIN_A2 (28u)
#define PIN_A3 (29u)

static const uint8_t A0  = PIN_A0;
static const uint8_t A1  = PIN_A1;
static const uint8_t A2  = PIN_A2;
static const uint8_t A3  = PIN_A3;

#define ADC_RESOLUTION 12

// Serial
#define PIN_SERIAL_TX (0ul)
#define PIN_SERIAL_RX (1ul)

// SPI
#define PIN_SPI_MISO  (16u)
#define PIN_SPI_MOSI  (19u)
#define PIN_SPI_SCK   (18u)
#define PIN_SPI_SS    (17u)

static const uint8_t SS   = PIN_SPI_SS;   // SPI Slave SS not used. Set here only for reference.
static const uint8_t MOSI = PIN_SPI_MOSI;
static const uint8_t MISO = PIN_SPI_MISO;
static const uint8_t SCK  = PIN_SPI_SCK;

// Wire
#define SDA        (6u)
#define SCL        (7u)

#define SERIAL_HOWMANY      1
#define SERIAL1_TX          (digitalPinToPinName(PIN_SERIAL_TX))
#define SERIAL1_RX          (digitalPinToPinName(PIN_SERIAL_RX))

#define SERIAL_CDC          1
#define HAS_UNIQUE_ISERIAL_DESCRIPTOR
#define BOARD_VENDORID      0x2886
#define BOARD_PRODUCTID     0x8042
#define BOARD_NAME          "RaspberryPi Pico"

uint8_t getUniqueSerialNumber(uint8_t* name);
void _ontouch1200bps_();

#define SPI_HOWMANY     (1)
#define SPI_MISO        (digitalPinToPinName(PIN_SPI_MISO))
#define SPI_MOSI        (digitalPinToPinName(PIN_SPI_MOSI))
#define SPI_SCK         (digitalPinToPinName(PIN_SPI_SCK))

#define WIRE_HOWMANY    (1)
#define I2C_SDA         (digitalPinToPinName(SDA))
#define I2C_SCL         (digitalPinToPinName(SCL))

#define digitalPinToPort(P)     (digitalPinToPinName(P)/32)

#define SERIAL_PORT_USBVIRTUAL      SerialUSB
#define SERIAL_PORT_MONITOR         SerialUSB
#define SERIAL_PORT_HARDWARE        Serial1
#define SERIAL_PORT_HARDWARE_OPEN   Serial1

#define USB_MAX_POWER   (500)

#endif //__PINS_ARDUINO__

earlephilhower commented 2 years ago

Great, thanks @ZackFreedman . I'll need to do some plumbing to allow swapping the i2c devices, but will get something together this weekend with the info you've provided.

I'm really glad it was just a variant issue. The core really is a thin wrapper around the really nice SDK, so I was worried I'd need to dig into the actual hardware performance!

numeru55 commented 2 years ago

Everyone, thank you for your consideration of my question.

@earlephilhower

#include <Wire.h>

void setup() {
  Wire.begin();

  Serial.begin(9600);
  Serial.println("\nBooted");
}

void loop() {
  Serial.println("Scan test");
  Wire.beginTransmission(0x40);
  byte error = Wire.endTransmission();
  if (error == 0) {
    Serial.println("OK");
  } else {
    Serial.println("Fail");
  }
  delay(100);

  Serial.println("Write byte(0) test");
  Wire.beginTransmission(0x40);
  Wire.write(byte(0));
  error = Wire.endTransmission();
  if (error == 0) {
    Serial.println("OK");
  } else {
    Serial.println("Fail");
  }
  delay(100);

  Serial.println("Write 1 test");
  Wire.beginTransmission(0x40);
  Wire.write(1);
  error = Wire.endTransmission();
  if (error == 0) {
    Serial.println("OK");
  } else {
    Serial.println("Fail");
  }
  delay(800);
}

I upload this using --fqbn rp2040:rp2040:seeed_xiao_rp2040 and the result is:

$ arduino-cli monitor -p /dev/cu.usbmodem14101
Connected to /dev/cu.usbmodem14101! Press CTRL-C to exit.
Fail
Write 1 test
Fail
Scan test
OK
Write byte(0) test
Fail
Write 1 test
Fail

And I got D6/D7 pulse:

On the other hand using --fqbn Seeeduino:rp2040:Seeed_XIAO_RP2040

$ arduino-cli monitor -p /dev/cu.usbmodem14101
Connected to /dev/cu.usbmodem14101! Press CTRL-C to exit.
Scan test
OK
Write byte(0) test
OK
Write 1 test
OK
Scan test
OK
Write byte(0) test
OK
Write 1 test
OK

The first pulse is same as above. 2nd and 3rd are:

And I also checked this with --fqbn rp2040:rp2040:rpipico

#include <Wire.h>

void setup() {
  Wire1.setSDA(6);
  Wire1.setSCL(7);
  Wire1.begin();

  Serial.begin(9600);
  Serial.println("\nBooted");
}

void loop() {
  Serial.println("Scan test");
  Wire1.beginTransmission(0x40);
  byte error = Wire1.endTransmission();
  if (error == 0) {
    Serial.println("OK");
  } else {
    Serial.println("Fail");
  }
  delay(100);

  Serial.println("Write byte(0) test");
  Wire1.beginTransmission(0x40);
  Wire1.write(byte(0));
  error = Wire1.endTransmission();
  if (error == 0) {
    Serial.println("OK");
  } else {
    Serial.println("Fail");
  }
  delay(100);

  Serial.println("Write 1 test");
  Wire1.beginTransmission(0x40);
  Wire1.write(1);
  error = Wire1.endTransmission();
  if (error == 0) {
    Serial.println("OK");
  } else {
    Serial.println("Fail");
  }
  delay(800);
}

It means that compile and upload Raspberry Pi PICO to XIAO RP2040.

The result was same as --fqbn Seeeduino:rp2040:Seeed_XIAO_RP2040

earlephilhower commented 2 years ago

@ZackFreedman can you confirm that you posted the right header? I'm looking at this SPI bit:

// SPI
#define PIN_SPI_MISO  (16u)
#define PIN_SPI_MOSI  (19u)
#define PIN_SPI_SCK   (18u)
#define PIN_SPI_SS    (17u)

But if I look at the v1.22 schematic I see this, which indicated SPI is on GPIO 1-4:

Pins GPIO16,17 are tied to color LEDs per that schematic...

ZackFreedman commented 2 years ago

Yep, that is the correct pins_arduino.h. It looks like you're right and Seeedstudio actually messed that up.

ZackFreedman commented 2 years ago

I'm really glad it was just a variant issue. The core really is a thin wrapper around the really nice SDK, so I was worried I'd need to dig into the actual hardware performance!

Me too... the I2C subsystem isn't adequately documented, the SDK code isn't well-commented, and they've made substantial changes to the library without effectively recording why. I wasted at least 30 hours trying to debug this before giving up and posting here.

Speaking of, in TwoWire::endTransmission() in Wire.cpp, it might be a good idea to catch PICO_ERROR_TIMEOUT (ie. i2c_write_blocking_until() returns 254) and return error 2 (NACK on transmit of address).

Also FYI, error 3 (NACK on transmit of data) is never returned. I'm assuming there's supposed to be if (ret == 1 && ret != len) return 3; or something.

earlephilhower commented 2 years ago

@ZackFreedman and @numeru55 can you please try #606 and report back?

ZackFreedman commented 2 years ago

I'm going to be totally honest, I'm not totally sure how to download the branch and get it into Arduino. I'll figure it out, but it'll take a bit.

numeru55 commented 2 years ago

@earlephilhower

Maybe wrong my test procedure, but I2C seems work fine.

https://github.com/earlephilhower/arduino-pico/pull/606

move to "Commits" and click "<>" right at "b39a646"

https://github.com/earlephilhower/arduino-pico/tree/b39a6462abbfef7b21cf896156a6d1fe7f6f3095

Download .zip and expand

https://github.com/earlephilhower/arduino-pico/archive/b39a6462abbfef7b21cf896156a6d1fe7f6f3095.zip

$ cp -r ~/Downloads/arduino-pico-b39a6462abbfef7b21cf896156a6d1fe7f6f3095/* ~/Library/Arduino15/packages/rp2040/hardware/rp2040/2.0.2/
$ arduino-cli compile --fqbn rp2040:rp2040:seeed_xiao_rp2040 INA219_test2

Sketch uses 63288 bytes (3%) of program storage space. Maximum is 2093056 bytes.
Global variables use 11836 bytes (4%) of dynamic memory, leaving 250308 bytes for local variables. Maximum is 262144 bytes.

$ arduino-cli upload -p /dev/cu.usbmodem14101 --fqbn rp2040:rp2040:seeed_xiao_rp2040 INA219_test2
Error during Upload: Failed uploading: cannot execute upload tool: fork/exec /Users/numeru55/Library/Arduino15/packages/rp2040/hardware/rp2040/2.0.2/system/python3/python3: no such file or directory

Upload error has occured, so I copy uf2 directly. Serial monitor shows OK and my logic analyzer displays good I2C message.

$ cp /private/var/folders/zb/rm2mwlx90_s8fw81lspk9fhc0000gn/T/arduino-sketch-D82F6E1B8D0A35FC799BD4FC481951F5/INA219_test2.ino.uf2 /Volumes/RPI-RP2/

$ arduino-cli monitor -p /dev/cu.usbmodem14101
Connected to /dev/cu.usbmodem14101! Press CTRL-C to exit.
Scan test
OK
Write byte(0) test
OK
Write 1 test
OK
Scan test
OK
Write byte(0) test
OK
Write 1 test
OK
Scan test
OK
Write byte(0) test
OK
Write 1 test
OK
Scan test
OK
Write byte(0) test
OK
Write 1 test
OK
^C
$

earlephilhower commented 2 years ago

Thanks, @numeru55 . Downloading the zip file really isn't the right way to get a test, but you answer seems to show it did work. I'll do a new release tomorrow including this change.

numeru55 commented 2 years ago

@earlephilhower, it's also my happy if my question help to improve your board core, which many RP2040 users love it!

josephduchesne commented 2 years ago

Thanks everyone! I'm glad to move away from the very bare bones seedstudio core :)

earlephilhower / arduino-pico

XIAO RP2040 failed @ Wire.write (I2C) #594

Discussed in https://github.com/earlephilhower/arduino-pico/discussions/591