baldram / ESP_VS1053_Library

A library for VS1053 MP3 Codec Breakout adapted for Espressif ESP8266 and ESP32 boards.
https://platformio.org/lib/show/1744/ESP_VS1053_Library
GNU General Public License v3.0
114 stars 36 forks source link

About Mp3PlayerDemo: switchToMp3Mode() before loadDefaultVs1053Patches #89

Open sebhuet opened 2 years ago

sebhuet commented 2 years ago

Hi,

IHMO, correct order should be

player.switchToMp3Mode();
if (player.getChipVersion() == 4) { // Only perform an update if we really are using a VS1053, not. eg. VS1003
    player.loadDefaultVs1053Patches(); 
}

otherwise player.switchToMp3Mode() reset feature remove patches.

Dr-Dawg commented 2 years ago

I think you are right, the vs1053b-patches.pdf states:

The patch must be re-loaded after each hardware or software reset. If you replace software reset by writing 0x50 to AIADDR, you do not need to reload the patch.

player.switchToMp3Mode(); performs a softReset with

writeRegister(SCI_MODE, _BV(SM_SDINEW) | _BV(SM_RESET));

So the patch is overwritten. Maybe, apart from changing the order of the commands, we should seek for an easy way to check whether the patch is active..

baldram commented 2 years ago

I think you are right, the vs1053b-patches.pdf states. he patch must be re-loaded after each hardware or software reset.

Ok, so both documentation and examples are affected. Changing the order would be the simplest solution. But...

Maybe, apart from changing the order of the commands, we should ...

Do I understand right that you are in doubt that reordering might have other undesirable results?

we should seek for an easy way to check whether the patch is active..

Not sure I get it right. Then if we find the way to check, what is the goal? First, load the patch in the original order. If the patch is not active after switching the mode, retry loading?

Dr-Dawg commented 2 years ago

Nope, I think reordering ist the solution.

It's just that for the second time we thought we are using the patch and actually we are not, see #66. So I just expressed the wish for some hint that would clearly indicate the patch is uploaded. Unfortunately, I have no idea, so I'd suggest to change order.

baldram commented 2 years ago

Are you willing to change docs and examples?

Dr-Dawg commented 2 years ago

Yep, I can do this..

Dr-Dawg commented 2 years ago

see #83

CelliesProjects commented 2 years ago

It's just that for the second time we thought we are using the patch and actually we are not, see https://github.com/baldram/ESP_VS1053_Library/discussions/66.

We need another miss to get to three strikes.

8D

baldram commented 2 years ago

It's just that for the second time we thought we are using the patch and actually we are not, see https://github.com/baldram/ESP_VS1053_Library/discussions/66.

We need another miss to get to three strikes.

8D

@CelliesProjects I got lost after looking at the code change with comment "apply patches before setting...", but looking at the change it's "after".

Do we need any additional adjustments here in this library?

Dr-Dawg commented 2 years ago

Yes, we do. But the adjustsments are already contained in MR #83.

Like @sebhuet said, it's basically just about performing player.loadDefaultVs1053Patches() after player.switchToMp3Mode in order to keep the update alive. That implemented, we can wait for the third strike ;-)

@baldram Did you had any chance to re-setup your workshop and test the MR yet?

baldram commented 2 years ago

@Dr-Dawg We lack of testers around, so I need to do so urgently to open the line of MRs and also fix the conflict.