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

replace settings[idx_blahdeblah] ? #116

Open
penfold42 opened this issue Oct 3, 2024 · 6 comments
Open

replace settings[idx_blahdeblah] ? #116

penfold42 opened this issue Oct 3, 2024 · 6 comments

Comments

@penfold42
Copy link
Contributor

With some of the changes recently I've tried to use bits sparingly.

As a result there's some pretty ugly bit masking/shifting scattered through ui.cpp.
Should we ditch the settings array and replace it with an extended rx_settings struct ?
I suspect we'd need a working copy in ui:: that the config menus can fiddle with.

Then we can shift all the bit-shifting and stuffing into the autosave/recall functions.
And probably do some input validation while we're at it.

grep 'settings\[' ui.cpp | wc -l
145

Its a stack of work, but I'm up for it.
Thoughts?

@dawsonjon
Copy link
Owner

The rx settings struct should be kept separate. It forms the interface between the UI and the receiver, so it should only contain things that are needed to control rx.cpp.

On the other hand, it is probably a good idea to replace the settings array with a struct of the same size using bit fields or similar instead. It's worth keeping in mind that only a subset of the settings array gets stored in a memory channel, but everything gets autosaved, so it needs to be partitioned somehow.

I have also been starting to think that the definition of settings, store, recall, autosave and apply settings should be in their own file separate from the UI.

@penfold42
Copy link
Contributor Author

And on a tangentially related topic, we need to look at the best way to make the UI make radio changes in real time rather than needing to "Apply"

@dawsonjon
Copy link
Owner

Doesn't it make sense to keep control of the receiver transactional?

@penfold42
Copy link
Contributor Author

If I'm adjusting the volume or filter bandwidth I'd like to hear the change in real-time as im twiddling the knob :)

@dawsonjon
Copy link
Owner

I see what you mean, so effectively apply_settings would be changed each time you turned the knob, pressing the menu(ok) button would call autosaved and pressing the back(cancel) button would put back the original setting, apply and exit.

Maybe we should wrap settings/recall/store/autosave/apply_settings in a settings/control class? We could have getters and setters for each setting (that cause the setting to be immediately applied to the receiver where appropriate) and maybe an apply() method that autosaves the settings and a cancel() method that restores the previously saved settings and applies those to the receiver?

It might also be a neat way to hide all the bit manipulation too potentially.

@penfold42
Copy link
Contributor Author

Yep, yep and yep :)

I toyed with passing call back pointers into number_entry(), enumerate_entry() but it was going to get super ugly and what you suggest is the right way to do it.

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

2 participants