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

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

Open
w-winter opened this issue Dec 10, 2020 · 4 comments
Open

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

w-winter opened this issue Dec 10, 2020 · 4 comments

Comments

@w-winter
Copy link

w-winter commented Dec 10, 2020

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

Chysn commented Dec 10, 2020 via email

@patrickdowling
Copy link
Collaborator

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

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

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.

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

3 participants