Skip to content

Commit

Permalink
Workaround for Capture startup hang (portapack-mayhem#1285)
Browse files Browse the repository at this point in the history
* Attempt to fix Capture startup hang

* Pump baseband_queue on M4 startup

* Synchronization experiment

* Moved SpectrumCapture member, better hang detection for M0

* Prevent execute from working on members until class has been initialized.

* Formatting

* Remove workaround.

* Rebase on next
  • Loading branch information
kallanreed authored Jul 22, 2023
1 parent 3b5890d commit 47e95c0
Show file tree
Hide file tree
Showing 14 changed files with 100 additions and 65 deletions.
27 changes: 6 additions & 21 deletions firmware/application/apps/capture_app.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ namespace ui {

CaptureAppView::CaptureAppView(NavigationView& nav)
: nav_{nav} {
freqman_set_bandwidth_option(SPEC_MODULATION, option_bandwidth);
baseband::run_image(portapack::spi_flash::image_tag_capture);

add_children({
Expand All @@ -49,44 +48,35 @@ CaptureAppView::CaptureAppView(NavigationView& nav)
&waterfall,
});

/* THE ONE LINE BELOW IS A TEMPORARY KLUDGE WORKAROUND FOR A MYSTERY M4 BASEBAND HANG ISSUE WHEN THE CAPTURE
APP IS THE FIRST APP STARTED AFTER POWER-UP, OR THE CAPTURE APP IS RUN AFTER RUNNING REPLAY WITH A HIGH
SAMPLE RATE. IT SHOULD NOT BE NECESSARY SINCE sampling_rate IS ALREADY INITIALIZED BY RADIOSTATE BUT
APPARENTLY IS ADDING JUST THE RIGHT AMOUNT OF DELAY. ISSUE DOES NOT AFFECT ALL PORTAPACK UNITS.
INVESTIGATION IS ONGOING, SEE ISSUE #1283. */
receiver_model.set_sampling_rate(3072000);
/* END MYSTERY HANG WORKAROUND. */

field_frequency_step.set_by_value(receiver_model.frequency_step());
field_frequency_step.on_change = [this](size_t, OptionsField::value_t v) {
receiver_model.set_frequency_step(v);
this->field_frequency.set_step(v);
};

option_format.set_selected_index(0); // Default to C16
option_format.on_change = [this](size_t, uint32_t file_type) {
record_view.set_file_type((RecordView::FileType)file_type);
};

freqman_set_bandwidth_option(SPEC_MODULATION, option_bandwidth);
option_bandwidth.on_change = [this](size_t, uint32_t base_rate) {
sampling_rate = 8 * base_rate; // Decimation by 8 done on baseband side
/* base_rate is used for FFT calculation and display LCD, and also in recording writing SD Card rate. */
/* base_rate is used for FFT calculation and display LCD, and also in recording writing SD Card rate. */
/* ex. sampling_rate values, 4Mhz, when recording 500 kHz (BW) and fs 8 Mhz, when selected 1 Mhz BW ... */
/* ex. recording 500kHz BW to .C16 file, base_rate clock 500kHz x2(I,Q) x 2 bytes (int signed) =2MB/sec rate SD Card */
waterfall.on_hide();
auto sampling_rate = 8 * base_rate; // Decimation by 8 done on baseband side.

/* Set up proper anti aliasing BPF bandwith in MAX2837 before ADC sampling according to the new added BW Options. */
auto anti_alias_baseband_bandwidth_filter = filter_bandwidth_for_sampling_rate(sampling_rate);

waterfall.stop();
record_view.set_sampling_rate(sampling_rate);
receiver_model.set_sampling_rate(sampling_rate);
receiver_model.set_baseband_bandwidth(anti_alias_baseband_bandwidth_filter);

waterfall.on_show();
waterfall.start();
};

option_bandwidth.set_selected_index(7); // Preselected default option 500kHz.
receiver_model.enable();
option_bandwidth.set_selected_index(7); // Preselected default option 500kHz.

record_view.on_error = [&nav](std::string message) {
nav.display_modal("Error", message);
Expand All @@ -98,11 +88,6 @@ CaptureAppView::~CaptureAppView() {
baseband::shutdown();
}

void CaptureAppView::on_hide() {
waterfall.on_hide();
View::on_hide();
}

void CaptureAppView::set_parent_rect(const Rect new_parent_rect) {
View::set_parent_rect(new_parent_rect);

Expand Down
3 changes: 0 additions & 3 deletions firmware/application/apps/capture_app.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ class CaptureAppView : public View {
CaptureAppView(NavigationView& nav);
~CaptureAppView();

void on_hide() override;
void focus() override;
void set_parent_rect(const Rect new_parent_rect) override;

Expand All @@ -54,8 +53,6 @@ class CaptureAppView : public View {
"rx_capture", app_settings::Mode::RX,
app_settings::Options::UseGlobalTargetFrequency};

uint32_t sampling_rate = 0;

Labels labels{
{{0 * 8, 1 * 16}, "Rate:", Color::light_grey()},
{{11 * 8, 1 * 16}, "Format:", Color::light_grey()},
Expand Down
39 changes: 35 additions & 4 deletions firmware/application/baseband_api.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,16 @@

#include "core_control.hpp"

/* Set true to enable additional checks to ensure
* M4 and M0 are synchronized before passing messages. */
static constexpr bool enforce_core_sync = true;

/* Set true to enable check for baseband messages getting stuck.
* This implies the baseband thread is not dequeuing and has probably stalled.
* NB: This check adds a small amout of overhead to the message sending code
* and may impact application perf if it is sending a lot of messages. */
static constexpr bool check_for_message_hang = true;

using namespace portapack;

namespace baseband {
Expand All @@ -40,8 +50,18 @@ static void send_message(const Message* const message) {
// another message is present before setting new message.
shared_memory.baseband_message = message;
creg::m0apptxevent::assert_event();
while (shared_memory.baseband_message)
;

if constexpr (check_for_message_hang) {
auto count = UINT32_MAX;
while (shared_memory.baseband_message && --count)
/* spin */;

if (count == 0)
chDbgPanic("Baseband Send Fail");
} else {
while (shared_memory.baseband_message)
/* spin */;
}
}

void AMConfig::apply() const {
Expand Down Expand Up @@ -276,17 +296,28 @@ void set_spectrum_painter_config(const uint16_t width, const uint16_t height, bo

static bool baseband_image_running = false;

void run_image(const portapack::spi_flash::image_tag_t image_tag) {
void run_image(const spi_flash::image_tag_t image_tag) {
if (baseband_image_running) {
chDbgPanic("BBRunning");
}

creg::m4txevent::clear();
shared_memory.clear_baseband_ready();

m4_init(image_tag, portapack::memory::map::m4_code, false);
m4_init(image_tag, memory::map::m4_code, false);
baseband_image_running = true;

creg::m4txevent::enable();

if constexpr (enforce_core_sync) {
// Wait up to 3 seconds for baseband to start handling events.
auto count = 3'000u;
while (!shared_memory.baseband_ready && --count)
chThdSleepMilliseconds(1);

if (count == 0)
chDbgPanic("Baseband Sync Fail");
}
}

void shutdown() {
Expand Down
20 changes: 9 additions & 11 deletions firmware/application/core_control.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,20 @@
* Boston, MA 02110-1301, USA.
*/

#include "baseband_api.hpp"
#include "core_control.hpp"

#include "hal.h"

#include "lpc43xx_cpp.hpp"
using namespace lpc43xx;

#include "message.hpp"
#include "baseband_api.hpp"
#include "lz4.h"
#include "message.hpp"

#include <cstring>

void m4_init(const portapack::spi_flash::image_tag_t image_tag, const portapack::memory::region_t to, const bool full_reset) {
const portapack::spi_flash::chunk_t* chunk = reinterpret_cast<const portapack::spi_flash::chunk_t*>(portapack::spi_flash::images.base());
using namespace lpc43xx;
using namespace portapack;

void m4_init(const spi_flash::image_tag_t image_tag, const memory::region_t to, const bool full_reset) {
const spi_flash::chunk_t* chunk = reinterpret_cast<const spi_flash::chunk_t*>(spi_flash::images.base());
while (chunk->tag) {
if (chunk->tag == image_tag) {
const void* src = &chunk->data[0];
Expand All @@ -49,9 +48,8 @@ void m4_init(const portapack::spi_flash::image_tag_t image_tag, const portapack:
LPC_CREG->M4MEMMAP = to.base();

/* Reset M4 core and optionally all peripherals */
LPC_RGU->RESET_CTRL[0] = (full_reset) ? (1 << 1) // PERIPH_RST
: (1 << 13) // M4_RST
;
LPC_RGU->RESET_CTRL[0] = (full_reset) ? (1 << 1) // PERIPH_RST
: (1 << 13); // M4_RST

return;
}
Expand Down
1 change: 1 addition & 0 deletions firmware/application/radio.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,7 @@ void set_baseband_filter_bandwidth(const uint32_t bandwidth_minimum) {

void set_baseband_rate(const uint32_t rate) {
portapack::clock_manager.set_sampling_frequency(rate);
// TODO: actually set baseband too?
}

void set_antenna_bias(const bool on) {
Expand Down
20 changes: 16 additions & 4 deletions firmware/application/ui/ui_spectrum.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -308,13 +308,25 @@ WaterfallView::WaterfallView(const bool cursor) {
}

void WaterfallView::on_show() {
// TODO: Assert that baseband is not shutdown.
baseband::spectrum_streaming_start();
start();
}

void WaterfallView::on_hide() {
// TODO: Assert that baseband is not shutdown.
baseband::spectrum_streaming_stop();
stop();
}

void WaterfallView::start() {
if (!running_) {
baseband::spectrum_streaming_start();
running_ = true;
}
}

void WaterfallView::stop() {
if (running_) {
baseband::spectrum_streaming_stop();
running_ = false;
}
}

void WaterfallView::show_audio_spectrum_view(const bool show) {
Expand Down
6 changes: 5 additions & 1 deletion firmware/application/ui/ui_spectrum.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -131,11 +131,14 @@ class WaterfallView : public View {
WaterfallView& operator=(const WaterfallView&) = delete;
WaterfallView& operator=(WaterfallView&&) = delete;

// TODO: remove these, use start/stop directly instead.
void on_show() override;
void on_hide() override;

void set_parent_rect(const Rect new_parent_rect) override;
void start();
void stop();

void set_parent_rect(const Rect new_parent_rect) override;
void show_audio_spectrum_view(const bool show);

private:
Expand All @@ -147,6 +150,7 @@ class WaterfallView : public View {

WaterfallWidget waterfall_widget{};
FrequencyScale frequency_scale{};
bool running_{false};

ChannelSpectrumFIFO* channel_fifo{nullptr};
AudioSpectrum* audio_spectrum_data{nullptr};
Expand Down
9 changes: 2 additions & 7 deletions firmware/application/ui_navigation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -497,13 +497,8 @@ ReceiversMenuView::ReceiversMenuView(NavigationView& nav) {
add_items({{"..", Color::light_grey(), &bitmap_icon_previous, [&nav]() { nav.pop(); }}});
}
add_items({
{
"ADS-B",
Color::green(),
&bitmap_icon_adsb,
[&nav]() { nav.push<ADSBRxView>(); },
},
//{ "ACARS", Color::yellow(), &bitmap_icon_adsb, [&nav](){ nav.push<ACARSAppView>(); }, },
{"ADS-B", Color::green(), &bitmap_icon_adsb, [&nav]() { nav.push<ADSBRxView>(); }},
//{ "ACARS", Color::yellow(), &bitmap_icon_adsb, [&nav](){ nav.push<ACARSAppView>(); }},
{"AIS Boats", Color::green(), &bitmap_icon_ais, [&nav]() { nav.push<AISAppView>(); }},
{"AFSK", Color::yellow(), &bitmap_icon_modem, [&nav]() { nav.push<AFSKRxView>(); }},
{"BTLE", Color::yellow(), &bitmap_icon_btle, [&nav]() { nav.push<BTLERxView>(); }},
Expand Down
19 changes: 10 additions & 9 deletions firmware/baseband/event_m4.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,21 +19,18 @@
* Boston, MA 02110-1301, USA.
*/

#include "event_m4.hpp"
#include "debug.hpp"

#include "portapack_shared_memory.hpp"

#include "message_queue.hpp"

#include "ch.h"

#include "debug.hpp"
#include "event_m4.hpp"
#include "lpc43xx_cpp.hpp"
using namespace lpc43xx;
#include "message_queue.hpp"
#include "portapack_shared_memory.hpp"

#include <cstdint>
#include <array>

using namespace lpc43xx;

extern "C" {

CH_IRQ_HANDLER(MAPP_IRQHandler) {
Expand Down Expand Up @@ -61,6 +58,10 @@ void EventDispatcher::run() {

lpc43xx::creg::m0apptxevent::enable();

// Indicate to the M0 thread that
// M4 is ready to receive message events.
shared_memory.set_baseband_ready();

while (is_running) {
const auto events = wait();
dispatch(events);
Expand Down
6 changes: 4 additions & 2 deletions firmware/baseband/proc_capture.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,19 +23,21 @@
#include "proc_capture.hpp"

#include "dsp_fir_taps.hpp"

#include "event_m4.hpp"

#include "utility.hpp"

CaptureProcessor::CaptureProcessor() {
decim_0.configure(taps_200k_decim_0.taps, 33554432);
decim_1.configure(taps_200k_decim_1.taps, 131072);

channel_spectrum.set_decimation_factor(1);
ready = true;
}

void CaptureProcessor::execute(const buffer_c8_t& buffer) {
if (!ready)
return;

/* 2.4576MHz, 2048 samples */
const auto decim_0_out = decim_0.execute(buffer, dst_buffer);
const auto decim_1_out = decim_1.execute(decim_0_out, dst_buffer);
Expand Down
6 changes: 5 additions & 1 deletion firmware/baseband/proc_capture.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,13 @@ class CaptureProcessor : public BasebandProcessor {

private:
// TODO: Repeated value needs to be transmitted from application side.
size_t baseband_fs = 0;
size_t baseband_fs = 3072000;
static constexpr auto spectrum_rate_hz = 50.0f;

// HACK: BasebandThread starts immediately and starts sending data to members
// before they are initialized. This is a workaround to prevent uninit data problems.
bool ready = false;

BasebandThread baseband_thread{baseband_fs, this, NORMALPRIO + 20, baseband::Direction::Receive};
RSSIThread rssi_thread{NORMALPRIO + 10};

Expand Down
2 changes: 1 addition & 1 deletion firmware/baseband/proc_gps_sim.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class ReplayProcessor : public BasebandProcessor {
void on_message(const Message* const message) override;

private:
size_t baseband_fs = 0;
size_t baseband_fs = 3072000;
static constexpr auto spectrum_rate_hz = 50.0f;

BasebandThread baseband_thread{baseband_fs, this, NORMALPRIO + 20, baseband::Direction::Transmit};
Expand Down
2 changes: 1 addition & 1 deletion firmware/baseband/proc_replay.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class ReplayProcessor : public BasebandProcessor {
void on_message(const Message* const message) override;

private:
size_t baseband_fs = 0;
size_t baseband_fs = 3072000;
static constexpr auto spectrum_rate_hz = 50.0f;

BasebandThread baseband_thread{baseband_fs, this, NORMALPRIO + 20, baseband::Direction::Transmit};
Expand Down
5 changes: 5 additions & 0 deletions firmware/common/portapack_shared_memory.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,11 @@ struct SharedMemory {
uint8_t data[512];
} bb_data{{{{0, 0}}, 0, {0}}};

// Set by the M4 to indicate that the baseband app is ready.
bool volatile baseband_ready{false};
void clear_baseband_ready() { baseband_ready = false; }
void set_baseband_ready() { baseband_ready = true; }

uint8_t volatile request_m4_performance_counter{0};
uint8_t volatile m4_cpu_usage{0};
uint16_t volatile m4_stack_usage{0};
Expand Down

0 comments on commit 47e95c0

Please sign in to comment.