Cleanup dead code in the ADSB RX proc (#1572)

* Cleanup dead code in the ADSB RX proc

* Fix comments

* Formatting

firmware/baseband/proc_adsbrx.cpp

@@ -20,6 +20,8 @@
  * Boston, MA 02110-1301, USA.
  */
 
+// https://www.icao.int/SAM/Documents/2015-SEMAUTOM/Ses4%20Presentation%20CUBA_ADSB.pdf
+
 #include "proc_adsbrx.hpp"
 #include "portapack_shared_memory.hpp"
 #include "sine_table_int8.hpp"
@@ -31,29 +33,27 @@
 using namespace adsb;
 
 void ADSBRXProcessor::execute(const buffer_c8_t& buffer) {
-    int8_t re, im;
-    uint32_t mag;
-    uint32_t c;
-    uint8_t bit, byte{};
-
     // This is called at 2M/2048 = 977Hz
-    // One pulse = 500ns = 2 samples
+    // Each sample is 500ns.
-    // One bit = 2 pulses = 1us = 4 samples
+    // One bit is 2 samples == 1us.
+    // Bit value is the transition between samples.
+    // i.e. lo->hi == 0, hi->lo == 1
 
     if (!configured) return;
 
+    uint8_t bit = 0;
+    uint8_t byte = 0;
+
     for (size_t i = 0; i < buffer.count; i++) {
-        // Compute sample's magnitude
+        // Compute sample's magnitude.
-        re = (int32_t)buffer.p[i].real();
+        int8_t re = buffer.p[i].real();
-        im = (int32_t)buffer.p[i].imag();
+        int8_t im = buffer.p[i].imag();
-        mag = ((uint32_t)(re * re) + (uint32_t)(im * im));
+        uint16_t mag = (re * re) + (im * im);
 
         if (decoding) {
-            // Decode
+            // 1 bit == 2 samples, transition defines bit value.
-
+            if ((sample_count & 1) == 1) {
-            // 1 bit lasts 2 samples
+                if (bit_count >= msg_len) {
-            if (sample_count & 1) {
-                if (bit_count >= msgLen) {
                     const ADSBFrameMessage message(frame, amp);
                     shared_memory.application_queue.push(message);
                     decoding = false;
@@ -65,40 +65,42 @@ void ADSBRXProcessor::execute(const buffer_c8_t& buffer) {
                 byte = bit | (byte << 1);
                 bit_count++;
 
-                // Perform checks at the end of the first byte
+                // Every 8th bit...
-                if (!(bit_count & 7)) {
+                if ((bit_count & 0x7) == 0) {
-                    // Store the byte
+                    // Store the byte.
                     frame.push_byte(byte);
 
-                    // Check at the end of the first byte of the message
+                    // Perform additional check on the first byte.
-                    uint8_t df = (byte >> 3);
+                    if (bit_count == 8) {
-                    if ((bit_count == 8) && !(df & 0x10)) {
+                        // Abandon all frames that aren't DF17 or DF18 extended squitters.
-                        msgLen = 56;  // DFs 16 or greater are long 112. DFs 15 or less are short 56.
+                        uint8_t df = (byte >> 3);
+                        if (df != 17 && df != 18) {
+                            decoding = false;
+                            bit = (prev_mag > mag) ? 1 : 0;
+                            frame.clear();
+                        }
                     }
+                }
+            }
 
-                    // Abondon all frames that arent DF17 or DF18 extended squitters
-                    if ((bit_count == 8) && !((df == 17) || (df == 18))) {
-                        decoding = false;
-                        bit = (prev_mag > mag) ? 1 : 0;
-                        frame.clear();
-                    }
-                }  // last bit of a byte
-            }      // Second sample of each bit
             sample_count++;
         }
 
-        // Continue looking for preamble even if in a packet
+        // Continue looking for preamble, even if in a packet.
-        // switch if new preamble is higher magnitude
+        // Switch if new preamble is higher magnitude.
 
-        // Shift the preamble
+        // Shift the preamble.
-        for (c = 0; c < (ADSB_PREAMBLE_LENGTH); c++) {
+        for (uint8_t c = 0; c < ADSB_PREAMBLE_LENGTH; c++) {
             shifter[c] = shifter[c + 1];
         }
         shifter[ADSB_PREAMBLE_LENGTH] = mag;
 
-        // First check of relations between the first 10 samples
+        // First check of relations between the first 12 samples
-        // representing a valid preamble. We don't even investigate further
+        // representing a valid preamble. We don't even investigate
-        // if this simple test is not passed
+        // further if this simple test is not passed.
+        // Preamble is 8us - or 16 samples.
+        //    0123456789ABCDEF
+        //    _-_-____-_-_____
         if (shifter[0] < shifter[1] &&
             shifter[1] > shifter[2] &&
             shifter[2] < shifter[3] &&
@@ -113,32 +115,30 @@ void ADSBRXProcessor::execute(const buffer_c8_t& buffer) {
             // The samples between the two spikes must be < than the average
             // of the high spikes level. We don't test bits too near to
             // the high levels as signals can be out of phase so part of the
-            // energy can be in the near samples
+            // energy can be in the near samples.
-            int32_t thisAmp = (shifter[1] + shifter[3] + shifter[8] + shifter[10]);
+            int32_t this_amp = (shifter[1] + shifter[3] + shifter[8] + shifter[10]);
-            uint32_t high = thisAmp / 9;
+            uint32_t high = this_amp / 9;  // TBD: Why 9?
-            if (
+            if (shifter[5] < high &&
-                shifter[5] < high &&
                 shifter[6] < high &&
                 // Similarly samples in the range 11-13 must be low, as it is the
                 // space between the preamble and real data. Again we don't test
-                // bits too near to high levels, see above
+                // bits too near to high levels, see above.
                 shifter[12] < high &&
                 shifter[13] < high &&
                 shifter[14] < high) {
-                if ((decoding == false) ||                    // New preamble
+                if ((decoding == false) ||                     // New preamble
-                    ((decoding == true) && (thisAmp > amp)))  // Higher power than existing packet
+                    ((decoding == true) && (this_amp > amp)))  // Higher power than existing packet
                 {
                     decoding = true;
-                    msgLen = 112;
+                    amp = this_amp;
-                    amp = thisAmp;
                     sample_count = 0;
                     bit_count = 0;
                     frame.clear();
                 }
-            }  // 4 & 5 low and 11-14 low
+            }
-        }      // Check for preamble pattern
+        }
 
-        // Store mag for next time
+        // Store mag for next time.
         prev_mag = mag;
     }
 }

firmware/baseband/proc_adsbrx.hpp

@@ -39,21 +39,19 @@ class ADSBRXProcessor : public BasebandProcessor {
     void on_message(const Message* const message) override;
 
    private:
-    static constexpr size_t baseband_fs = 2000000;
+    static constexpr size_t baseband_fs = 2'000'000;
+    static constexpr size_t msg_len = 112;
 
     ADSBFrame frame{};
     bool configured{false};
+    bool decoding{false};
+
     uint32_t prev_mag{0};
-    size_t bit_count{0}, sample_count{0};
-    size_t msgLen{112};
-    uint32_t shifter[ADSB_PREAMBLE_LENGTH + 1];
-    bool decoding{};
-    bool preamble{}, active{};
-    uint16_t bit_pos{0};
-    uint8_t cur_bit{0};
-    uint32_t sample{0};
-    int32_t re{}, im{};
     int32_t amp{0};
+    size_t bit_count{0};
+    size_t sample_count{0};
+
+    uint32_t shifter[ADSB_PREAMBLE_LENGTH + 1];
 
     /* NB: Threads should be the last members in the class definition. */
     BasebandThread baseband_thread{baseband_fs, this, baseband::Direction::Receive};