m5stack / M5Core2

M5Core2 Arduino Library
MIT License
262 stars 113 forks source link

Adding possiblity to not initialize speaker #121

Closed OekoSolveMG closed 1 year ago

OekoSolveMG commented 1 year ago

Currently the library always initalizes the speaker, this can cause problems tough because if the W5500 V12 is used in combination with the Core2 it uses Pin 33 for RX and Pin 2 for TX.

This TX pin is the same pin that is used for the initalizing of the I2S bus where Pin 2 is the data out pin, this results in the speaker playing noise every time data is sent over the TX line.

Additionally the Speaker has been adjusted to simply allow to pass a pointer to an array of bytes so any sound could be played and additionally the include on a .c file has been removed.

This issue exists since Version 1.4.0, where the Speaker example has been integrated into the library itself.

@Tinyu-Zhao Would be appreciated if this could be merged

tobozo commented 1 year ago

thanks for this, very interesting!

the fature is valid however the integration is inconsistent: I2CEnable and SpeakerEnable are inverted.

M5Core2.h image

M5Core2.cpp image

Moreover, this integration will break many existing projects using the mbus_mode_t argument in, M5.begin().

A more suitable solution must be found to introduce this feature (e.g. adding SpeakerEnable at the end, not in the middle of the begin() arguments list, or creating a enableSpeaker() function).

OekoSolveMG commented 1 year ago

@tobozo Sounds good I will adjust it to be at the end of the argument list and use the default parameter of true, this will keep the code for everyone the same (speaker was previously always initalised as well) and add the possiblity to not initalise it if it causes problems.

ayasystems commented 1 year ago

The same here. Speak is generating noise with can module

ayasystems commented 1 year ago

@Tinyu-Zhao please merge the change

nickn17 commented 1 year ago

We need this patch too. Thank you for merge.