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

Added possibility to change DOUT pin. #182

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

frederic-bonjour
Copy link

Use AudioOutputI2S::DOUT = 27; before creating any instance of AudioOutputI2S.

If you use I2C as well, do not forget to add pull-up resistors on SDA and SCL lines to avoid any problem ;)

Use `AudioOutputI2S::DOUT = 27;` before creating any instance of `AudioOutputI2S`.
Copy link

@meltdown03 meltdown03 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your changes include the deletion of line 85 of the source file which wouldn't effect the esp32 as it's in the #else section. An accident perhaps?

@AidanTek
Copy link

AidanTek commented Jul 3, 2019

Is there any reason why LRCLK and BCLK pins can't be changed as well? I think other pins on the ESP32 are capable and not all breakouts (like the HUZZAH32) provide access to the pins that are hard coded in the library - to me it makes more sense in usage if the pins can be optionally changed when the class is initialised, something like:

AudioOutputI2S *out;

void setup() {
    out = new AudioOutputI2S(15, 14, 32); // lrclk, bclk, dout
}

I don't know lots about the ESP32 or I2S but I could have a stab at making this change - I am very busy at the moment so I couldn't do it immediately

@meltdown03
Copy link

You can change it fairly easily right after creating the "out" instance:
out -> SetPinout(15, 14, 32);

Now it would be using those pins I believe.

@AidanTek
Copy link

AidanTek commented Jul 3, 2019

You can change it fairly easily right after creating the "out" instance:
out -> SetPinout(15, 14, 32);

Now it would be using those pins I believe.

I don't understand the purpose of the pull request in that case??

@meltdown03
Copy link

Yeah, I guess it's not really needed as long as that works. I can test it later and let you know unless someone else wants to try it.

@AidanTek
Copy link

AidanTek commented Jul 3, 2019

I tried it already

out -> SetPinout()

Works fine

Edit: just a thought but maybe a documentation PR would be helpful?

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

Successfully merging this pull request may close these issues.

None yet

3 participants