dawsonjon / PicoRX

Build a SDR SW/MW/LW Receiver with a Raspberry Pi Pico
Other
252 stars 32 forks source link

replace settings[idx_blahdeblah] ? #116

Open penfold42 opened 1 month ago

penfold42 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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

penfold42 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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.