arduino / ArduinoCore-samd

Arduino Core for SAMD21 CPU
GNU Lesser General Public License v2.1
475 stars 722 forks source link

I2S.end() hangs #386

Closed wjmb closed 5 years ago

wjmb commented 5 years ago

On Feather M0 LoRa (Adafruit 3178) with ICS43432 (Pesky products), I run into an issue when calling I2S.end(). When called for the second time, it stalls there.

I want to begin and end I2S within a function, rather than beginning it once and never ending it. When trying to debug it by putting in a bunch of Serial.println("dbg"); statements in the I2S.end() implementation, it doesn't stall. When I remove them one by one, starting at the last one, it stalls at

i2sd.disableSerializer(_deviceIndex); i2sd.disableClockUnit(_deviceIndex);

The minimal sketch in which I see this issue is I2S_end_issue.txt

Any advice on what I might be doing wrong would be appreciated!

amotl commented 5 years ago

I've put the minimal example reproducing the error into a Gist to make it look better, see Basic audio recorder for ARM Cortex-M0 and ICS43432 I2S microphone.

Thanks in advance for your efforts, @sandeepmistry and @Rocketct!

Rocketct commented 5 years ago

Hi @wjmb @amotl i have tested this sketch and reproduced the issue, seem that the end hangs in wait of i2s.SYNCBUSY.bit.SEREN0 in the API i2sd.disableSerializer(_deviceIndex);, this could means that hang because the communication is not ended. Due to the fact that i don't have here your mic device i make two experiments, first i had simulate a I2S communication with another arduino board and second i test the I2S using another mi,c in both case i had note that by setting correctly the parameters of I2S.begin(I2S_PHILIPS_MODE, SAMPLE_RATE, 32) all goes fine and the hangs not happens. @wjmb i look at your product, i have find the following one, let me know if is correct, https://www.tindie.com/products/onehorse/ics43432-i2s-digital-microphone/ , in the description is written that Signed 24 Bit PCM than could you try your setup by make the following change in your sketch:

This should make works your sketch

amotl commented 5 years ago

Dear @Rocketct,

thank you so much for taking the time to have a look at the issue we are having.

Setting the bitsPerSample (the desired bits per sample (i.e 8, 16, 32)) parameter of the I2S.begin() method correctly sounds perfectly reasonable.

As I don't have any hardware on my workbench, I am looking forward to any feedback from @wjmb here.

With kind regards, Andreas.

wjmb commented 5 years ago

Dear @Rocketct and @amotl,

Thanks for looking into this.

Indeed, https://www.tindie.com/products/onehorse/ics43432-i2s-digital-microphone/ is the product that I use. Although the microphones precision is 24 bits, I might have been mislead by the 32 bit word specified in the datasheet:

image

I will try your suggestions (24 bit in one test and higher sampling frequency in another test) and report back on the result.

Best regards,

Wouter.

amotl commented 5 years ago

Would probably have been confused either.

On the one hand, they are saying

The 24-bit serial data is embedded in a 32-bit serial word with eight bits of tri-state padding. -- https://www.tindie.com/products/onehorse/ics43432-i2s-digital-microphone/

but the data sheet is very clear on that on the other hand.

The slave serial data port’s format is I²S, 24‐bit, twos complement. -- https://www.invensense.com/wp-content/uploads/2015/02/ICS-43432-data-sheet-v1.3.pdf

I will try your suggestions (24 bit in one test and higher sampling frequency in another test) and report back on the result.

Good luck!

amotl commented 5 years ago

The data sheet also has further important information:

See https://www.invensense.com/wp-content/uploads/2015/02/ICS-43432-data-sheet-v1.3.pdf, page 10 ff.

wjmb commented 5 years ago

Dear @Rocketct and @amotl,

Unfortunately, neither suggestion yielded the desired result. Here are my observations:

Starting Failed to initialize I2S!

I hope this helps.

Rocketct commented 5 years ago

Hi @wjmb seems that in this casae there is just some code written by another user, in his sketch it set the begin int this way:

if(!I2S.begin(I2S_PHILIPS_MODE, 16000, 32))

here the code https://github.com/kriswiner/Butterfly/blob/master/ICS43434_USBSerial.ino

then, SAMPLE_RATE seems that should be 16000, seems that with this product you should set the bitsPerSample to 32, i suggest to try first the example's one, after if the end hang again, change 32 with 24 because in the datasheet https://d3s5r33r268y59.cloudfront.net/datasheets/10069/2017-07-15-04-17-34/DS-000069-ICS-43434-v1.2.pdf is written that also this use a Digital I²Sinterface with high precision 24-bit data than the serialize could stay in wait again for the reason discussed before. Abuot the frequency maybe could not works 25000 because is written that in low power mode, the supported sample rate frequency are in the range of 6.25 –18.75kHz instead in high definition mode are supported only the frequency in 23–51.6kHz, than as test i suggest to do:

let me know the results

wjmb commented 5 years ago

Hi @Rocketct,

Both tests didn't give any new insight w.r.t. the previous ones: behaviour at 16 kHz is the same as at 12.5 and at 25 kHz. 24 bits gives the same message about failing to initialise.

It appears to me that 32 bit is the right setting for this microphone (as supported by the datasheet, where the clock counts through 32 bits per word. That only 24 of these have significance is for the application to figure out).

Also, this microphone supports a range of frequencies and I see code snippets on the internet that range from low to the typical 44.1 kHz for audio. This is consistent with the fact that at the various frequencies I do see similar behaviour.

I was wondering about the following: the reason for me to call begin() and end() is because I want to disable the microphone temporarily and any interrupt activity associated with it. Is there another public method to this library that accomplishes this? Or could you make one available publically?

Best regards,

Wouter.

Rocketct commented 5 years ago

@wjmb no the only way to disable the I2s interrupt reception is to call the end(), there is another way what can i suggest is to use the API interrupt() and noInterrupts(), https://www.arduino.cc/reference/en/language/functions/interrupts/nointerrupts/ but in this case you disable all the interrupt activity of the board, not only the I2S one, clearly depend on what you need to do, but could be a solution for your problem.

Rocketct commented 5 years ago

closed due to lack of feedback @wjmb if you need reopen the issue