Stricter PI acceptance algorithm (#106)

* Require three (instead of two) repeats of a new PI before accepting it * Require three (instead of two) synchronization pulses before locking
windytan · Jun 13, 2024 · 9016e3e · 9016e3e
1 parent ab6df48
commit 9016e3e
Show file tree

Hide file tree

Showing 5 changed files with 89 additions and 85 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -13,6 +13,9 @@
 * * Print a warning if the raw MPX input sample rate is not specified
 * * Improve error reporting in general
 * * Add `--output hex` (same as `--output-hex`) to mirror `--input hex`
+* Noise resistance improvements:
+  * Require three (instead of two) repeats of a new PI before accepting it
+  * Require three (instead of two) synchronization pulses before locking
 
 ## 0.21 (2024-01-26)
 

diff --git a/src/block_sync.cc b/src/block_sync.cc
@@ -173,30 +173,37 @@ ErrorCorrectionResult correctBurstErrors(Block block, Offset expected_offset) {
   return result;
 }
 
+// Could this pulse realistically follow other?
+bool SyncPulse::couldFollow(const SyncPulse& other) const {
+  const int sync_distance = bit_position - other.bit_position;
+
+  return sync_distance % kBlockLength == 0 && sync_distance / kBlockLength <= 6 &&
+         offset != Offset::invalid && other.offset != Offset::invalid &&
+         (getBlockNumberForOffset(other.offset) + sync_distance / kBlockLength) % 4 ==
+             getBlockNumberForOffset(offset);
+}
+
 void SyncPulseBuffer::push(Offset offset, int bitcount) {
   for (size_t i = 0; i < pulses_.size() - 1; i++) {
     pulses_[i] = pulses_[i + 1];
   }
 
   SyncPulse new_pulse;
-  new_pulse.offset   = offset;
-  new_pulse.bitcount = bitcount;
+  new_pulse.offset       = offset;
+  new_pulse.bit_position = bitcount;
 
   pulses_.back() = new_pulse;
 }
 
+// Search for three sync pulses in the correct cyclic rhythm
 bool SyncPulseBuffer::isSequenceFound() const {
-  for (size_t prev_i = 0; prev_i < pulses_.size() - 1; prev_i++) {
-    const int sync_distance = pulses_.back().bitcount - pulses_[prev_i].bitcount;
+  const auto& third = pulses_.back();
 
-    const bool found =
-        (sync_distance % kBlockLength == 0 && sync_distance / kBlockLength <= 6 &&
-         pulses_[prev_i].offset != Offset::invalid &&
-         (getBlockNumberForOffset(pulses_[prev_i].offset) + sync_distance / kBlockLength) % 4 ==
-             getBlockNumberForOffset(pulses_.back().offset));
-
-    if (found)
-      return true;
+  for (size_t i_first = 0; i_first < pulses_.size() - 2; i_first++) {
+    for (size_t i_second = i_first + 1; i_second < pulses_.size() - 1; i_second++) {
+      if (third.couldFollow(pulses_[i_second]) && pulses_[i_second].couldFollow(pulses_[i_first]))
+        return true;
+    }
   }
   return false;
 }

diff --git a/src/block_sync.h b/src/block_sync.h
@@ -25,8 +25,10 @@
 namespace redsea {
 
 struct SyncPulse {
-  Offset offset { Offset::invalid };
-  int bitcount  { -1 };
+  Offset offset{Offset::invalid};
+  int bit_position{-1};
+
+  bool couldFollow(const SyncPulse& other) const;
 };
 
 class SyncPulseBuffer {
@@ -39,8 +41,8 @@ class SyncPulseBuffer {
 };
 
 struct ErrorCorrectionResult {
-  bool     succeeded      { false };
-  uint32_t corrected_bits { 0 };
+  bool succeeded{false};
+  uint32_t corrected_bits{0};
 };
 
 class BlockStream {
@@ -58,20 +60,20 @@ class BlockStream {
   void findBlockInInputRegister();
   void handleNewlyReceivedGroup();
 
-  int      bitcount_                  { 0 };
-  int      num_bits_until_next_block_ { 1 };
-  uint32_t input_register_            { 0 };
-  Offset   expected_offset_           { Offset::A };
-  bool     is_in_sync_                { false };
+  int bitcount_{0};
+  int num_bits_until_next_block_{1};
+  uint32_t input_register_{0};
+  Offset expected_offset_{Offset::A};
+  bool is_in_sync_{false};
   RunningSum<int, 50> block_error_sum50_;
-  const    Options options_;
+  const Options options_;
   RunningAverage<float, kNumBlerAverageGroups> bler_average_;
-  Group    current_group_;
-  Group    ready_group_;
-  bool     has_group_ready_           { false };
-  size_t   num_bits_since_sync_lost_  { 0 };
+  Group current_group_;
+  Group ready_group_;
+  bool has_group_ready_{false};
+  size_t num_bits_since_sync_lost_{0};
   SyncPulseBuffer sync_buffer_;
 };
 
-} // namespace redsea
-#endif // BLOCK_SYNC_H_
+}  // namespace redsea
+#endif  // BLOCK_SYNC_H_
diff --git a/src/channel.h b/src/channel.h
@@ -23,49 +23,49 @@
 
 namespace redsea {
 
+// Normally, the PI code is not expected to change. This class keeps track of the current PI
+// code and ignores spurious bit errors.
 class CachedPI {
  public:
-  enum class Result {
-    ChangeConfirmed, NoChange, SpuriousChange
-  };
+  enum class Result { ChangeConfirmed, NoChange, SpuriousChange };
 
   CachedPI() = default;
-  Result update(uint16_t pi) {
-    pi_prev2_ = pi_prev1_;
-    pi_prev1_ = pi;
 
+  // Input the most recently received PI code.
+  Result update(const uint16_t pi) {
     Result status(Result::SpuriousChange);
 
-    // Repeated PI confirms that it changed
-    if (has_previous_ && pi_prev1_ == pi_prev2_) {
-      status = (pi == pi_confirmed_ ? Result::NoChange :
-                                      Result::ChangeConfirmed);
+    // Three repeats of the same PI --> confirmed change
+    if (has_previous_ && pi_prev1_ == pi_prev2_ && pi == pi_prev1_) {
+      status        = (pi == pi_confirmed_ ? Result::NoChange : Result::ChangeConfirmed);
       pi_confirmed_ = pi;
     }
 
-    // So noisy that two PIs in a row get corrupted
-    if (has_previous_ && (pi_prev1_ != pi_confirmed_ &&
-                          pi_prev2_ != pi_confirmed_ &&
-                          pi_prev1_ != pi_prev2_)) {
+    // So noisy that two PIs in a row get corrupted --> drop
+    if (has_previous_ && (pi != pi_confirmed_ && pi_prev1_ != pi_confirmed_ && pi != pi_prev1_)) {
       reset();
     } else {
       has_previous_ = true;
     }
+
+    pi_prev2_ = pi_prev1_;
+    pi_prev1_ = pi;
+
     return status;
   }
   uint16_t get() const {
     return pi_confirmed_;
   }
   void reset() {
     pi_confirmed_ = pi_prev1_ = pi_prev2_ = 0;
-    has_previous_ = false;
+    has_previous_                         = false;
   }
 
  private:
-  uint16_t pi_confirmed_ { 0 };
-  uint16_t pi_prev1_     { 0 };
-  uint16_t pi_prev2_     { 0 };
-  bool     has_previous_ { false };
+  uint16_t pi_confirmed_{0};
+  uint16_t pi_prev1_{0};
+  uint16_t pi_prev2_{0};
+  bool has_previous_{false};
 };
 
 class Channel {

diff --git a/src/util.h b/src/util.h
@@ -28,15 +28,15 @@
 namespace redsea {
 
 // extract N-bit integer from word, starting at starting_at from the right
-template<size_t N>
+template <size_t N>
 uint16_t getBits(uint16_t word, size_t starting_at) {
   static_assert(N > 0 && N <= 16, "");
   return (word >> starting_at) & ((1 << N) - 1);
 }
 
 // extract N-bit integer from the concatenation of word1 and word2, starting at
 // starting_at from the right
-template<size_t N>
+template <size_t N>
 uint32_t getBits(uint16_t word1, uint16_t word2, size_t starting_at) {
   static_assert(N > 0 && N <= 32, "");
   return (((word1 << 16) + word2) >> starting_at) & ((1 << N) - 1);
@@ -65,24 +65,20 @@ class CSVTable {
 
 std::vector<std::string> splitLine(const std::string& line, char delimiter);
 
-template<typename Container>
-std::vector<std::vector<std::string>> readCSVContainer(const Container& csvdata,
-                                              char delimiter) {
+template <typename Container>
+std::vector<std::vector<std::string>> readCSVContainer(const Container& csvdata, char delimiter) {
   std::vector<std::vector<std::string>> lines;
 
   std::transform(csvdata.cbegin(), csvdata.cend(), std::back_inserter(lines),
-                 [&](const std::string& line) {
-                   return splitLine(line, delimiter);
-                 });
+                 [&](const std::string& line) { return splitLine(line, delimiter); });
 
   return lines;
 }
 
-std::vector<std::vector<std::string>> readCSV(const std::string& filename,
-                                              char delimiter);
+std::vector<std::vector<std::string>> readCSV(const std::string& filename, char delimiter);
 CSVTable readCSVWithTitles(const std::string& filename, char delimiter);
 
-template<typename Container>
+template <typename Container>
 CSVTable readCSVContainerWithTitles(const Container& csvdata, char delimiter) {
   CSVTable table;
 
@@ -94,8 +90,7 @@ CSVTable readCSVContainerWithTitles(const Container& csvdata, char delimiter) {
 
     const std::vector<std::string> fields = splitLine(line, delimiter);
     if (is_title_row) {
-      for (size_t i = 0; i < fields.size(); i++)
-        table.titles[fields[i]] = i;
+      for (size_t i = 0; i < fields.size(); i++) table.titles[fields[i]] = i;
       is_title_row = false;
     } else {
       if (fields.size() <= table.titles.size())
@@ -108,22 +103,19 @@ CSVTable readCSVContainerWithTitles(const Container& csvdata, char delimiter) {
 
 class CarrierFrequency {
  public:
-  enum class Band {
-    LF_MF, FM
-  };
+  enum class Band { LF_MF, FM };
+
  public:
   explicit CarrierFrequency(uint16_t code, Band band = Band::FM);
   bool isValid() const;
   int kHz() const;
   std::string str() const;
-  friend bool operator== (const CarrierFrequency &f1,
-                          const CarrierFrequency &f2);
-  friend bool operator< (const CarrierFrequency &f1,
-                         const CarrierFrequency &f2);
+  friend bool operator==(const CarrierFrequency& f1, const CarrierFrequency& f2);
+  friend bool operator<(const CarrierFrequency& f1, const CarrierFrequency& f2);
 
  private:
-  uint16_t code_ {};
-  Band band_ { Band::FM };
+  uint16_t code_{};
+  Band band_{Band::FM};
 };
 
 class AltFreqList {
@@ -137,38 +129,38 @@ class AltFreqList {
 
  private:
   std::array<int, 25> alt_freqs_;
-  size_t num_expected_ { 0 };
-  size_t num_received_ { 0 };
-  bool lf_mf_follows_  { false };
+  size_t num_expected_{0};
+  size_t num_received_{0};
+  bool lf_mf_follows_{false};
 };
 
-template<typename T, size_t N>
+template <typename T, size_t N>
 class RunningSum {
  public:
   RunningSum() {
-    std::fill(history_.begin(), history_.end(), 0.f);
+    clear();
   }
   T getSum() const {
-    return std::accumulate(history_.cbegin(), history_.cend(), 0);
+    return std::accumulate(history_.cbegin(), history_.cend(), T{0});
   }
   void push(int number) {
     history_[pointer_] = number;
-    pointer_ = (pointer_ + 1) % history_.size();
+    pointer_           = (pointer_ + 1) % history_.size();
   }
   void clear() {
-    std::fill(history_.begin(), history_.end(), 0);
+    std::fill(history_.begin(), history_.end(), T{0});
   }
 
  private:
-  std::array<T, N> history_;
-  size_t pointer_ { 0 };
+  std::array<T, N> history_{};
+  size_t pointer_{};
 };
 
-template<typename T, size_t N>
+template <typename T, size_t N>
 class RunningAverage {
  public:
   RunningAverage() {
-    std::fill(history_.begin(), history_.end(), 0.f);
+    std::fill(history_.begin(), history_.end(), T{0});
   }
   void push(T value) {
     sum_ -= history_[ptr_];
@@ -178,13 +170,13 @@ class RunningAverage {
   }
 
   float getAverage() const {
-    return 1.0f * sum_ / history_.size();
+    return static_cast<float>(sum_) / history_.size();
   }
 
  private:
-  std::array<T, N> history_;
-  T      sum_ { 0 };
-  size_t ptr_ { 0 };
+  std::array<T, N> history_{};
+  T sum_{};
+  size_t ptr_{};
 };
 
 std::string rtrim(std::string s);