From 7ebf3d3cdd2ec11f5fece5716d352022f0650c4e Mon Sep 17 00:00:00 2001 From: Kyle Reed <3761006+kallanreed@users.noreply.github.com> Date: Sun, 18 Jun 2023 22:48:29 -0700 Subject: [PATCH] Fix hang in Audio when switching from Capture app, & bug fixes. (#1167) * Fix hang in Audio when switching from Capture app * Bug fixes --- .../application/apps/analog_audio_app.cpp | 99 ++++++++----------- .../application/apps/analog_audio_app.hpp | 32 ++---- firmware/application/apps/capture_app.cpp | 2 + .../application/apps/ui_looking_glass_app.hpp | 3 + firmware/application/apps/ui_mictx.cpp | 6 -- firmware/application/apps/ui_playlist.cpp | 6 ++ firmware/application/apps/ui_scanner.hpp | 2 +- firmware/application/radio_state.hpp | 11 ++- firmware/application/ui/ui_spectrum.cpp | 2 + firmware/application/ui/ui_spectrum.hpp | 4 + 10 files changed, 78 insertions(+), 89 deletions(-) diff --git a/firmware/application/apps/analog_audio_app.cpp b/firmware/application/apps/analog_audio_app.cpp index ee7d2105..0a2dfbe7 100644 --- a/firmware/application/apps/analog_audio_app.cpp +++ b/firmware/application/apps/analog_audio_app.cpp @@ -22,21 +22,19 @@ #include "analog_audio_app.hpp" +#include "audio.hpp" #include "baseband_api.hpp" - +#include "file.hpp" +#include "freqman.hpp" #include "portapack.hpp" #include "portapack_persistent_memory.hpp" -using namespace portapack; -using namespace tonekey; - -#include "audio.hpp" -#include "file.hpp" - +#include "string_format.hpp" #include "utility.hpp" -#include "string_format.hpp" +#include "debug.hpp" -#include "freqman.hpp" +using namespace portapack; +using namespace tonekey; namespace ui { @@ -45,8 +43,8 @@ namespace ui { static const Style& style_options_group = Styles::bg_blue; AMOptionsView::AMOptionsView( - const Rect parent_rect, - const Style* const style) + Rect parent_rect, + const Style* style) : View{parent_rect} { set_style(style); @@ -65,8 +63,8 @@ AMOptionsView::AMOptionsView( /* NBFMOptionsView *******************************************************/ NBFMOptionsView::NBFMOptionsView( - const Rect parent_rect, - const Style* const style) + Rect parent_rect, + const Style* style) : View{parent_rect} { set_style(style); @@ -90,8 +88,8 @@ NBFMOptionsView::NBFMOptionsView( /* WFMOptionsView *******************************************************/ WFMOptionsView::WFMOptionsView( - const Rect parent_rect, - const Style* const style) + Rect parent_rect, + const Style* style) : View{parent_rect} { set_style(style); @@ -111,8 +109,8 @@ WFMOptionsView::WFMOptionsView( SPECOptionsView::SPECOptionsView( AnalogAudioView* view, - const Rect parent_rect, - const Style* const style) + Rect parent_rect, + const Style* style) : View{parent_rect} { set_style(style); @@ -137,6 +135,10 @@ SPECOptionsView::SPECOptionsView( AnalogAudioView::AnalogAudioView( NavigationView& nav) : nav_(nav) { + // A baseband image _must_ be running before + // interacting with the waterfall view. + baseband::run_image(portapack::spi_flash::image_tag_wideband_spectrum); + add_children({&rssi, &channel, &audio, @@ -164,9 +166,12 @@ AnalogAudioView::AnalogAudioView( this->on_show_options_rf_gain(); }; - const auto modulation = receiver_model.modulation(); - options_modulation.set_by_value(toUType(modulation)); + auto modulation = receiver_model.modulation(); + // This app doesn't handle "Capture" mode. + if (modulation > ReceiverModel::Mode::SpectrumAnalysis) + modulation = ReceiverModel::Mode::SpectrumAnalysis; + options_modulation.set_by_value(toUType(modulation)); options_modulation.on_change = [this](size_t, OptionsField::value_t v) { this->on_modulation_changed(static_cast(v)); }; @@ -184,8 +189,9 @@ AnalogAudioView::AnalogAudioView( audio::output::start(); - update_modulation(static_cast(modulation)); - on_modulation_changed(static_cast(modulation)); + // This call starts the correct baseband image to run + // and sets the radio up as necessary for the given modulation. + on_modulation_changed(modulation); } size_t AnalogAudioView::get_spec_bw_index() { @@ -212,40 +218,15 @@ void AnalogAudioView::set_spec_trigger(uint16_t trigger) { } AnalogAudioView::~AnalogAudioView() { - // // save app settings - // app_settings.rx_frequency = field_frequency.value(); - // app_settings.lna = receiver_model.lna(); - // app_settings.vga = receiver_model.vga(); - // app_settings.rx_amp = receiver_model.rf_amp(); - // app_settings.step = receiver_model.frequency_step(); - // app_settings.modulation = (uint8_t)receiver_model.modulation(); - // app_settings.am_config_index = receiver_model.am_configuration(); - // app_settings.nbfm_config_index = receiver_model.nbfm_configuration(); - // app_settings.wfm_config_index = receiver_model.wfm_configuration(); - // app_settings.squelch = receiver_model.squelch_level(); - // settings.save("rx_audio", &app_settings); - - // TODO: Manipulating audio codec here, and in ui_receiver.cpp. Good to do - // both? audio::output::stop(); - - receiver_model.set_sampling_rate(3072000); // Just a hack to avoid hanging other apps if the last modulation was SPEC receiver_model.disable(); - baseband::shutdown(); } -void AnalogAudioView::on_hide() { - // TODO: Terrible kludge because widget system doesn't notify Waterfall that - // it's being shown or hidden. - waterfall.on_hide(); - View::on_hide(); -} - -void AnalogAudioView::set_parent_rect(const Rect new_parent_rect) { +void AnalogAudioView::set_parent_rect(Rect new_parent_rect) { View::set_parent_rect(new_parent_rect); - const ui::Rect waterfall_rect{0, header_height, new_parent_rect.width(), new_parent_rect.height() - header_height}; + ui::Rect waterfall_rect{0, header_height, new_parent_rect.width(), new_parent_rect.height() - header_height}; waterfall.set_parent_rect(waterfall_rect); } @@ -257,13 +238,15 @@ void AnalogAudioView::on_baseband_bandwidth_changed(uint32_t bandwidth_hz) { receiver_model.set_baseband_bandwidth(bandwidth_hz); } -void AnalogAudioView::on_modulation_changed(const ReceiverModel::Mode modulation) { - // TODO: Terrible kludge because widget system doesn't notify Waterfall that - // it's being shown or hidden. - waterfall.on_hide(); +void AnalogAudioView::on_modulation_changed(ReceiverModel::Mode modulation) { + // This app doesn't know what to do with "Capture" mode. + if (modulation > ReceiverModel::Mode::SpectrumAnalysis) + modulation = ReceiverModel::Mode::SpectrumAnalysis; + + baseband::spectrum_streaming_stop(); update_modulation(modulation); on_show_options_modulation(); - waterfall.on_show(); + baseband::spectrum_streaming_start(); } void AnalogAudioView::remove_options_widget() { @@ -315,7 +298,7 @@ void AnalogAudioView::on_show_options_rf_gain() { void AnalogAudioView::on_show_options_modulation() { std::unique_ptr widget; - const auto modulation = static_cast(receiver_model.modulation()); + const auto modulation = receiver_model.modulation(); switch (modulation) { case ReceiverModel::Mode::AMAudio: widget = std::make_unique(options_view_rect, &style_options_group); @@ -342,6 +325,7 @@ void AnalogAudioView::on_show_options_modulation() { break; default: + chDbgPanic("Unhandled Mode"); break; } @@ -358,7 +342,7 @@ void AnalogAudioView::on_reference_ppm_correction_changed(int32_t v) { persistent_memory::set_correction_ppb(v * 1000); } -void AnalogAudioView::update_modulation(const ReceiverModel::Mode modulation) { +void AnalogAudioView::update_modulation(ReceiverModel::Mode modulation) { audio::output::mute(); record_view.stop(); @@ -379,7 +363,8 @@ void AnalogAudioView::update_modulation(const ReceiverModel::Mode modulation) { image_tag = portapack::spi_flash::image_tag_wideband_spectrum; break; default: - return; + chDbgPanic("Unhandled Mode"); + break; } baseband::run_image(image_tag); @@ -418,7 +403,7 @@ void AnalogAudioView::update_modulation(const ReceiverModel::Mode modulation) { } } -void AnalogAudioView::handle_coded_squelch(const uint32_t value) { +void AnalogAudioView::handle_coded_squelch(uint32_t value) { float diff, min_diff = value; size_t min_idx{0}; size_t c; diff --git a/firmware/application/apps/analog_audio_app.hpp b/firmware/application/apps/analog_audio_app.hpp index b76cc0c0..1fc04f62 100644 --- a/firmware/application/apps/analog_audio_app.hpp +++ b/firmware/application/apps/analog_audio_app.hpp @@ -38,7 +38,7 @@ namespace ui { class AMOptionsView : public View { public: - AMOptionsView(const Rect parent_rect, const Style* const style); + AMOptionsView(Rect parent_rect, const Style* style); private: Text label_config{ @@ -56,7 +56,7 @@ class AMOptionsView : public View { class NBFMOptionsView : public View { public: - NBFMOptionsView(const Rect parent_rect, const Style* const style); + NBFMOptionsView(Rect parent_rect, const Style* style); private: Text label_config{ @@ -84,7 +84,7 @@ class NBFMOptionsView : public View { class WFMOptionsView : public View { public: - WFMOptionsView(const Rect parent_rect, const Style* const style); + WFMOptionsView(Rect parent_rect, const Style* style); private: Text label_config{ @@ -103,7 +103,7 @@ class AnalogAudioView; class SPECOptionsView : public View { public: - SPECOptionsView(AnalogAudioView* view, const Rect parent_rect, const Style* const style); + SPECOptionsView(AnalogAudioView* view, Rect parent_rect, const Style* style); private: Text label_config{ @@ -140,10 +140,7 @@ class AnalogAudioView : public View { AnalogAudioView(NavigationView& nav); ~AnalogAudioView(); - void on_hide() override; - - void set_parent_rect(const Rect new_parent_rect) override; - + void set_parent_rect(Rect new_parent_rect) override; void focus() override; std::string title() const override { return "Audio RX"; }; @@ -170,8 +167,6 @@ class AnalogAudioView : public View { uint32_t spec_bw = 20000000; uint16_t spec_trigger = 63; - // bool exit_on_squelch { false }; - RSSI rssi{ {21 * 8, 0, 6 * 8, 4}}; @@ -221,7 +216,7 @@ class AnalogAudioView : public View { spectrum::WaterfallWidget waterfall{true}; void on_baseband_bandwidth_changed(uint32_t bandwidth_hz); - void on_modulation_changed(const ReceiverModel::Mode modulation); + void on_modulation_changed(ReceiverModel::Mode modulation); void on_show_options_frequency(); void on_show_options_rf_gain(); void on_show_options_modulation(); @@ -231,22 +226,13 @@ class AnalogAudioView : public View { void remove_options_widget(); void set_options_widget(std::unique_ptr new_widget); - void update_modulation(const ReceiverModel::Mode modulation); + void update_modulation(ReceiverModel::Mode modulation); - // void squelched(); - void handle_coded_squelch(const uint32_t value); - - /*MessageHandlerRegistration message_handler_squelch_signal { - Message::ID::RequestSignal, - [this](const Message* const p) { - (void)p; - this->squelched(); - } - };*/ + void handle_coded_squelch(uint32_t value); MessageHandlerRegistration message_handler_coded_squelch{ Message::ID::CodedSquelch, - [this](const Message* const p) { + [this](const Message* p) { const auto message = *reinterpret_cast(p); this->handle_coded_squelch(message.value); }}; diff --git a/firmware/application/apps/capture_app.cpp b/firmware/application/apps/capture_app.cpp index 55cf48ef..99bae401 100644 --- a/firmware/application/apps/capture_app.cpp +++ b/firmware/application/apps/capture_app.cpp @@ -113,6 +113,8 @@ CaptureAppView::CaptureAppView(NavigationView& nav) } CaptureAppView::~CaptureAppView() { + // Most other apps can't handle "Capture" mode, set to something standard. + receiver_model.set_modulation(ReceiverModel::Mode::WidebandFMAudio); receiver_model.disable(); baseband::shutdown(); } diff --git a/firmware/application/apps/ui_looking_glass_app.hpp b/firmware/application/apps/ui_looking_glass_app.hpp index 6ccf20c2..d692717b 100644 --- a/firmware/application/apps/ui_looking_glass_app.hpp +++ b/firmware/application/apps/ui_looking_glass_app.hpp @@ -26,6 +26,7 @@ #include "ui.hpp" #include "portapack.hpp" +#include "app_settings.hpp" #include "baseband_api.hpp" #include "radio_state.hpp" #include "receiver_model.hpp" @@ -71,6 +72,8 @@ class GlassView : public View { private: NavigationView& nav_; RxRadioState radio_state_{}; + app_settings::SettingsManager settings_{ + "rx_glass", app_settings::Mode::RX}; struct preset_entry { rf::Frequency min{}; diff --git a/firmware/application/apps/ui_mictx.cpp b/firmware/application/apps/ui_mictx.cpp index e9beb001..395affc0 100644 --- a/firmware/application/apps/ui_mictx.cpp +++ b/firmware/application/apps/ui_mictx.cpp @@ -595,12 +595,6 @@ MicTXView::MicTXView( } }; - // These shouldn't be necessary, but because - // this app uses both transmitter_model and directly - // configures the baseband, these end up being required. - transmitter_model.set_sampling_rate(sampling_rate); - transmitter_model.set_baseband_bandwidth(1750000); - set_tx(false); audio::set_rate(audio::Rate::Hz_24000); diff --git a/firmware/application/apps/ui_playlist.cpp b/firmware/application/apps/ui_playlist.cpp index 3f700603..46d5f0e9 100644 --- a/firmware/application/apps/ui_playlist.cpp +++ b/firmware/application/apps/ui_playlist.cpp @@ -142,7 +142,9 @@ bool PlaylistView::next_track() { /* Transmits the current_entry_ */ void PlaylistView::send_current_track() { + // Prepare to send a file. replay_thread_.reset(); + transmitter_model.disable(); ready_signal_ = false; if (!current_entry_) @@ -172,6 +174,10 @@ void PlaylistView::send_current_track() { transmitter_model.set_baseband_bandwidth(baseband_bandwidth); transmitter_model.enable(); + // Set baseband sample rate too for waterfall to be correct. + // TODO: Why doesn't the transmitter_model just handle this? + baseband::set_sample_rate(transmitter_model.sampling_rate()); + // Use the ReplayThread class to send the data. replay_thread_ = std::make_unique( std::move(reader), diff --git a/firmware/application/apps/ui_scanner.hpp b/firmware/application/apps/ui_scanner.hpp index b4c6003b..0dfda179 100644 --- a/firmware/application/apps/ui_scanner.hpp +++ b/firmware/application/apps/ui_scanner.hpp @@ -96,7 +96,7 @@ class ScannerView : public View { private: app_settings::SettingsManager settings_{ - "scanner", app_settings::Mode::RX}; + "rx_scanner", app_settings::Mode::RX}; NavigationView& nav_; RxRadioState radio_state_{}; diff --git a/firmware/application/radio_state.hpp b/firmware/application/radio_state.hpp index 026cf06b..eaf4ae61 100644 --- a/firmware/application/radio_state.hpp +++ b/firmware/application/radio_state.hpp @@ -30,6 +30,12 @@ /* Stashes current radio bandwidth and sampling rate. * Inits to defaults, and restores previous when destroyed. + * The idea here is that an app will eventually call enable + * on the model which will sync all of the model state into + * the radio. We tried lazily setting these using the + * set_configuration_without_update function, but there are + * still various UI components (mainly Waterfall) that need + * the radio set in order to load properly. * NB: This member must be added before SettingsManager. */ template class RadioState { @@ -41,8 +47,9 @@ class RadioState { RadioState(uint32_t new_bandwidth, uint32_t new_sampling_rate) : prev_bandwidth_{model->baseband_bandwidth()}, prev_sampling_rate_{model->sampling_rate()} { - model->set_configuration_without_update( - new_bandwidth, new_sampling_rate); + // Update the new settings in the radio. + model->set_sampling_rate(new_sampling_rate); + model->set_baseband_bandwidth(new_bandwidth); } ~RadioState() { diff --git a/firmware/application/ui/ui_spectrum.cpp b/firmware/application/ui/ui_spectrum.cpp index 305a89de..6634de78 100644 --- a/firmware/application/ui/ui_spectrum.cpp +++ b/firmware/application/ui/ui_spectrum.cpp @@ -311,10 +311,12 @@ WaterfallWidget::WaterfallWidget(const bool cursor) { } void WaterfallWidget::on_show() { + // TODO: Assert that baseband is not shutdown. baseband::spectrum_streaming_start(); } void WaterfallWidget::on_hide() { + // TODO: Assert that baseband is not shutdown. baseband::spectrum_streaming_stop(); } diff --git a/firmware/application/ui/ui_spectrum.hpp b/firmware/application/ui/ui_spectrum.hpp index 83b9539c..2ea8a066 100644 --- a/firmware/application/ui/ui_spectrum.hpp +++ b/firmware/application/ui/ui_spectrum.hpp @@ -104,6 +104,10 @@ class FrequencyScale : public Widget { void draw_filter_ranges(Painter& painter, const Rect r); }; +/* NB: These visualizations rely on having a baseband image running. + * If the baseband is shutdown or otherwise not running when interacting + * with these, they will almost certainly hang the device. */ + class WaterfallView : public Widget { public: void on_show() override;