ESP_VS1053_Library icon indicating copy to clipboard operation
ESP_VS1053_Library copied to clipboard

About Mp3PlayerDemo: switchToMp3Mode() before loadDefaultVs1053Patches

Open sebhuet opened this issue 3 years ago • 10 comments

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.

sebhuet avatar Feb 27 '22 14:02 sebhuet

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..

Dr-Dawg avatar Feb 27 '22 18:02 Dr-Dawg

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?

baldram avatar Mar 22 '22 20:03 baldram

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.

Dr-Dawg avatar Mar 23 '22 07:03 Dr-Dawg

Are you willing to change docs and examples?

baldram avatar Mar 25 '22 21:03 baldram

Yep, I can do this..

Dr-Dawg avatar Mar 26 '22 11:03 Dr-Dawg

see #83

Dr-Dawg avatar Mar 26 '22 12:03 Dr-Dawg

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 avatar Jul 04 '22 14:07 CelliesProjects

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?

baldram avatar Jul 04 '22 17:07 baldram

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?

Dr-Dawg avatar Jul 05 '22 10:07 Dr-Dawg

@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.

baldram avatar Jul 05 '22 10:07 baldram