mxmxmx / O_C

display w/ dac
472 stars 122 forks source link

VBiasManager parameters are not saved by "Save Settings" #108

Open w-winter opened 3 years ago

w-winter commented 3 years ago

The VOR option chosen through VBiasManager is not saved by save_global_settings() or save_app_data(). It must always be re-selected every time the module is powered up. Also, every time you do the Save Settings action, the last-selected VOR setting is immediately forgotten. Apologies if discussing VOR in an issue here is considered outside the scope of the main O_C repo.

@Chysn (tagging you because of https://github.com/mxmxmx/O_C/pull/90 - thanks for that! ), is there a hardware limitation preventing the chosen VOR option to be saved to EEPROM? That would be a shame. Or do the aforementioned save methods (and/or all of the apps' own save functions; for example, https://github.com/mxmxmx/O_C/blob/master/software/o_c_REV/APP_ENVGEN.ino#L852-L857 ) still need to be updated? Or maybe SettingsBase.Save in util_settings?

Chysn commented 3 years ago

@Chysn https://github.com/Chysn (tagging you because of #90 https://github.com/mxmxmx/O_C/pull/90 - thanks for that! ), is there a hardware limitation preventing the chosen VOR option to be saved to EEPROM? That would be a shame. Or do the aforementioned save methods (and/or all of the apps' own save functions; for example, https://github.com/mxmxmx/O_C/blob/master/software/o_c_REV/APP_ENVGEN.ino#L852-L857 ) still need to be updated? Or maybe SettingsBase.Save in util_settings?

No hardware limitation, but it's a matter of allocating very limited space. I can't say whether such opportunities still exist in the Ornament and Crime firmware.

--Jason

On Wed, Dec 9, 2020 at 10:00 PM W. Winter notifications@github.com wrote:

The VOR option chosen through VBiasManager is not saved by save_global_settings() or save_app_data(). It must always be re-selected every time the module is powered up. Apologies if discussing VOR in an issue here is considered outside the scope of the main O_C repo.

@Chysn https://github.com/Chysn (tagging you because of #90 https://github.com/mxmxmx/O_C/pull/90 - thanks for that! ), is there a hardware limitation preventing the chosen VOR option to be saved to EEPROM? That would be a shame. Or do the aforementioned save methods (and/or all of the apps' own save functions; for example, https://github.com/mxmxmx/O_C/blob/master/software/o_c_REV/APP_ENVGEN.ino#L852-L857 ) still need to be updated? Or maybe SettingsBase.Save in util_settings?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mxmxmx/O_C/issues/108, or unsubscribe https://github.com/notifications/unsubscribe-auth/AENMQVHP3VEE4ZG3MWSO2N3SUA2URANCNFSM4UUMITVQ .

patrickdowling commented 3 years ago

Yeah, space is a bit tight but I think it'd fit somewhere. Shay mentioned wanting it to be saved per app which is a bit more involved (it'd either have to be in all the _save/_loads, or added as a (hidden?) entry in the settings arrays). The one issue is that it will likely break existing settings data, and possibly calibration.

Right now the VOR stuff is in a bit of limbo. I don't really think it should live in this repo - or rather, I don't think we should be fielding support for it - but AFAIK there's no other plan. There's a few other unsolved issues as well.

w-winter commented 3 years ago

Thanks for clarifying @patrickdowling and @Chysn . I understand that there are a couple of other issues besides storing the VOR settings: VOR breaking autotune, and a ~3mV offset between the calibration interface and normal operation of the DAC.

The VOR storage part irks me and I'd like to find someone to help with these issues. I also understand that you'd prefer for further development of the VOR features to not live within the O_C repo. Would it work if Shay forked this repo and further VOR work happened there, or would a more radical break than forking be preferable?

patrickdowling commented 3 years ago

The incomplete state kind of irks me as well (even if it's not really my circus). My personal preference would be that Shay have his own fork(s), put the releases and track his hardware changes there, so it also becomes the default port of call for support issues. The various #defines have gotten a bit out of hand.

To be fair, Shay and I were in touch a while back, but all other issues aside, to fix things the way I'd like them done requires more time investment than I've wanted to divert from other things.