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

DRAFT: Fixes for I2S #7117

Closed
wants to merge 58 commits into from
Closed

DRAFT: Fixes for I2S #7117

wants to merge 58 commits into from

Conversation

PilnyTomas
Copy link
Contributor

@PilnyTomas PilnyTomas commented Aug 12, 2022

Description of Change

Few various fixes for I2S
WARNING - this is not yet tested and still may contain crashing bugs!

Major changes

  1. Support for second I2S module on ESP32 Second I2S on ESP32 non-working #7111.
  2. README and comments in the header file with expected behavior were added.
  3. 24bit support 24 _bitsPerSample not handled in I2SClass::read() #7107.
  4. Added CI tests.

Minor changes

  • Added mode to string array used in outputs: print("Current mode is %s\n", i2s_mode_text[mode]);
  • Extended function documentation.
  • Proprietary functions using normal mutex and counting recursion named _take_if_not_holding and _give_if_top_call has been simplified using recursive mutex and renamed to _take_mux and _give_mux. If the function fails it will still print log_e and return 0. If it succeeds it will return 1.
  • log_x will now also print the device index to distinct which object / I2S module is printing this message.
  • Added description for modes.
  • simple functions setBufferSize and getBufferSize has been renamed and extended to setDMABufferFrameSize, getDMABufferFrameSize, getDMABufferSampleSize``getDMABufferByteSize and also new functions getRingBufferSampleSize, getRingBufferByteSize has been added.
  • New getters has been added: getI2SNum, isInitialized, getSampleRate, getBitsPerSample.
  • Private variable _buffer_byte_size has removed completely - instead of this is used function getRingBufferByteSize.
  • Function availableForWrite has now another variant availableSamplesForWrite.
  • Private variable _i2s_dma_buffer_size has been renamed to _i2s_dma_buffer_frame_size to describe the unit.
  • Constructor sets default data pins based on deviceIndex.
  • Added data fix for 24 bps
  • Added data fix for I2S_RIGHT_JUSTIFIED_MODE and I2S_LEFT_JUSTIFIED_MODE.

Tests scenarios

Performed for Example sketches with following options:
SimpleTone, Philis mode: 8bps + 8000 Hz; 8bps + 44100 Hz; 32bps + 8000 Hz; 32bps + 44100 Hz;
SimpleTone, DAC mode: 16bps
InputSerialPlotter: 32bps, 8000 Hz and 32000 Hz
ADCPlotter: Only 16 bps is allowed, any other bps correctly prints Err msg and stops. Tested Sampling rates: 8000 and 44100.

Example Sketch ESP32 ESP32-S3 ESP32-C3 ESP32-S2
SimpleTone, Philis mode
SimpleTone, DAC mode OK
InputSerialPlotter OK
ADCPlotter OK N/A N/A N/A

Related links

#7111, #7107

Known issues

  • Example InputSerialPlotter on my hardware reads lots of zeroes. If those are filtered out and not printed it may seem that the input is lagging.
  • ADC can be noisy - signal filtering is advised

@SuGlider SuGlider marked this pull request as draft August 12, 2022 11:20
@VojtechBartoska VojtechBartoska added this to the 2.0.6 milestone Oct 5, 2022
@VojtechBartoska
Copy link
Contributor

VojtechBartoska commented Oct 5, 2022

later on, consider adding HW tests.

Ideas:

  • just compilation
  • through GPIO (sender & receiver)

@VojtechBartoska VojtechBartoska added the Status: Awaiting triage Issue is waiting for triage label Jan 3, 2023
@VojtechBartoska VojtechBartoska removed this from the 2.0.7 milestone Jan 3, 2023
@CLAassistant
Copy link

CLAassistant commented May 6, 2023

CLA assistant check
All committers have signed the CLA.

@VojtechBartoska
Copy link
Contributor

@me-no-dev This PR can be later closed in favor of new implementation of I2S

@VojtechBartoska VojtechBartoska self-assigned this Aug 15, 2023
@VojtechBartoska
Copy link
Contributor

new I2S library was implemented here: #8714

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Peripherals API Relates to peripheral's APIs. Status: Awaiting triage Issue is waiting for triage
Projects
Development

Successfully merging this pull request may close these issues.

3 participants