search

image-et-son / p600fw

GliGli based Prophet 600 firmware upgrade
10 stars 4 forks source link

Bug: Vintage and Ext Voltage values are not stored in patch, always zero on load #53

Closed el-folie closed 2 years ago

el-folie commented 2 years ago

Work around so far: Switching to Preset mode and back to Live mode.

Complete description with audio examples in the gearslutz thread.

el-folie commented 2 years ago

While the Spread effect now returns to 0 whith knob value at 0 there´s still one problem:

When shortly switching to preset mode (e.g. to compare to another sound) and then back to panel mode, the Spread setting is lost and reset to 0.

In contrast to that, the last Detune setting is being retained in panel memory, so the last Spread value not staying in panel memory must be parameter dependent and not "panel mode memory save routine"-dependent.

el-folie commented 2 years ago

It´s still like this on my P600: Switching from manual mode (with some Spread value) shortly to preset and back to panel mode, the Spread value gets lost and set to zero.

Does it really work on your P600s?

matrix12x commented 2 years ago

I confirm I have the same exact issue as described by @el-folie

el-folie commented 2 years ago

Thanks for confirming Matrix12x.

matrix12x commented 2 years ago

@image-et-son you think the changes to storage.c and storage.h may have caused this? I have had that happen. if something is re-ordered or renamed.

image-et-son commented 2 years ago

There is some very weird behaviour with transfer of menu parameter value to the live mode patch (to retrieve when switching to and from preset mode) and storage. The ext voltage and and the vintage values are not stored in either and reset to zero. Also when storing patches from live mode some menu parameters are reset to the last stored live mode patch, but only if they were the last value to be changed. This is a mystery and very important to resolve.

matrix12x commented 2 years ago

I confirm the Ext Voltage parameter not being stored. In preset mode I modified a patch to add the Ext Voltage (noise on my P600) and I hit record to save the preset (for example in spot 01) then without hitting anything after the save, the Ext Voltage defaults back to 00. Same thing happens if I switch to manual and then back to preset, or if I set the ext voltage while in manual and then switch to preset and back to manual, the Ext Voltage goes back to 00.

looking at storage.c I don't see where cpExternal is stored.

for example I see where cpSeqArpClock is stored,

for(cp=cpModDelay;cp<cpSeqArpClock;++cp) // skip arp/seq clock storageWrite16(currentPreset.continuousParameters[cp]);

and where cpSpread should be stored: storageWrite16(currentPreset.steppedParameters[cpSpread]);

I think cpExternal might have been missed. Or I am missing it in reading the code

also, on a side note, I noticed this one was flipped around at some point vs the original GliGli code: currentPreset.steppedParameters[holdPedal]=currentPreset.steppedParameters[spAmpEnvSlow]; What does it correct for?

image-et-son commented 2 years ago

Hi, sorry, the external CV was missing in 11 but I have it in my local code with exactly the same error or for spread/vintage.

The hold pedal thing is a little crazy: since the parameter is in MIDI CC 64 it is in the right place so you can use the sp index directly for mapping to CC values (this type of design saves a lot of glue code :-), so is good). But for the patch storage the hold pedal is irrelevant. So instead of the hold pedal at that position the amp envelope speed is stored. And the sp for amp envelope speed (25?) later on is skipped in patch storage. I today added more commenting on this :-)

I have solved one problem: the menu parameter values form live more are only stored when you select another menu parameter, not when you change the value (which is OK) but when you store the patch from live live mode the code goes back to live mode an recovers the latest saved value (which may be different). So in the storage procedure I also store the latest live mode now.

But the non-saved spread and ext volt remains obscure to me

image-et-son commented 2 years ago

OK, that was a stupid mistake. I referred to .steppedParameters[cpSpread] instead of .continuousParameters[cpSpread] in the code.

At least I found the error above in the processes.

matrix12x commented 2 years ago

cool. and that makes sense for cpSpread.

el-folie commented 2 years ago

Tested - works, values are retained when switching to preset and back top panel.

matrix12x commented 2 years ago

Alpha 12: I confirm Ext Voltage(noise) and Vintage(spread) store correctly