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

About Mp3PlayerDemo: switchToMp3Mode() before loadDefaultVs1053Patches #89

Open
sebhuet opened this issue Feb 27, 2022 · 10 comments
Open

Comments

@sebhuet
Copy link

sebhuet commented Feb 27, 2022

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
Copy link

Dr-Dawg commented Feb 27, 2022

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
Copy link
Owner

baldram commented Mar 22, 2022

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
Copy link

Dr-Dawg commented Mar 23, 2022

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
Copy link
Owner

baldram commented Mar 25, 2022

Are you willing to change docs and examples?

@Dr-Dawg
Copy link

Dr-Dawg commented Mar 26, 2022

Yep, I can do this..

@Dr-Dawg
Copy link

Dr-Dawg commented Mar 26, 2022

see #83

CelliesProjects added a commit to CelliesProjects/ESP32_VS1053_Stream that referenced this issue Jul 4, 2022
@CelliesProjects
Copy link

CelliesProjects commented Jul 4, 2022

It's just that for the second time we thought we are using the patch and actually we are not, see #66.

We need another miss to get to three strikes.

8D

@baldram
Copy link
Owner

baldram commented Jul 4, 2022

It's just that for the second time we thought we are using the patch and actually we are not, see #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
Copy link

Dr-Dawg commented Jul 5, 2022

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
Copy link
Owner

baldram commented Jul 5, 2022

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

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

No branches or pull requests

4 participants