POCSAG State machine fix (#1410)

* Reset color for well-formed message fragments

* better colors

* Fix POGSAG decode state machine

* Invert is_message to make more clear

* Use new escape string constants

* Run ECC twice, better diagnostics

* center status icon

---------

Co-authored-by: kallanreed <kallanreed@noreply.github.com>

firmware/application/apps/pocsag_app.cpp

@@ -43,10 +43,8 @@ void POCSAGLogger::log_raw_data(const pocsag::POCSAGPacket& packet, const uint32
     log_file.write_entry(packet.timestamp(), entry);
 }
 
-void POCSAGLogger::log_decoded(
+void POCSAGLogger::log_decoded(Timestamp timestamp, const std::string& text) {
-    const pocsag::POCSAGPacket& packet,
+    log_file.write_entry(timestamp, text);
-    const std::string text) {
-    log_file.write_entry(packet.timestamp(), text);
 }
 
 namespace ui {
@@ -59,6 +57,7 @@ POCSAGSettingsView::POCSAGSettingsView(
         {&check_log,
          &check_log_raw,
          &check_small_font,
+         &check_show_bad,
          &check_ignore,
          &field_ignore,
          &button_save});
@@ -66,6 +65,7 @@ POCSAGSettingsView::POCSAGSettingsView(
     check_log.set_value(settings_.enable_logging);
     check_log_raw.set_value(settings_.enable_raw_log);
     check_small_font.set_value(settings_.enable_small_font);
+    check_show_bad.set_value(settings_.hide_bad_data);
     check_ignore.set_value(settings_.enable_ignore);
     field_ignore.set_value(settings_.address_to_ignore);
 
@@ -73,6 +73,7 @@ POCSAGSettingsView::POCSAGSettingsView(
         settings_.enable_logging = check_log.value();
         settings_.enable_raw_log = check_log_raw.value();
         settings_.enable_small_font = check_small_font.value();
+        settings_.hide_bad_data = check_show_bad.value();
         settings_.enable_ignore = check_ignore.value();
         settings_.address_to_ignore = field_ignore.value();
 
@@ -93,6 +94,7 @@ POCSAGAppView::POCSAGAppView(NavigationView& nav)
          &field_frequency,
          &field_volume,
          &image_status,
+         &text_packet_count,
          &button_ignore_last,
          &button_config,
          &console});
@@ -143,46 +145,25 @@ void POCSAGAppView::refresh_ui() {
             : &Styles::white);
 }
 
-void POCSAGAppView::on_packet(const POCSAGPacketMessage* message) {
+void POCSAGAppView::handle_decoded(Timestamp timestamp, const std::string& prefix) {
-    packet_toggle = !packet_toggle;
+    bool bad_data = pocsag_state.errors >= 3;
-    image_status.set_foreground(packet_toggle
-                                    ? Color::dark_grey()
-                                    : Color::white());
 
-    const uint32_t roundVal = 50;
-    const uint32_t bitrate_rounded = roundVal * ((message->packet.bitrate() + (roundVal / 2)) / roundVal);
-    auto bitrate = to_string_dec_uint(bitrate_rounded);
-    auto timestamp = to_string_datetime(message->packet.timestamp(), HM);
-    auto prefix = timestamp + " " + bitrate;
-
-    if (logging_raw())
-        logger.log_raw_data(message->packet, receiver_model.target_frequency());
-
-    if (message->packet.flag() != NORMAL) {
-        console.writeln("\n" STR_COLOR_DARK_RED + prefix + " CRC ERROR: " + pocsag::flag_str(message->packet.flag()));
-        last_address = 0;
-        return;
-    } else {
-        pocsag_decode_batch(message->packet, &pocsag_state);
-
-        /*
     // Too many errors for reliable decode.
-        if (pocsag_state.errors >= 3) {
+    if (bad_data && hide_bad_data()) {
         console.write("\n" STR_COLOR_MAGENTA + prefix + " Too many decode errors.");
         last_address = 0;
         return;
     }
-        */
 
     // Ignored address.
     if (ignore() && pocsag_state.address == settings_.address_to_ignore) {
-            console.write("\n" STR_COLOR_DARK_CYAN + prefix + " Ignored: " + to_string_dec_uint(pocsag_state.address));
+        console.write("\n" STR_COLOR_CYAN + prefix + " Ignored: " + to_string_dec_uint(pocsag_state.address));
         last_address = pocsag_state.address;
         return;
     }
 
-        // Color indicates the message has lots of decoding errors.
+    // Color indicates the message has a lot of decoding errors.
-        std::string color = pocsag_state.errors >= 3 ? STR_COLOR_MAGENTA : "";
+    std::string color = bad_data ? STR_COLOR_MAGENTA : STR_COLOR_WHITE;
 
     std::string console_info = "\n" + color + prefix;
     console_info += " #" + to_string_dec_uint(pocsag_state.address);
@@ -194,7 +175,7 @@ void POCSAGAppView::on_packet(const POCSAGPacketMessage* message) {
 
         if (logging()) {
             logger.log_decoded(
-                    message->packet,
+                timestamp,
                 to_string_dec_uint(pocsag_state.address) +
                     " F" + to_string_dec_uint(pocsag_state.function) +
                     " Address only");
@@ -213,13 +194,64 @@ void POCSAGAppView::on_packet(const POCSAGPacketMessage* message) {
 
         if (logging()) {
             logger.log_decoded(
-                    message->packet,
+                timestamp,
                 to_string_dec_uint(pocsag_state.address) +
                     " F" + to_string_dec_uint(pocsag_state.function) +
                     " > " + pocsag_state.output);
         }
     }
 }
+
+static Color get_status_color(const POCSAGState& state) {
+    if (state.out_type == IDLE)
+        return Color::white();
+
+    switch (state.mode) {
+        case STATE_CLEAR:
+            return Color::cyan();
+        case STATE_HAVE_ADDRESS:
+            return Color::yellow();
+        case STATE_GETTING_MSG:
+            return Color::green();
+    }
+
+    // Shouldn't get here...
+    return Color::red();
+}
+
+void POCSAGAppView::on_packet(const POCSAGPacketMessage* message) {
+    const uint32_t roundVal = 50;
+    const uint32_t bitrate_rounded = roundVal * ((message->packet.bitrate() + (roundVal / 2)) / roundVal);
+    auto bitrate = to_string_dec_uint(bitrate_rounded);
+    auto timestamp = to_string_datetime(message->packet.timestamp(), HM);
+    auto prefix = timestamp + " " + bitrate;
+
+    // Display packet count to be able to tell whether baseband sent a packet for a tone.
+    ++packet_count;
+    text_packet_count.set(to_string_dec_uint(packet_count));
+
+    if (logging_raw())
+        logger.log_raw_data(message->packet, receiver_model.target_frequency());
+
+    if (message->packet.flag() != NORMAL) {
+        console.writeln("\n" STR_COLOR_RED + prefix + " CRC ERROR: " + pocsag::flag_str(message->packet.flag()));
+        last_address = 0;
+    } else {
+        // Set color before to be able to see if decode gets stuck.
+        image_status.set_foreground(Color::magenta());
+        pocsag_state.codeword_index = 0;
+        pocsag_state.errors = 0;
+
+        // Handle multiple messages (if any).
+        while (pocsag_decode_batch(message->packet, pocsag_state))
+            handle_decoded(message->packet.timestamp(), prefix);
+
+        // Handle the remainder.
+        handle_decoded(message->packet.timestamp(), prefix);
+    }
+
+    // Set status icon color to indicate state machine state.
+    image_status.set_foreground(get_status_color(pocsag_state));
 }
 
 void POCSAGAppView::on_stats(const POCSAGStatsMessage*) {

firmware/application/apps/pocsag_app.hpp

@@ -44,7 +44,7 @@ class POCSAGLogger {
     }
 
     void log_raw_data(const pocsag::POCSAGPacket& packet, const uint32_t frequency);
-    void log_decoded(const pocsag::POCSAGPacket& packet, const std::string text);
+    void log_decoded(Timestamp timestamp, const std::string& text);
 
    private:
     LogFile log_file{};
@@ -57,6 +57,7 @@ struct POCSAGSettings {
     bool enable_logging = false;
     bool enable_raw_log = false;
     bool enable_ignore = false;
+    bool hide_bad_data = false;
     uint32_t address_to_ignore = 0;
 };
 
@@ -87,14 +88,20 @@ class POCSAGSettingsView : public View {
         "Use Small Font",
         false};
 
-    Checkbox check_ignore{
+    Checkbox check_show_bad{
         {2 * 8, 8 * 16},
         22,
+        "Hide Bad Data",
+        false};
+
+    Checkbox check_ignore{
+        {2 * 8, 10 * 16},
+        22,
         "Enable Ignored Address",
         false};
 
     NumberField field_ignore{
-        {7 * 8, 9 * 16 + 8},
+        {7 * 8, 11 * 16 + 8},
         7,
         {0, 9999999},
         1,
@@ -118,6 +125,7 @@ class POCSAGAppView : public View {
     bool logging() const { return settings_.enable_logging; };
     bool logging_raw() const { return settings_.enable_raw_log; };
     bool ignore() const { return settings_.enable_ignore; };
+    bool hide_bad_data() const { return settings_.hide_bad_data; };
 
     NavigationView& nav_;
     RxRadioState radio_state_{
@@ -134,16 +142,19 @@ class POCSAGAppView : public View {
             {"small_font"sv, &settings_.enable_small_font},
             {"enable_logging"sv, &settings_.enable_logging},
             {"enable_ignore"sv, &settings_.enable_ignore},
+            {"address_to_ignore"sv, &settings_.address_to_ignore},
+            {"hide_bad_data"sv, &settings_.hide_bad_data},
         }};
 
     void refresh_ui();
+    void handle_decoded(Timestamp timestamp, const std::string& prefix);
     void on_packet(const POCSAGPacketMessage* message);
     void on_stats(const POCSAGStatsMessage* stats);
 
     uint32_t last_address = 0xFFFFFFFF;
     pocsag::POCSAGState pocsag_state{};
     POCSAGLogger logger{};
-    bool packet_toggle = false;
+    uint16_t packet_count = 0;
 
     RFAmpField field_rf_amp{
         {13 * 8, 0 * 16}};
@@ -163,11 +174,15 @@ class POCSAGAppView : public View {
         {28 * 8, 0 * 16}};
 
     Image image_status{
-        {7 * 8, 1 * 16 + 2, 16, 16},
+        {0 * 8 + 4, 1 * 16 + 2, 16, 16},
         &bitmap_icon_pocsag,
         Color::white(),
         Color::black()};
 
+    Text text_packet_count{
+        {3 * 8, 1 * 16 + 2, 5 * 8, 16},
+        "0"};
+
     Button button_ignore_last{
         {10 * 8, 1 * 16, 12 * 8, 20},
         "Ignore Last"};

firmware/common/pocsag.cpp

@@ -226,14 +226,17 @@ void pocsag_encode(const MessageType type, BCHCode& BCH_code, const uint32_t fun
 }
 
 // -------------------------------------------------------------------------------
+// Get the number of bits that differ between the two values.
 // -------------------------------------------------------------------------------
-inline int bitsDiff(unsigned long left, unsigned long right) {
+inline uint8_t bitsDiff(unsigned long left, unsigned long right) {
     unsigned long xord = left ^ right;
-    int count = 0;
+    uint8_t count = 0;
+
     for (int i = 0; i < 32; i++) {
-        if ((xord & 0x01) != 0) ++count;
+        if ((xord & 0x01) == 1) ++count;
         xord = xord >> 1;
     }
+
     return (count);
 }
 
@@ -310,7 +313,7 @@ void setupecc() {
 
 // -------------------------------------------------------------------------------
 // -------------------------------------------------------------------------------
-inline int errorCorrection(uint32_t* val) {
+inline int errorCorrection(uint32_t& val) {
     // Set up the tables the first time
     if (eccSetup == 0) {
         setupecc();
@@ -327,7 +330,7 @@ inline int errorCorrection(uint32_t* val) {
     // for (i=0; i<=20; i++)
     ecc = 0;
     for (i = 31; i >= 11; --i) {
-        if ((*val & (1 << i))) {
+        if (val & (1 << i)) {
             ecc = ecc ^ ecs[31 - i];
             pari = pari ^ 0x01;
         }
@@ -337,7 +340,7 @@ inline int errorCorrection(uint32_t* val) {
     acc = 0;
     for (i = 10; i >= 1; --i) {
         acc = acc << 1;
-        if ((*val & (1 << i))) {
+        if (val & (1 << i)) {
             acc = acc ^ 0x01;
         }
     }
@@ -355,12 +358,12 @@ inline int errorCorrection(uint32_t* val) {
             b2 = b2 & 0x1f;
 
             if (b2 != 0x1f) {
-                *val ^= 0x01 << (31 - b2);
+                val ^= 0x01 << (31 - b2);
                 ecc = ecc ^ ecs[b2];
             }
 
             if (b1 != 0x1f) {
-                *val ^= 0x01 << (31 - b1);
+                val ^= 0x01 << (31 - b1);
                 ecc = ecc ^ ecs[b1];
             }
 
@@ -377,78 +380,86 @@ inline int errorCorrection(uint32_t* val) {
     return errl;
 }
 
-void pocsag_decode_batch(const POCSAGPacket& batch, POCSAGState* const state) {
+bool pocsag_decode_batch(const POCSAGPacket& batch, POCSAGState& state) {
-    int errors = 0;
+    constexpr uint8_t codeword_max = 16;
-    uint32_t codeword;
+    state.output.clear();
-    char ascii_char;
-    std::string output_text = "";
 
-    state->out_type = EMPTY;
+    while (state.codeword_index < codeword_max) {
+        auto codeword = batch[state.codeword_index];
+        bool is_address = (codeword & 0x80000000U) == 0;
 
-    // For each codeword...
+        // Error correct twice. First time to fix any errors it can,
-    for (size_t i = 0; i < 16; i++) {
+        // second time to count number of errors that couldn't be fixed.
-        codeword = batch[i];
+        errorCorrection(codeword);
+        auto error_count = errorCorrection(codeword);
 
-        errorCorrection(&codeword);
+        switch (state.mode) {
-        errors = errorCorrection(&codeword);
+            case STATE_CLEAR:
+                if (is_address && codeword != POCSAG_IDLEWORD) {
+                    state.function = (codeword >> 11) & 3;
+                    state.address = (codeword >> 10) & 0x1FFFF8U;  // 18 MSBs are transmitted
+                    state.mode = STATE_HAVE_ADDRESS;
+                    state.out_type = ADDRESS;
+                    state.errors = error_count;
 
-        if (!(codeword & 0x80000000U)) {
+                    state.ascii_idx = 0;
-            // Address codeword
+                    state.ascii_data = 0;
-            if (state->mode == STATE_CLEAR) {
+                } else if (codeword == POCSAG_IDLEWORD) {
-                // if (codeword != POCSAG_IDLEWORD) {
+                    state.out_type = IDLE;
-                if (!(bitsDiff(codeword, POCSAG_IDLEWORD) < 1)) {
-                    state->function = (codeword >> 11) & 3;
-                    state->address = (codeword >> 10) & 0x1FFFF8U;  // 18 MSBs are transmitted
-                    state->mode = STATE_HAVE_ADDRESS;
-                    state->out_type = ADDRESS;
-                    state->errors = errors;
-
-                    state->ascii_idx = 0;
-                    state->ascii_data = 0;
                 }
-            } else {
+                break;
-                state->mode = STATE_CLEAR;  // New address = new message
+
-            }
+            case STATE_HAVE_ADDRESS:
-        } else {
+                if (is_address) {
-            state->errors += errors;
+                    // Got another address, return the current state.
-            // Message codeword
+                    state.mode = STATE_CLEAR;
-            if (state->mode == STATE_HAVE_ADDRESS) {
+                    return true;
-                // First message codeword: complete address
-                state->address |= (i >> 1);  // Add in the 3 LSBs (frame #)
-                state->mode = STATE_GETTING_MSG;
                 }
 
-            state->out_type = MESSAGE;
+                // First message codeword, complete the address.
+                state.address |= (state.codeword_index >> 1);  // Add in the 3 LSBs (frame #).
+                state.mode = STATE_GETTING_MSG;
+                [[fallthrough]];
 
-            state->ascii_data |= ((codeword >> 11) & 0xFFFFF);  // Get 20 message bits
+            case STATE_GETTING_MSG:
-            state->ascii_idx += 20;
+                if (is_address) {
+                    // Codeword isn't a message, return the current state.
+                    state.mode = STATE_CLEAR;
+                    return true;
+                }
 
-            // Raw 20 bits to 7 bit reversed ASCII
+                state.out_type = MESSAGE;
-            while (state->ascii_idx >= 7) {
+                state.errors += error_count;
-                state->ascii_idx -= 7;
+                state.ascii_data |= (codeword >> 11) & 0xFFFFF;  // Get 20 message bits.
-                ascii_char = ((state->ascii_data) >> (state->ascii_idx)) & 0x7F;
+                state.ascii_idx += 20;
 
-                // Bottom's up
+                // Raw 20 bits to 7 bit reversed ASCII.
+                // NB: This is processed MSB first, any remaining bits are shifted
+                // up so a whole 7 bits are processed with the next codeword.
+                while (state.ascii_idx >= 7) {
+                    state.ascii_idx -= 7;
+                    char ascii_char = (state.ascii_data >> state.ascii_idx) & 0x7F;
+
+                    // Bottom's up (reverse the bits).
                     ascii_char = (ascii_char & 0xF0) >> 4 | (ascii_char & 0x0F) << 4;  // 01234567 -> 45670123
                     ascii_char = (ascii_char & 0xCC) >> 2 | (ascii_char & 0x33) << 2;  // 45670123 -> 67452301
-                ascii_char = (ascii_char & 0xAA) >> 2 | (ascii_char & 0x55);       // 67452301 -> *7654321
+                    ascii_char = (ascii_char & 0xAA) >> 2 | (ascii_char & 0x55);       // 67452301 -> 76543210
 
-                // Translate non-printable chars
+                    // Translate non-printable chars. TODO: Leave CRLF?
-                if ((ascii_char < 32) || (ascii_char > 126)) {
+                    if (ascii_char < 32 || ascii_char > 126)
-                    // output_text += "[" + to_string_dec_uint(ascii_char) + "]";
+                        state.output += ".";
-                    output_text += ".";
+                    else
-                } else
+                        state.output += ascii_char;
-                    output_text += ascii_char;
                 }
 
-            state->ascii_data <<= 20;  // Remaining bits are for next time...
+                state.ascii_data <<= 20;  // Remaining bits are for next iteration...
-        }
+                break;
         }
 
-    state->output = output_text;
+        state.codeword_index++;
+    }
 
-    if (state->mode == STATE_HAVE_ADDRESS)
+    return false;
-        state->mode = STATE_CLEAR;
 }
 
 } /* namespace pocsag */

firmware/common/pocsag.hpp

@@ -35,7 +35,7 @@
 
 namespace pocsag {
 
-// Todo: these enums suck, make a better decode_batch
+// TODO: these enums suck, make a better decode_batch
 
 enum Mode : uint32_t {
     STATE_CLEAR,
@@ -45,6 +45,7 @@ enum Mode : uint32_t {
 
 enum OutputType : uint32_t {
     EMPTY,
+    IDLE,
     ADDRESS,
     MESSAGE
 };
@@ -56,6 +57,7 @@ enum MessageType : uint32_t {
 };
 
 struct POCSAGState {
+    uint8_t codeword_index;
     uint32_t function;
     uint32_t address;
     Mode mode = STATE_CLEAR;
@@ -78,7 +80,9 @@ std::string flag_str(PacketFlag packetflag);
 void insert_BCH(BCHCode& BCH_code, uint32_t* codeword);
 uint32_t get_digit_code(char code);
 void pocsag_encode(const MessageType type, BCHCode& BCH_code, const uint32_t function, const std::string message, const uint32_t address, std::vector<uint32_t>& codewords);
-void pocsag_decode_batch(const POCSAGPacket& batch, POCSAGState* const state);
+
+// Returns true if the batch has more to process.
+bool pocsag_decode_batch(const POCSAGPacket& batch, POCSAGState& state);
 
 } /* namespace pocsag */