Fix baseband thread init order bug for all procs. (#1293)

firmware/baseband/baseband_thread.cpp

@@ -46,34 +46,42 @@ Thread* BasebandThread::thread = nullptr;
 BasebandThread::BasebandThread(
     uint32_t sampling_rate,
     BasebandProcessor* const baseband_processor,
-    const tprio_t priority,
-    baseband::Direction direction)
-    : baseband_processor{baseband_processor},
-      _direction{direction},
-      sampling_rate{sampling_rate} {
-    thread = chThdCreateStatic(baseband_thread_wa, sizeof(baseband_thread_wa),
-                               priority, ThreadBase::fn,
-                               this);
+    baseband::Direction direction,
+    bool auto_start,
+    tprio_t priority)
+    : baseband_processor_{baseband_processor},
+      direction_{direction},
+      sampling_rate_{sampling_rate},
+      priority_{priority} {
+    if (auto_start) start();
 }
 
 BasebandThread::~BasebandThread() {
-    chThdTerminate(thread);
-    chThdWait(thread);
-    thread = nullptr;
+    if (thread) {
+        chThdTerminate(thread);
+        chThdWait(thread);
+        thread = nullptr;
+    }
+}
+
+void BasebandThread::start() {
+    if (!thread) {
+        thread = chThdCreateStatic(
+            baseband_thread_wa, sizeof(baseband_thread_wa),
+            priority_, ThreadBase::fn, this);
+    }
 }
 
 void BasebandThread::set_sampling_rate(uint32_t new_sampling_rate) {
-    sampling_rate = new_sampling_rate;
+    sampling_rate_ = new_sampling_rate;
 }
 
 void BasebandThread::run() {
     baseband_sgpio.init();
     baseband::dma::init();
 
     const auto baseband_buffer = std::make_unique<std::array<baseband::sample_t, 8192>>();
-    baseband::dma::configure(
-        baseband_buffer->data(),
-        direction());
+    baseband::dma::configure(baseband_buffer->data(), direction());
     // baseband::dma::allocate(4, 2048);
 
     baseband_sgpio.configure(direction());
@@ -85,10 +93,10 @@ void BasebandThread::run() {
         const auto buffer_tmp = baseband::dma::wait_for_buffer();
         if (buffer_tmp) {
             buffer_c8_t buffer{
-                buffer_tmp.p, buffer_tmp.count, sampling_rate};
+                buffer_tmp.p, buffer_tmp.count, sampling_rate_};
 
-            if (baseband_processor) {
-                baseband_processor->execute(buffer);
+            if (baseband_processor_) {
+                baseband_processor_->execute(buffer);
             }
         }
     }

firmware/baseband/baseband_thread.hpp

@@ -28,34 +28,44 @@
 
 #include <ch.h>
 
+/* NB: Because ThreadBase threads start when then are initialized (by default),
+ * they should be the last members in a Processor class to ensure the rest of the
+ * members are fully initialized before data handling starts. If the Procressor
+ * needs to do additional initialization (in its ctor), set 'auto_start' to false
+ * and manually call 'start()' on the thread. */
+
 class BasebandThread : public ThreadBase {
    public:
     BasebandThread(
         uint32_t sampling_rate,
         BasebandProcessor* const baseband_processor,
-        const tprio_t priority,
-        const baseband::Direction direction = baseband::Direction::Receive);
+        baseband::Direction direction,
+        bool auto_start = true,
+        tprio_t priority = (NORMALPRIO + 20));
     ~BasebandThread();
 
     BasebandThread(const BasebandThread&) = delete;
     BasebandThread(BasebandThread&&) = delete;
     BasebandThread& operator=(const BasebandThread&) = delete;
     BasebandThread& operator=(BasebandThread&&) = delete;
 
+    void start() override;
+
     // This getter should die, it's just here to leak information to code that
     // isn't in the right place to begin with.
     baseband::Direction direction() const {
-        return _direction;
+        return direction_;
     }
 
     void set_sampling_rate(uint32_t new_sampling_rate);
 
    private:
     static Thread* thread;
 
-    BasebandProcessor* baseband_processor{nullptr};
-    baseband::Direction _direction{baseband::Direction::Receive};
-    uint32_t sampling_rate{0};
+    BasebandProcessor* baseband_processor_;
+    baseband::Direction direction_;
+    uint32_t sampling_rate_;
+    const tprio_t priority_;
 
     void run() override;
 };

firmware/baseband/proc_acars.cpp

@@ -32,6 +32,7 @@ ACARSProcessor::ACARSProcessor() {
     decim_0.configure(taps_11k0_decim_0.taps, 33554432);
     decim_1.configure(taps_11k0_decim_1.taps, 131072);
     packet.clear();
+    baseband_thread.start();
 }
 
 void ACARSProcessor::execute(const buffer_c8_t& buffer) {

firmware/baseband/proc_acars.hpp

@@ -110,9 +110,6 @@ class ACARSProcessor : public BasebandProcessor {
    private:
     static constexpr size_t baseband_fs = 2457600;
 
-    BasebandThread baseband_thread{baseband_fs, this, NORMALPRIO + 20, baseband::Direction::Receive};
-    RSSIThread rssi_thread{NORMALPRIO + 10};
-
     std::array<complex16_t, 512> dst{};
     const buffer_c16_t dst_buffer{
         dst.data(),
@@ -138,6 +135,11 @@ class ACARSProcessor : public BasebandProcessor {
         };*/
     baseband::Packet packet{};
 
+    /* NB: Threads should be the last members in the class definition. */
+    BasebandThread baseband_thread{
+        baseband_fs, this, baseband::Direction::Receive, /*auto_start*/ false};
+    RSSIThread rssi_thread{};
+
     void consume_symbol(const float symbol);
     void payload_handler(const baseband::Packet& packet);
 };

firmware/baseband/proc_adsbrx.hpp

@@ -36,15 +36,11 @@ using namespace adsb;
 class ADSBRXProcessor : public BasebandProcessor {
    public:
     void execute(const buffer_c8_t& buffer) override;
-
     void on_message(const Message* const message) override;
 
    private:
     static constexpr size_t baseband_fs = 2000000;
 
-    BasebandThread baseband_thread{baseband_fs, this, NORMALPRIO + 20, baseband::Direction::Receive};
-    RSSIThread rssi_thread{NORMALPRIO + 10};
-
     ADSBFrame frame{};
     bool configured{false};
     uint32_t prev_mag{0};
@@ -58,6 +54,10 @@ class ADSBRXProcessor : public BasebandProcessor {
     uint32_t sample{0};
     int32_t re{}, im{};
     int32_t amp{0};
+
+    /* NB: Threads should be the last members in the class definition. */
+    BasebandThread baseband_thread{baseband_fs, this, baseband::Direction::Receive};
+    RSSIThread rssi_thread{};
 };
 
 #endif

firmware/baseband/proc_adsbtx.hpp

@@ -29,14 +29,11 @@
 class ADSBTXProcessor : public BasebandProcessor {
    public:
     void execute(const buffer_c8_t& buffer) override;
-
     void on_message(const Message* const p) override;
 
    private:
     bool configured = false;
 
-    BasebandThread baseband_thread{4000000, this, NORMALPRIO + 20, baseband::Direction::Transmit};
-
     const complex8_t am_lut[4] = {
         {127, 0},
         {0, 127},
@@ -48,6 +45,9 @@ class ADSBTXProcessor : public BasebandProcessor {
     uint32_t phase{0};
 
     TXProgressMessage txprogress_message{};
+
+    /* NB: Threads should be the last members in the class definition. */
+    BasebandThread baseband_thread{4000000, this, baseband::Direction::Transmit};
 };
 
 #endif

firmware/baseband/proc_afsk.hpp

@@ -32,14 +32,11 @@
 class AFSKProcessor : public BasebandProcessor {
    public:
     void execute(const buffer_c8_t& buffer) override;
-
     void on_message(const Message* const msg) override;
 
    private:
     bool configured = false;
 
-    BasebandThread baseband_thread{AFSK_SAMPLERATE, this, NORMALPRIO + 20, baseband::Direction::Transmit};
-
     uint32_t afsk_samples_per_bit{0};
     uint32_t afsk_phase_inc_mark{0};
     uint32_t afsk_phase_inc_space{0};
@@ -59,6 +56,9 @@ class AFSKProcessor : public BasebandProcessor {
     int8_t re{0}, im{0};
 
     TXProgressMessage txprogress_message{};
+
+    /* NB: Threads should be the last members in the class definition. */
+    BasebandThread baseband_thread{AFSK_SAMPLERATE, this, baseband::Direction::Transmit};
 };
 
 #endif

firmware/baseband/proc_afskrx.hpp

@@ -38,7 +38,6 @@
 class AFSKRxProcessor : public BasebandProcessor {
    public:
     void execute(const buffer_c8_t& buffer) override;
-
     void on_message(const Message* const message) override;
 
    private:
@@ -53,9 +52,6 @@ class AFSKRxProcessor : public BasebandProcessor {
         RECEIVE
     };
 
-    BasebandThread baseband_thread{baseband_fs, this, NORMALPRIO + 20, baseband::Direction::Receive};
-    RSSIThread rssi_thread{NORMALPRIO + 10};
-
     std::array<complex16_t, 512> dst{};
     const buffer_c16_t dst_buffer{
         dst.data(),
@@ -93,9 +89,13 @@ class AFSKRxProcessor : public BasebandProcessor {
     bool trigger_word{};
     bool triggered{};
 
-    void configure(const AFSKRxConfigureMessage& message);
-
     AFSKDataMessage data_message{false, 0};
+
+    /* NB: Threads should be the last members in the class definition. */
+    BasebandThread baseband_thread{baseband_fs, this, baseband::Direction::Receive};
+    RSSIThread rssi_thread{};
+
+    void configure(const AFSKRxConfigureMessage& message);
 };
 
 #endif /*__PROC_TPMS_H__*/

firmware/baseband/proc_ais.cpp

@@ -30,6 +30,7 @@
 AISProcessor::AISProcessor() {
     decim_0.configure(taps_11k0_decim_0.taps, 33554432);
     decim_1.configure(taps_11k0_decim_1.taps, 131072);
+    baseband_thread.start();
 }
 
 void AISProcessor::execute(const buffer_c8_t& buffer) {

firmware/baseband/proc_ais.hpp

@@ -51,9 +51,6 @@ class AISProcessor : public BasebandProcessor {
    private:
     static constexpr size_t baseband_fs = 2457600;
 
-    BasebandThread baseband_thread{baseband_fs, this, NORMALPRIO + 20, baseband::Direction::Receive};
-    RSSIThread rssi_thread{NORMALPRIO + 10};
-
     std::array<complex16_t, 512> dst{};
     const buffer_c16_t dst_buffer{
         dst.data(),
@@ -77,6 +74,11 @@ class AISProcessor : public BasebandProcessor {
             this->payload_handler(packet);
         }};
 
+    /* NB: Threads should be the last members in the class definition. */
+    BasebandThread baseband_thread{
+        baseband_fs, this, baseband::Direction::Receive, /*auto_start*/ false};
+    RSSIThread rssi_thread{};
+
     void consume_symbol(const float symbol);
     void payload_handler(const baseband::Packet& packet);
 };

firmware/baseband/proc_am_audio.hpp

@@ -38,17 +38,13 @@
 class NarrowbandAMAudio : public BasebandProcessor {
    public:
     void execute(const buffer_c8_t& buffer) override;
-
     void on_message(const Message* const message) override;
 
    private:
     static constexpr size_t baseband_fs = 3072000;
     static constexpr size_t decim_2_decimation_factor = 4;
     static constexpr size_t channel_filter_decimation_factor = 1;
 
-    BasebandThread baseband_thread{baseband_fs, this, NORMALPRIO + 20, baseband::Direction::Receive};
-    RSSIThread rssi_thread{NORMALPRIO + 10};
-
     std::array<complex16_t, 512> dst{};
     const buffer_c16_t dst_buffer{
         dst.data(),
@@ -65,6 +61,7 @@ class NarrowbandAMAudio : public BasebandProcessor {
     int32_t channel_filter_low_f = 0;
     int32_t channel_filter_high_f = 0;
     int32_t channel_filter_transition = 0;
+    bool configured{false};
 
     bool modulation_ssb = false;
     dsp::demodulate::AM demod_am{};
@@ -74,7 +71,10 @@ class NarrowbandAMAudio : public BasebandProcessor {
 
     SpectrumCollector channel_spectrum{};
 
-    bool configured{false};
+    /* NB: Threads should be the last members in the class definition. */
+    BasebandThread baseband_thread{baseband_fs, this, baseband::Direction::Receive};
+    RSSIThread rssi_thread{};
+
     void configure(const AMConfigureMessage& message);
     void capture_config(const CaptureConfigMessage& message);
 

firmware/baseband/proc_am_tv.hpp

@@ -38,15 +38,11 @@
 class WidebandFMAudio : public BasebandProcessor {
    public:
     void execute(const buffer_c8_t& buffer) override;
-
     void on_message(const Message* const message) override;
 
    private:
     static constexpr size_t baseband_fs = 2000000;
 
-    BasebandThread baseband_thread{baseband_fs, this, NORMALPRIO + 20, baseband::Direction::Receive};
-    RSSIThread rssi_thread{NORMALPRIO + 10};
-
     std::array<complex16_t, 512> dst{};
     const buffer_c16_t dst_buffer{
         dst.data(),
@@ -55,8 +51,12 @@ class WidebandFMAudio : public BasebandProcessor {
     AudioSpectrum audio_spectrum{};
     TvCollector channel_spectrum{};
     std::array<complex16_t, 256> spectrum{};
-
     bool configured{false};
+
+    /* NB: Threads should be the last members in the class definition. */
+    BasebandThread baseband_thread{baseband_fs, this, baseband::Direction::Receive};
+    RSSIThread rssi_thread{};
+
     void configure(const WFMConfigureMessage& message);
 };
 

firmware/baseband/proc_aprsrx.hpp

@@ -75,7 +75,6 @@ static uint16_t crc_ccitt_tab[256] = {
 class APRSRxProcessor : public BasebandProcessor {
    public:
     void execute(const buffer_c8_t& buffer) override;
-
     void on_message(const Message* const message) override;
 
    private:
@@ -90,9 +89,6 @@ class APRSRxProcessor : public BasebandProcessor {
         IN_FRAME
     };
 
-    BasebandThread baseband_thread{baseband_fs, this, NORMALPRIO + 20, baseband::Direction::Receive};
-    RSSIThread rssi_thread{NORMALPRIO + 10};
-
     std::array<complex16_t, 512> dst{};
     const buffer_c16_t dst_buffer{
         dst.data(),
@@ -135,6 +131,10 @@ class APRSRxProcessor : public BasebandProcessor {
 
     aprs::APRSPacket aprs_packet{};
 
+    /* NB: Threads should be the last members in the class definition. */
+    BasebandThread baseband_thread{baseband_fs, this, baseband::Direction::Receive};
+    RSSIThread rssi_thread{};
+
     void configure(const APRSRxConfigureMessage& message);
     void capture_config(const CaptureConfigMessage& message);
     void parse_packet();