Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Si4703 I2C requestFrom (32 bytes) fails with ESP32, works with ESP8266 #682

Closed
AlyoshaVasilieva opened this issue Sep 29, 2017 · 24 comments
Closed

Comments

@AlyoshaVasilieva
Copy link

AlyoshaVasilieva commented Sep 29, 2017

Hardware:

Board: WEMOS LOLIN32 Lite
Core Installation/update date: 29/Sep/2017
IDE name: Arduino IDE
Flash Frequency: 80Mhz
Upload Speed: 256000

Description:

ESP32 fails on a requestFrom call to an Si4703 with requested size of 32 bytes. Result code from requestFrom is 0. i2c_err_t inside requestFrom (from i2cRead call) is 3, which I think corresponds to I2C_ERROR_TIMEOUT. (note: fixed previous line in edit)

ESP8266 (NodeMCU 1.0) works fine and I can run the entire radio system with it.

Made sure to update Arduino core fully, running on latest commit 4230aec.

Sketch:

#include <Wire.h>

#define SI4703_ADDR 0x10

#define ESP32
#ifdef ESP32
#define RST 4
#define SDIO 19
#define SCL 23
#else
#define RST D7
#define SDIO D3
#define SCL D4
#endif

void setup() {
  Serial.begin(115200);
  delay(20);
  Serial.println();
  pinMode(RST, OUTPUT);
  pinMode(SDIO, OUTPUT);
  digitalWrite(SDIO, LOW);
  digitalWrite(RST, LOW);
  delay(1);
  digitalWrite(RST, HIGH);
  delay(1); // Above sets up Si4703 as I2C and resets it

  Wire.begin(SDIO, SCL);

  uint8_t res = Wire.requestFrom(SI4703_ADDR, 32);
  //We want to read the entire register set from 0x0A to 0x09 = 32 bytes.
  Serial.print("requestFrom result code: ");
  Serial.println(res);
  if (res != 32)
    Serial.println("Mismatch detected");

  Serial.println("Spinning on available == 32");
  while(Wire.available() < 32) ; //Wait for 16 words/32 bytes to come back from Si4703
  Serial.println("Finished spinning");

  uint16_t registers[16];
  //Remember, register 0x0A comes in first so we have to shuffle the array around a bit
  for(int x = 0x0A ; ; x++) { //Read in these 32 bytes
    if(x == 0x10) x = 0; //Loop back to zero
    registers[x] = read16();
    if(x == 0x09) break; //We're done!
  }
}

uint16_t read16() {
  uint8_t hiByte = Wire.read();
  uint8_t loByte = Wire.read();
  return((hiByte << 8) + loByte);
}

void loop() {

}

Sketch is a minimal version of https://github.com/mathertel/Radio - since ESP32 fails at requestFrom no need to go much further.

Debug Messages:

requestFrom result code: 0
Mismatch detected
Spinning on available == 32

ESP32 hangs forever at this point since request failed. Wire.available() always returns 0.

@euquiq
Copy link

euquiq commented Sep 29, 2017

Hint: You may want to read it in two 16 byte chunks, instead only one of 32 bytes. That would probably make it work ok, at this time.

@AlyoshaVasilieva
Copy link
Author

AlyoshaVasilieva commented Sep 30, 2017

Using this following method to read 16-bytes at once on ESP32:

  for (int i = 0; i < 2; i++) {
    int offset = i == 1 ? 16 : 0;
    Serial.println(Wire.requestFrom(SI4703_ADDR, 16));
    while (Wire.available() < 16) ; // spin
    for (int k = 0; k < 16; k++) {
      data[k + offset] = Wire.read();
    }
  }

I get this sequence of bytes (hex):
00 00 00 00 00 00 00 00 00 00 00 00 12 42 12 00 00 00 00 00 00 00 00 00 00 00 00 00 12 42 12 00

Using this method on ESP8266 to read 32 bytes at once:

void readReg32(uint8_t* data) {
  uint8_t res = Wire.requestFrom(SI4703_ADDR, 32); //We want to read the entire register set from 0x0A to 0x09 = 32 bytes.
  Serial.print("requestFrom result code: ");
  Serial.println(res);
  if (res != 32)
    Serial.println("Mismatch detected");
  Serial.println("Spinning on available == 32");
  while(Wire.available() < 32) ; //Wait for 16 words/32 bytes to come back from Si4703
  Serial.println("Finished spinning");
  for (int i = 0; i < 32; i++) {
    data[i] = Wire.read();
  }
}

I get this sequence:
00 00 00 00 00 00 00 00 00 00 00 00 12 42 12 00 00 00 00 00 00 00 00 00 00 00 01 00 00 00 00 00

Because of how Si4703 works I believe I need to request and read all 32 bytes at once. Reading 16 bytes at once gives me the same bytes twice (tested with both ESP8266 and ESP32).

Datasheet says:

For read operations, the control word and device acknowledge is followed by an eight bit data word shifted out on falling SCLK edges. Any number of data bytes can be read by sending a low ACK to the device. Device register addresses are incremented by an internal address counter, starting at the upper byte of register 0Ah, followed by the lower byte of register 0Ah, and wrapping back to 00h at the end of the register file. The transfer ends with the STOP conditions regardless of the state of the acknowledge.

@me-no-dev
Copy link
Member

Can you please just for fun try to read 31 bytes and see if that would change anything in the output. I don't have such I2C device around to try, but the number 32 might be the issue here ;) (hardware thing)

@me-no-dev
Copy link
Member

Or even try 33 bytes

@AlyoshaVasilieva
Copy link
Author

AlyoshaVasilieva commented Sep 30, 2017

Reading 33 bytes works! I assume it's wrapping around again, can't tell. Any number higher than 33 fails.

Device is functional by reading 33 bytes at once and I have all registers (I think). Thanks for the help.

@me-no-dev
Copy link
Member

good clue as well :) now I need to figure out how to get this device and be able to fix the bus.

@AlyoshaVasilieva
Copy link
Author

If it helps I bought it here: https://www.aliexpress.com/item/Si4703-FM-Tuner-Evaluation-Board-radio-tuner-board/32404611530.html

Name seems to just be "Si4703 Evaluation Board"

@lonerzzz
Copy link
Contributor

I have been looking at the API a lot lately and there is an anomaly in terms of there being a timeout on the individual command operations. I have played with it enough to come to the conclusion that the same timeout is applied whether reading 10 bytes or 32 bytes. The timeout doesn't seem to be between bytes as I would expect, given that there should be no fixed timeout for I2C at all. I am off for the evening but tomorrow will be trying to modify the HAL layer to apply the timeout to a byte at a time to at least handle some clock stretching.

In the meantime, you may want to try increasing the i2c->dev->timeout.tout setting to the maximum as specified in the file esp32-hal-i2c.c file within the i2cInit method.

@lonerzzz
Copy link
Contributor

lonerzzz commented Sep 30, 2017

I looked further into the esp32-hal-i2c.c file and found that one problem is that when a longer message is sent with I2C_CMD_READ followed only by I2C_CMD_END, that the done flag is not getting set on the command entry with the I2C_CMD_END command so a timeout occurs. Likewise, the end_detect interrupt bit is not being set either. This is despite the actual I2C_CMD_READ command completing successfully.

@me-no-dev
Copy link
Member

When is that case happening?
END is sent if there are more bytes to be sent after. Else it ends with STOP or READ (if sendStop is false)\

@lonerzzz
Copy link
Contributor

If you have more than the 32 bytes that the hardware expected and that is the chunk size as specified in the file, this occurs.

@me-no-dev
Copy link
Member

if more than 32 bytes it sends end after each 32 and stop at the end ;) you have a loop there

@lonerzzz
Copy link
Contributor

Exactly, the end in this situation is somehow problematic. I also tried multiple I2C_CMD_READS in the same block by breaking down the 32 into blocks of 4 and those each worked, still hanging on the I2C_CMD_END.

@me-no-dev
Copy link
Member

That end is in the specs though... it signals that there will be more reads to follow

@lonerzzz
Copy link
Contributor

Agreed, I was looking at the details in the manual around writes and the handling of interrupts because reads are not explained as fully. I can't see that there is anything wrong with the esp32-hal-i2c.c file, only that the expectation of the I2C_CMD_END command done being set isn't happening.

@me-no-dev
Copy link
Member

It’s a weird bus design i must say

@me-no-dev
Copy link
Member

Command done is not set maybe because of that exact reason? It expects more to do?

@lonerzzz
Copy link
Contributor

lonerzzz commented Sep 30, 2017

That is why I am looking at the interrupts trying to see if there is some other indicator to let the calling portion of the application know that it should take action. I am also going to try monitoring the done status of the item preceding END to see if that can be used to mark completion before trying more reading.

@me-no-dev
Copy link
Member

unfortunately I do not have any I2C devices with me, so there is little I can help with for a few days. Wish you luck though!

@stickbreaker
Copy link
Contributor

@lonerzzz and @me-no-dev
I think the END command NEVER sets its DONE flag. I think the design assumes that the ISR call by the I2C_END_DETECT_INT is going to refill the CMD[0..15] with new commands and continue the I2C operation.

I think that once an END is encountered, the I2C state machine hangs waiting for new commands to be loaded. To me this makes perfect sense, process all commands until END pause, continue.

Chuck.

@lonerzzz
Copy link
Contributor

lonerzzz commented Oct 20, 2017

Your understanding matches mine. Since done was not set, I tried polling the status of the entry before END, but I found that once END is called, several things fail:

  1. the SCL can continue toggling indefinitely
  2. subsequent END calls for the next block have further problems
  3. the state machine does not properly recover
  4. only the first END interrupt occurs

These observations may not be quite correct, but I think that is where I got to.

@stickbreaker
Copy link
Contributor

@lonerzzz
When END triggers I2C_END_DETECT_INT, how do you respond?
fill CMD[] with next sequence
Clear INT bit: I2C_INT_CLR_REG &= ~I2C_END_DETECT_INT_RAW_M;
Start processing I2C_CTR_REG |= I2C_TRANS_START;
return;

Chuck.

@stickbreaker
Copy link
Contributor

@AlyoshaVasilieva I think I have created a solution for your problem #839

Chuck.

@copercini
Copy link
Contributor

The I2C core was changed to @stickbreaker code officially after 13dcfe5 (thanks @stickbreaker), Let's close issues with old code and if have problems with the new code, open new issues =)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants