ADSB RX App cleanup (#1569)

* WIP refactoring/gardening

* WIP cleanup, adding status dots

* Rename ageStep

* WIP Cleanup

* Wrapping up ADSB refactor/cleanup.

* Don't initialize strings to "     "

* Better map refresh

* Fix colorization of recent entries

* Fit and finish

firmware/application/apps/ui_adsb_rx.cpp

@@ -1,6 +1,7 @@
 /*
  * Copyright (C) 2015 Jared Boone, ShareBrained Technology, Inc.
  * Copyright (C) 2017 Furrtek
+ * Copyright (C) 2023 Kyle Reed
  *
  * This file is part of PortaPack.
  *
@@ -20,21 +21,23 @@
  * Boston, MA 02110-1301, USA.
  */
 
-#include <strings.h>
+#include <algorithm>
 
 #include "ui_adsb_rx.hpp"
 #include "ui_alphanum.hpp"
 
-#include "rtc_time.hpp"
-#include "string_format.hpp"
 #include "baseband_api.hpp"
 #include "portapack_persistent_memory.hpp"
+#include "rtc_time.hpp"
+#include "string_format.hpp"
 
 using namespace portapack;
 
 namespace ui {
 
-bool ac_details_view_active{false};
+static std::string get_map_tag(const AircraftRecentEntry& entry) {
+    return trimr(entry.callsign.empty() ? entry.icao_str : entry.callsign);
+}
 
 template <>
 void RecentEntriesTable<AircraftRecentEntries>::draw(
@@ -43,23 +46,25 @@ void RecentEntriesTable<AircraftRecentEntries>::draw(
     Painter& painter,
     const Style& style) {
     Color target_color;
-    auto entry_age = entry.age;
     std::string entry_string;
 
-    // Color decay for flights not being updated anymore
+    switch (entry.state) {
-    if (entry_age < ADSB_CURRENT) {
+        case ADSBAgeState::Invalid:
-        entry_string = "";
+        case ADSBAgeState::Current:
-        target_color = Color::green();
+            entry_string = "";
-    } else if (entry_age < ADSB_RECENT) {
+            target_color = Color::green();
-        entry_string = STR_COLOR_LIGHT_GREY;
+            break;
-        target_color = Color::light_grey();
+        case ADSBAgeState::Recent:
-    } else {
+            entry_string = STR_COLOR_LIGHT_GREY;
-        entry_string = STR_COLOR_DARK_GREY;
+            target_color = Color::light_grey();
-        target_color = Color::grey();
+            break;
-    }
+        default:
+            entry_string = STR_COLOR_DARK_GREY;
+            target_color = Color::grey();
+    };
 
     entry_string +=
-        (entry.callsign[0] != ' ' ? entry.callsign + " " : entry.icaoStr + "   ") +
+        (entry.callsign.empty() ? entry.icao_str + "   " : entry.callsign + " ") +
         to_string_dec_uint((unsigned int)(entry.pos.altitude / 100), 4) +
         to_string_dec_uint((unsigned int)entry.velo.speed, 4) +
         to_string_dec_uint((unsigned int)(entry.amp >> 9), 4) + " " +
@@ -72,47 +77,59 @@ void RecentEntriesTable<AircraftRecentEntries>::draw(
         entry_string);
 
     if (entry.pos.valid)
-        painter.draw_bitmap(target_rect.location() + Point(8 * 8, 0), bitmap_target, target_color, style.background);
+        painter.draw_bitmap(target_rect.location() + Point(8 * 8, 0),
+                            bitmap_target, target_color, style.background);
 }
 
-void ADSBLogger::log_str(std::string& logline) {
+/* ADSBLogger ********************************************/
-    log_file.write_entry(logline);
+
+void ADSBLogger::log(const ADSBLogEntry& log_entry) {
+    std::string log_line;
+    log_line.reserve(100);
+
+    log_line = log_entry.raw_data;
+    log_line += "ICAO:" + log_entry.icao;
+
+    if (!log_entry.callsign.empty())
+        log_line += " " + log_entry.callsign;
+
+    if (log_entry.pos.valid)
+        log_line += " Alt:" + to_string_dec_int(log_entry.pos.altitude) +
+                    " Lat:" + to_string_decimal(log_entry.pos.latitude, 7) +
+                    " Lon:" + to_string_decimal(log_entry.pos.longitude, 7);
+
+    if (log_entry.vel.valid)
+        log_line += " Type:" + to_string_dec_uint(log_entry.vel_type) +
+                    " Hdg:" + to_string_dec_uint(log_entry.vel.heading) +
+                    " Spd: " + to_string_dec_int(log_entry.vel.speed);
+
+    log_file.write_entry(log_line);
 }
 
-// Aircraft Details
+/* ADSBRxAircraftDetailsView *****************************/
-void ADSBRxAircraftDetailsView::focus() {
-    button_close.focus();
-}
-
-ADSBRxAircraftDetailsView::~ADSBRxAircraftDetailsView() {
-    on_close_();
-}
 
 ADSBRxAircraftDetailsView::ADSBRxAircraftDetailsView(
     NavigationView& nav,
-    const AircraftRecentEntry& entry,
+    const AircraftRecentEntry& entry) {
-    const std::function<void(void)> on_close)
+    add_children(
-    : entry_copy(entry),
+        {&labels,
-      on_close_(on_close) {
+         &text_icao_address,
-    add_children({&labels,
+         &text_registration,
-                  &text_icao_address,
+         &text_manufacturer,
-                  &text_registration,
+         &text_model,
-                  &text_manufacturer,
+         &text_type,
-                  &text_model,
+         &text_number_of_engines,
-                  &text_type,
+         &text_engine_type,
-                  &text_number_of_engines,
+         &text_owner,
-                  &text_engine_type,
+         &text_operator,
-                  &text_owner,
+         &button_close});
-                  &text_operator,
-                  &button_close});
 
-    std::unique_ptr<ADSBLogger> logger{};
+    text_icao_address.set(entry.icao_str);
-
-    icao_code = to_string_hex(entry_copy.ICAO_address, 6);
-    text_icao_address.set(to_string_hex(entry_copy.ICAO_address, 6));
 
     // Try getting the aircraft information from icao24.db
-    return_code = db.retrieve_aircraft_record(&aircraft_record, icao_code);
+    std::database db{};
+    std::database::AircraftDBRecord aircraft_record;
+    auto return_code = db.retrieve_aircraft_record(&aircraft_record, entry.icao_str);
     switch (return_code) {
         case DATABASE_RECORD_FOUND:
             text_registration.set(aircraft_record.aircraft_registration);
@@ -120,6 +137,7 @@ ADSBRxAircraftDetailsView::ADSBRxAircraftDetailsView(
             text_model.set(aircraft_record.aircraft_model);
             text_owner.set(aircraft_record.aircraft_owner);
             text_operator.set(aircraft_record.aircraft_operator);
+
             // Check for ICAO type, e.g. L2J
             if (strlen(aircraft_record.icao_type) == 3) {
                 switch (aircraft_record.icao_type[0]) {
@@ -142,7 +160,8 @@ ADSBRxAircraftDetailsView::ADSBRxAircraftDetailsView(
                         text_type.set("Tilt-wing aircraft");
                         break;
                 }
-                text_number_of_engines.set(std::string(1, aircraft_record.icao_type[1]));
+
+                text_number_of_engines.set(std::string{1, aircraft_record.icao_type[1]});
                 switch (aircraft_record.icao_type[2]) {
                     case 'P':
                         text_engine_type.set("Piston engine");
@@ -157,8 +176,8 @@ ADSBRxAircraftDetailsView::ADSBRxAircraftDetailsView(
                         text_engine_type.set("Electric engine");
                         break;
                 }
-
             }
+
             // Check for ICAO type designator
             else if (strlen(aircraft_record.icao_type) == 4) {
                 if (strcmp(aircraft_record.icao_type, "SHIP") == 0)
@@ -179,77 +198,49 @@ ADSBRxAircraftDetailsView::ADSBRxAircraftDetailsView(
                     text_type.set("Powered parachute/paraplane");
             }
             break;
+
         case DATABASE_NOT_FOUND:
             text_manufacturer.set("No icao24.db file");
             break;
     }
+
     button_close.on_select = [&nav](Button&) {
-        ac_details_view_active = false;
         nav.pop();
     };
-};
-
-// End of Aicraft details
-
-void ADSBRxDetailsView::focus() {
-    button_see_map.focus();
 }
 
-void ADSBRxDetailsView::update(const AircraftRecentEntry& entry) {
+void ADSBRxAircraftDetailsView::focus() {
-    entry_copy = entry;
+    button_close.focus();
-    uint32_t age = entry_copy.age;
-
-    if (age < 60)
-        text_last_seen.set(to_string_dec_uint(age) + " seconds ago");
-    else
-        text_last_seen.set(to_string_dec_uint(age / 60) + " minutes ago");
-
-    text_infos.set(entry_copy.info_string);
-    if (entry_copy.velo.heading < 360 && entry_copy.velo.speed >= 0) {  // I don't like this but...
-        text_info2.set("Hdg:" + to_string_dec_uint(entry_copy.velo.heading) + " Spd:" + to_string_dec_int(entry_copy.velo.speed));
-    } else {
-        text_info2.set("");
-    }
-    text_frame_pos_even.set(to_string_hex_array(entry_copy.frame_pos_even.get_raw_data(), 14));
-    text_frame_pos_odd.set(to_string_hex_array(entry_copy.frame_pos_odd.get_raw_data(), 14));
-
-    if (send_updates) {
-        geomap_view->update_tag(trimr(entry.callsign[0] != ' ' ? entry.callsign : to_string_hex(entry.ICAO_address, 6)));
-        geomap_view->update_position(entry_copy.pos.latitude, entry_copy.pos.longitude, entry_copy.velo.heading, entry_copy.pos.altitude);
-    }
 }
 
-ADSBRxDetailsView::~ADSBRxDetailsView() {
+/* ADSBRxDetailsView *************************************/
-    ac_details_view_active = false;
-    on_close_();
-}
 
 ADSBRxDetailsView::ADSBRxDetailsView(
     NavigationView& nav,
-    const AircraftRecentEntry& entry,
+    const AircraftRecentEntry& entry)
-    const std::function<void(void)> on_close)
+    : entry_(entry) {
-    : entry_copy(entry),
+    add_children(
-      on_close_(on_close) {
+        {&labels,
-    add_children({&labels,
+         &text_icao_address,
-                  &text_icao_address,
+         &text_callsign,
-                  &text_callsign,
+         &text_last_seen,
-                  &text_last_seen,
+         &text_airline,
-                  &text_airline,
+         &text_country,
-                  &text_country,
+         &text_infos,
-                  &text_infos,
+         &text_info2,
-                  &text_info2,
+         &text_frame_pos_even,
-                  &text_frame_pos_even,
+         &text_frame_pos_odd,
-                  &text_frame_pos_odd,
+         &button_aircraft_details,
-                  &button_aircraft_details,
+         &button_see_map});
-                  &button_see_map});
 
-    std::unique_ptr<ADSBLogger> logger{};
+    // The following won't change for a given airborne aircraft.
-    update(entry_copy);
+    // Try getting the airline's name from airlines.db.
+    // NB: Only works once callsign has been read and won't be updated.
+    std::database db;
+    std::database::AirlinesDBRecord airline_record;
+    std::string airline_code = entry_.callsign.substr(0, 3);
+    auto return_code = db.retrieve_airline_record(&airline_record, airline_code);
 
-    // The following won't (shouldn't !) change for a given airborne aircraft
-    // Try getting the airline's name from airlines.db
-    airline_code = entry_copy.callsign.substr(0, 3);
-    return_code = db.retrieve_airline_record(&airline_record, airline_code);
     switch (return_code) {
         case DATABASE_RECORD_FOUND:
             text_airline.set(airline_record.airline);
@@ -260,34 +251,125 @@ ADSBRxDetailsView::ADSBRxDetailsView(
             break;
     }
 
-    text_callsign.set(entry_copy.callsign);
+    text_icao_address.set(entry_.icao_str);
-    text_icao_address.set(to_string_hex(entry_copy.ICAO_address, 6));
 
     button_aircraft_details.on_select = [this, &nav](Button&) {
-        ac_details_view_active = true;
+        aircraft_details_view_ = nav.push<ADSBRxAircraftDetailsView>(entry_);
-        aircraft_details_view = nav.push<ADSBRxAircraftDetailsView>(entry_copy, [this]() { send_updates = false; });
+        nav.set_on_pop([this]() {
-        send_updates = false;
+            aircraft_details_view_ = nullptr;
+            refresh_ui();
+        });
     };
 
     button_see_map.on_select = [this, &nav](Button&) {
-        if (!send_updates) {  // Prevent recursively launching the map
+        geomap_view_ = nav.push<GeoMapView>(
-            geomap_view = nav.push<GeoMapView>(
+            get_map_tag(entry_),
-                trimr(entry_copy.callsign[0] != ' ' ? entry_copy.callsign : entry_copy.icaoStr),
+            entry_.pos.altitude,
-                entry_copy.pos.altitude,
+            GeoPos::alt_unit::FEET,
-                GeoPos::alt_unit::FEET,
+            entry_.pos.latitude,
-                entry_copy.pos.latitude,
+            entry_.pos.longitude,
-                entry_copy.pos.longitude,
+            entry_.velo.heading);
-                entry_copy.velo.heading,
+        nav.set_on_pop([this]() {
-                [this]() {
+            geomap_view_ = nullptr;
-                    send_updates = false;
+            refresh_ui();
-                });
+        });
-            send_updates = true;
-        }
     };
+
+    refresh_ui();
 };
 
-void ADSBRxView::focus() {
+void ADSBRxDetailsView::focus() {
-    field_vga.focus();
+    button_see_map.focus();
+}
+
+void ADSBRxDetailsView::update(const AircraftRecentEntry& entry) {
+    entry_ = entry;
+
+    if (aircraft_details_view_) {
+        // AC Details view is showing, nothing to update.
+    } else if (geomap_view_) {
+        // Map is showing, update the current item.
+        geomap_view_->update_tag(get_map_tag(entry_));
+        geomap_view_->update_position(entry.pos.latitude, entry.pos.longitude, entry.velo.heading, entry.pos.altitude);
+    } else {
+        // Details is showing, update details.
+        refresh_ui();
+    }
+}
+
+void ADSBRxDetailsView::clear_map_markers() {
+    if (geomap_view_)
+        geomap_view_->clear_markers();
+}
+
+bool ADSBRxDetailsView::add_map_marker(const AircraftRecentEntry& entry) {
+    // Map not shown, can't add markers.
+    if (!geomap_view_)
+        return false;
+
+    GeoMarker marker{};
+    marker.lon = entry.pos.longitude;
+    marker.lat = entry.pos.latitude;
+    marker.angle = entry.velo.heading;
+    marker.tag = get_map_tag(entry);
+
+    auto markerStored = geomap_view_->store_marker(marker);
+    return markerStored == MARKER_STORED;
+}
+
+void ADSBRxDetailsView::refresh_ui() {
+    auto age = entry_.age;
+    if (age < 60)
+        text_last_seen.set(to_string_dec_uint(age) + " seconds ago");
+    else
+        text_last_seen.set(to_string_dec_uint(age / 60) + " minutes ago");
+
+    text_callsign.set(entry_.callsign);
+    text_infos.set(entry_.info_string);
+    if (entry_.velo.heading < 360 && entry_.velo.speed >= 0)
+        text_info2.set("Hdg:" + to_string_dec_uint(entry_.velo.heading) +
+                       " Spd:" + to_string_dec_int(entry_.velo.speed));
+    else
+        text_info2.set("");
+
+    text_frame_pos_even.set(to_string_hex_array(entry_.frame_pos_even.get_raw_data(), 14));
+    text_frame_pos_odd.set(to_string_hex_array(entry_.frame_pos_odd.get_raw_data(), 14));
+}
+
+/* ADSBRxView ********************************************/
+
+ADSBRxView::ADSBRxView(NavigationView& nav) {
+    baseband::run_image(portapack::spi_flash::image_tag_adsb_rx);
+    add_children(
+        {&labels,
+         &field_lna,
+         &field_vga,
+         &field_rf_amp,
+         &rssi,
+         &recent_entries_view,
+         &status_frame,
+         &status_good_frame});
+
+    recent_entries_view.set_parent_rect({0, 16, 240, 272});
+    recent_entries_view.on_select = [this, &nav](const AircraftRecentEntry& entry) {
+        detail_key = entry.key();
+        details_view = nav.push<ADSBRxDetailsView>(entry);
+
+        nav.set_on_pop([this]() {
+            detail_key = AircraftRecentEntry::invalid_key;
+            details_view = nullptr;
+        });
+    };
+
+    signal_token_tick_second = rtc_time::signal_tick_second += [this]() {
+        on_tick_second();
+    };
+
+    logger = std::make_unique<ADSBLogger>();
+    logger->append(LOG_ROOT_DIR "/ADSB.TXT");
+
+    receiver_model.enable();
+    baseband::set_adsb();
 }
 
 ADSBRxView::~ADSBRxView() {
@@ -296,232 +378,185 @@ ADSBRxView::~ADSBRxView() {
     baseband::shutdown();
 }
 
-AircraftRecentEntry ADSBRxView::find_or_create_entry(uint32_t ICAO_address) {
+void ADSBRxView::focus() {
-    auto it = find(recent, ICAO_address);
+    field_vga.focus();
-
-    // If not found
-    if (it == std::end(recent)) {
-        recent.emplace_front(ICAO_address);  // Add it
-        it = find(recent, ICAO_address);     // Find it again
-    }
-    return *it;
-}
-
-void ADSBRxView::replace_entry(AircraftRecentEntry& entry) {
-    uint32_t ICAO_address = entry.ICAO_address;
-
-    std::replace_if(
-        recent.begin(), recent.end(),
-        [ICAO_address](const AircraftRecentEntry& compEntry) { return ICAO_address == compEntry.ICAO_address; },
-        entry);
-}
-
-void ADSBRxView::remove_old_entries() {
-    auto it = recent.rbegin();
-    auto end = recent.rend();
-    while (it != end) {
-        if (it->age_state >= 4) {
-            std::advance(it, 1);
-            recent.erase(it.base());
-        } else {
-            break;  // stop looking because the list is sorted
-        }
-    }
-}
-
-void ADSBRxView::sort_entries_by_state() {
-    // Sorting List pn age_state using lambda function as comparator
-    recent.sort([](const AircraftRecentEntry& left, const AircraftRecentEntry& right) { return (left.age_state < right.age_state); });
 }
 
 void ADSBRxView::on_frame(const ADSBFrameMessage* message) {
-    logger = std::make_unique<ADSBLogger>();
-    rtc::RTC datetime;
-    std::string callsign;
-    std::string str_info;
-    std::string logentry;
-
     auto frame = message->frame;
     uint32_t ICAO_address = frame.get_ICAO_address();
+    status_frame.toggle();
 
-    if (frame.check_CRC() && ICAO_address) {
+    // Bad frame, skip it.
-        rtcGetTime(&RTCD1, &datetime);
+    if (!frame.check_CRC() || ICAO_address == 0)
-        auto entry = find_or_create_entry(ICAO_address);
+        return;
-        frame.set_rx_timestamp(datetime.minute() * 60 + datetime.second());
+
-        entry.reset_age();
+    ADSBLogEntry log_entry;
-        if (entry.hits == 0) {
+    status_good_frame.toggle();
-            entry.amp = message->amp;  // Store amplitude on first hit
+
-        } else {
+    rtc::RTC datetime;
-            entry.amp = ((entry.amp * 15) + message->amp) >> 4;  // Update smoothed amplitude on updates
+    rtcGetTime(&RTCD1, &datetime);
+    frame.set_rx_timestamp(datetime.minute() * 60 + datetime.second());
+
+    // NB: Reference to update entry in-place.
+    auto& entry = find_or_create_entry(ICAO_address);
+    entry.inc_hit();
+    entry.reset_age();
+
+    // Store smoothed amplitude on updates.
+    entry.amp = entry.hits == 0
+                    ? message->amp
+                    : ((entry.amp * 15) + message->amp) >> 4;
+
+    log_entry.raw_data = to_string_hex_array(frame.get_raw_data(), 14);
+    log_entry.icao = entry.icao_str;
+
+    if (frame.get_DF() == DF_ADSB) {
+        uint8_t msg_type = frame.get_msg_type();
+        uint8_t msg_sub = frame.get_msg_sub();
+        uint8_t* raw_data = frame.get_raw_data();
+
+        // 4: // surveillance, altitude reply
+        if ((msg_type >= AIRCRAFT_ID_L) && (msg_type <= AIRCRAFT_ID_H)) {
+            entry.set_callsign(decode_frame_id(frame));
+            log_entry.callsign = entry.callsign;
         }
 
-        entry.inc_hit();
+        // 9:
-        if (logger) {
+        // 18: // Extended squitter/non-transponder
-            logentry += to_string_hex_array(frame.get_raw_data(), 14) + " ";
+        // 21: // Comm-B, identity reply
-            logentry += "ICAO:" + entry.icaoStr + " ";
+        // 20: // Comm-B, altitude reply
-        }
+        else if (((msg_type >= AIRBORNE_POS_BARO_L) && (msg_type <= AIRBORNE_POS_BARO_H)) ||
+                 ((msg_type >= AIRBORNE_POS_GPS_L) && (msg_type <= AIRBORNE_POS_GPS_H))) {
+            entry.set_frame_pos(frame, raw_data[6] & 4);
+            log_entry.pos = entry.pos;
 
-        if (frame.get_DF() == DF_ADSB) {
+            if (entry.pos.valid) {
-            uint8_t msg_type = frame.get_msg_type();
+                std::string str_info =
-            uint8_t msg_sub = frame.get_msg_sub();
+                    "Alt:" + to_string_dec_int(entry.pos.altitude) +
-            uint8_t* raw_data = frame.get_raw_data();
+                    " Lat:" + to_string_decimal(entry.pos.latitude, 2) +
+                    " Lon:" + to_string_decimal(entry.pos.longitude, 2);
 
-            // 4: // surveillance, altitude reply
+                entry.set_info_string(std::move(str_info));
-            if ((msg_type >= AIRCRAFT_ID_L) && (msg_type <= AIRCRAFT_ID_H)) {
-                callsign = decode_frame_id(frame);
-                entry.set_callsign(callsign);
-                if (logger) {
-                    logentry += callsign + " ";
-                }
             }
-            // 9:
-            // 18: { // Extended squitter/non-transponder
-            // 21: // Comm-B, identity reply
-            // 20: // Comm-B, altitude reply
-            else if (((msg_type >= AIRBORNE_POS_BARO_L) && (msg_type <= AIRBORNE_POS_BARO_H)) ||
-                     ((msg_type >= AIRBORNE_POS_GPS_L) && (msg_type <= AIRBORNE_POS_GPS_H))) {
-                entry.set_frame_pos(frame, raw_data[6] & 4);
 
-                if (entry.pos.valid) {
+        } else if (msg_type == AIRBORNE_VEL && msg_sub >= VEL_GND_SUBSONIC && msg_sub <= VEL_AIR_SUPERSONIC) {
-                    str_info = "Alt:" + to_string_dec_int(entry.pos.altitude) +
+            entry.set_frame_velo(frame);
-                               " Lat:" + to_string_decimal(entry.pos.latitude, 2) +
+            log_entry.vel = entry.velo;
-                               " Lon:" + to_string_decimal(entry.pos.longitude, 2);
+            log_entry.vel_type = msg_sub;
-
-                    entry.set_info_string(str_info);
-
-                    if (logger) {
-                        // printing the coordinates in the log file with more
-                        // resolution, as we are not constrained by screen
-                        // real estate there:
-
-                        std::string log_info = "Alt:" + to_string_dec_int(entry.pos.altitude) +
-                                               " Lat:" + to_string_decimal(entry.pos.latitude, 7) +
-                                               " Lon:" + to_string_decimal(entry.pos.longitude, 7);
-                        logentry += log_info + " ";
-                    }
-                }
-            } else if (msg_type == AIRBORNE_VEL && msg_sub >= VEL_GND_SUBSONIC && msg_sub <= VEL_AIR_SUPERSONIC) {
-                entry.set_frame_velo(frame);
-                if (logger) {
-                    logger->append(LOG_ROOT_DIR "/ADSB.TXT");
-                    logentry += "Type:" + to_string_dec_uint(msg_sub) +
-                                " Hdg:" + to_string_dec_uint(entry.velo.heading) +
-                                " Spd: " + to_string_dec_int(entry.velo.speed);
-                }
-            }
-            replace_entry(entry);
-        }  // frame.get_DF() == DF_ADSB
-
-        if (logger) {
-            logger->append(LOG_ROOT_DIR "/ADSB.TXT");
-            // will log each frame in format:
-            // 20171103100227 8DADBEEFDEADBEEFDEADBEEFDEADBEEF ICAO:nnnnnn callsign Alt:nnnnnn Latnnn.nn Lonnnn.nn
-            logger->log_str(logentry);
         }
     }
+
+    logger->log(log_entry);
 }
 
 void ADSBRxView::on_tick_second() {
-    if (recent.size() <= 16) {  // Not many entries update everything (16 is one screen full)
+    status_frame.reset();
-        updateDetailsAndMap(1);
+    status_good_frame.reset();
-        updateRecentEntries();
+
-    } else if (updateState == 0) {  // Even second
+    ++tick_count;
-        updateState = 1;
+    ++ticks_since_marker_refresh;
-        updateDetailsAndMap(2);
+
-    } else {  // Odd second only performed when there are many entries
+    // Small list, update all at once.
-        updateState = 0;
+    if (recent.size() <= max_update_entries) {
-        updateRecentEntries();
+        update_recent_entries(/*age_delta*/ 1);
+        refresh_ui();
+        return;
     }
+
+    // Too many items, split update work into two phases:
+    // Entry maintenance and UI update.
+    if ((tick_count & 1) == 0)
+        update_recent_entries(/*age_delta*/ 2);
+    else
+        refresh_ui();
 }
 
-void ADSBRxView::updateDetailsAndMap(int ageStep) {
+void ADSBRxView::refresh_ui() {
-    ui::GeoMarker marker;
+    // There's only one ticks handler, but 3 UIs that need to be updated.
-    bool storeNewMarkers = false;
+    // This code will dispatch updates to the currently active view.
 
-    // NB: Temporarily pausing updates in rtc_timer_tick context when viewing AC Details screen (kludge for some Guru faults)
+    if (details_view) {
-    // TODO: More targeted blocking of updates in rtc_timer_tick when ADSB processes are running
+        // The details view is showing, forward updates to that UI.
-    if (ac_details_view_active)
+        bool current_updated = false;
-        return;
+        bool map_needs_update = false;
 
-    // Sort and truncate the entries, grouped, newest group first
+        if (details_view->map_active()) {
-    sort_entries_by_state();
+            // Is it time to clear and refresh the map's markers?
-    truncate_entries(recent);
+            if (ticks_since_marker_refresh >= MARKER_UPDATE_SECONDS) {
-    remove_old_entries();
+                map_needs_update = true;
+                ticks_since_marker_refresh = 0;
+                details_view->clear_map_markers();
+            }
+        } else {
+            // Refresh map immediately once active.
+            ticks_since_marker_refresh = MARKER_UPDATE_SECONDS;
+        }
 
-    // Calculate if it is time to update markers
+        // Process the entries list.
-    if (send_updates && details_view && details_view->geomap_view) {
+        for (const auto& entry : recent) {
-        ticksSinceMarkerRefresh += ageStep;
+            // Found the entry being shown in details view. Update it.
-        if (ticksSinceMarkerRefresh >= MARKER_UPDATE_SECONDS) {  // Update other aircraft every few seconds
+            if (entry.key() == detail_key) {
-            storeNewMarkers = true;
+                details_view->update(entry);
-            ticksSinceMarkerRefresh = 0;
+                current_updated = true;
+            }
+
+            // NB: current entry also gets a marker so it shows up if map is panned.
+            if (map_needs_update && entry.pos.valid && entry.state <= ADSBAgeState::Recent) {
+                map_needs_update = details_view->add_map_marker(entry);
+            }
+
+            // Any work left to do?
+            if (current_updated && !map_needs_update)
+                break;
         }
     } else {
-        ticksSinceMarkerRefresh = MARKER_UPDATE_SECONDS;  // Send the markers as soon as the geoview exists
+        // Main page is the top view. Redraw the entries view.
-    }
-
-    // Increment age, and pass updates to the details and map
-    const bool otherMarkersCanBeSent = send_updates && storeNewMarkers && details_view && details_view->geomap_view;  // Save retesting all of this
-    MapMarkerStored markerStored = MARKER_NOT_STORED;
-    if (otherMarkersCanBeSent) {
-        details_view->geomap_view->clear_markers();
-    }
-    // Loop through all entries
-    for (auto& entry : recent) {
-        entry.inc_age(ageStep);
-
-        // Only if there is a details view
-        if (send_updates && details_view) {
-            if (entry.key() == detailed_entry_key)  // Check if the ICAO address match
-            {
-                details_view->update(entry);
-            }
-            // Store if the view is present and the list isn't full
-            // Note -- Storing the selected entry too, in case map panning occurs
-            if (otherMarkersCanBeSent && (markerStored != MARKER_LIST_FULL) && entry.pos.valid && (entry.age_state <= 2)) {
-                marker.lon = entry.pos.longitude;
-                marker.lat = entry.pos.latitude;
-                marker.angle = entry.velo.heading;
-                marker.tag = trimr(entry.callsign[0] != ' ' ? entry.callsign : entry.icaoStr);
-                markerStored = details_view->geomap_view->store_marker(marker);
-            }
-        }
-    }  // Loop through all entries, if only to update the age
-}
-
-void ADSBRxView::updateRecentEntries() {
-    // Redraw the list of aircraft
-    if (!send_updates) {
         recent_entries_view.set_dirty();
     }
 }
 
-ADSBRxView::ADSBRxView(NavigationView& nav) {
+void ADSBRxView::update_recent_entries(int age_delta) {
-    baseband::run_image(portapack::spi_flash::image_tag_adsb_rx);
+    for (auto& entry : recent)
-    add_children({&labels,
+        entry.inc_age(age_delta);
-                  &field_lna,
-                  &field_vga,
-                  &field_rf_amp,
-                  &rssi,
-                  &recent_entries_view});
 
-    recent_entries_view.set_parent_rect({0, 16, 240, 272});
+    // Sort and truncate the entries, grouped by state, newest first.
-    recent_entries_view.on_select = [this, &nav](const AircraftRecentEntry& entry) {
+    sort_entries_by_state();
-        detailed_entry_key = entry.key();
+    truncate_entries(recent);
-        details_view = nav.push<ADSBRxDetailsView>(
+    remove_expired_entries();
-            entry,
+}
-            [this]() {
-                send_updates = false;
-            });
-        send_updates = true;
-    };
 
-    signal_token_tick_second = rtc_time::signal_tick_second += [this]() {
+AircraftRecentEntry& ADSBRxView::find_or_create_entry(uint32_t ICAO_address) {
-        on_tick_second();
+    // Find ...
-    };
+    auto it = find(recent, ICAO_address);
+    if (it != recent.end())
+        return *it;
 
-    baseband::set_adsb();
+    // ... or Create.
+    return recent.emplace_front(ICAO_address);
+}
 
-    receiver_model.enable();
+void ADSBRxView::sort_entries_by_state() {
+    recent.sort([](const auto& left, const auto& right) {
+        return (left.state < right.state);
+    });
+}
+
+void ADSBRxView::remove_expired_entries() {
+    // NB: Assumes entried are sorted with oldest last.
+    auto it = recent.rbegin();
+    auto end = recent.rend();
+
+    // Find the first !expired entry from the back.
+    while (it != end) {
+        if (it->state != ADSBAgeState::Expired)
+            break;
+
+        std::advance(it, 1);
+    }
+
+    // Remove the range of expired items.
+    recent.erase(it.base(), recent.end());
 }
 
 } /* namespace ui */

firmware/application/apps/ui_adsb_rx.hpp

@@ -1,6 +1,7 @@
 /*
  * Copyright (C) 2015 Jared Boone, ShareBrained Technology, Inc.
  * Copyright (C) 2017 Furrtek
+ * Copyright (C) 2023 Kyle Reed
  *
  * This file is part of PortaPack.
  *
@@ -21,29 +22,23 @@
  */
 
 #include "ui.hpp"
-
 #include "ui_receiver.hpp"
 #include "ui_geomap.hpp"
-#include "string_format.hpp"
 
-#include "file.hpp"
-#include "database.hpp"
-#include "recent_entries.hpp"
-#include "log_file.hpp"
 #include "adsb.hpp"
-#include "message.hpp"
 #include "app_settings.hpp"
-#include "radio_state.hpp"
 #include "crc.hpp"
+#include "database.hpp"
+#include "file.hpp"
+#include "log_file.hpp"
+#include "message.hpp"
+#include "radio_state.hpp"
+#include "recent_entries.hpp"
+#include "string_format.hpp"
 
 using namespace adsb;
 
 namespace ui {
-
-#define ADSB_CURRENT 10  // Seconds
-#define ADSB_RECENT 30   // Seconds
-#define ADSB_REMOVE 300  // Used for removing old entries
-
 #define AIRCRAFT_ID_L 1  // aircraft ID message type (lowest type id)
 #define AIRCRAFT_ID_H 4  // aircraft ID message type (highest type id)
 
@@ -66,8 +61,26 @@ namespace ui {
 #define VEL_AIR_SUBSONIC 3
 #define VEL_AIR_SUPERSONIC 4
 
-#define O_E_FRAME_TIMEOUT 20  // timeout between odd and even frames
+#define O_E_FRAME_TIMEOUT 20     // timeout between odd and even frames
+#define MARKER_UPDATE_SECONDS 5  // "other" map marker redraw interval
 
+/* Thresholds (in seconds) that define the transition between ages. */
+struct ADSBAgeLimit {
+    static constexpr int Current = 10;
+    static constexpr int Recent = 30;
+    static constexpr int Expired = 300;
+};
+
+/* Age states used for sorting and drawing recent entries. */
+enum class ADSBAgeState : uint8_t {
+    Invalid,
+    Current,
+    Recent,
+    Old,
+    Expired,
+};
+
+/* Data extracted from ADSB frames. */
 struct AircraftRecentEntry {
     using Key = uint32_t;
 
@@ -76,30 +89,30 @@ struct AircraftRecentEntry {
     uint32_t ICAO_address{};
     uint16_t hits{0};
 
-    uint16_t age_state{1};
+    ADSBAgeState state{ADSBAgeState::Invalid};
-    uint32_t age{0};
+    uint32_t age{0};  // In seconds
     uint32_t amp{0};
     adsb_pos pos{false, 0, 0, 0};
     adsb_vel velo{false, 0, 999, 0};
     ADSBFrame frame_pos_even{};
     ADSBFrame frame_pos_odd{};
 
-    std::string icaoStr{"      "};
+    std::string icao_str{};
-    std::string callsign{"        "};
+    std::string callsign{};
-    std::string info_string{""};
+    std::string info_string{};
 
-    AircraftRecentEntry(
+    AircraftRecentEntry(const uint32_t ICAO_address)
-        const uint32_t ICAO_address)
         : ICAO_address{ICAO_address} {
-        this->icaoStr = to_string_hex(ICAO_address, 6);
+        this->icao_str = to_string_hex(ICAO_address, 6);
     }
 
+    /* RecentEntries helpers expect a "key" on every item. */
     Key key() const {
         return ICAO_address;
     }
 
-    void set_callsign(std::string& new_callsign) {
+    void set_callsign(std::string new_callsign) {
-        callsign = new_callsign;
+        callsign = std::move(new_callsign);
     }
 
     void inc_hit() {
@@ -122,8 +135,8 @@ struct AircraftRecentEntry {
         velo = decode_frame_velo(frame);
     }
 
-    void set_info_string(std::string& new_info_string) {
+    void set_info_string(std::string new_info_string) {
-        info_string = new_info_string;
+        info_string = std::move(new_info_string);
     }
 
     void reset_age() {
@@ -132,59 +145,59 @@ struct AircraftRecentEntry {
 
     void inc_age(int delta) {
         age += delta;
-        if (age < ADSB_CURRENT) {
+
-            age_state = pos.valid ? 0 : 1;
+        if (age < ADSBAgeLimit::Current)
-        } else if (age < ADSB_RECENT) {
+            state = pos.valid ? ADSBAgeState::Invalid
-            age_state = 2;
+                              : ADSBAgeState::Current;
-        } else if (age < ADSB_REMOVE) {
+
-            age_state = 3;
+        else if (age < ADSBAgeLimit::Recent)
-        } else {
+            state = ADSBAgeState::Recent;
-            age_state = 4;
+
-        }
+        else if (age < ADSBAgeLimit::Expired)
+            state = ADSBAgeState::Old;
+
+        else
+            state = ADSBAgeState::Expired;
     }
 };
 
+// NB: uses std::list underneath so assuming refs are NOT invalidated.
 using AircraftRecentEntries = RecentEntries<AircraftRecentEntry>;
 
+/* Holds data for logging. */
+struct ADSBLogEntry {
+    std::string raw_data{};
+    std::string icao{};
+    std::string callsign{};
+    adsb_pos pos{};
+    adsb_vel vel{};
+    uint8_t vel_type{};
+};
+
+// TODO: Make logging optional.
+/* Logs entries to a log file. */
 class ADSBLogger {
    public:
     Optional<File::Error> append(const std::filesystem::path& filename) {
         return log_file.append(filename);
     }
-    void log_str(std::string& logline);
+    void log(const ADSBLogEntry& log_entry);
 
    private:
     LogFile log_file{};
 };
 
+/* Shows detailed information about an aircraft. */
 class ADSBRxAircraftDetailsView : public View {
    public:
-    ADSBRxAircraftDetailsView(NavigationView&, const AircraftRecentEntry& entry, const std::function<void(void)> on_close);
+    ADSBRxAircraftDetailsView(
-    ~ADSBRxAircraftDetailsView();
+        NavigationView&,
-
+        const AircraftRecentEntry& entry);
-    ADSBRxAircraftDetailsView(const ADSBRxAircraftDetailsView&) = delete;
-    ADSBRxAircraftDetailsView(ADSBRxAircraftDetailsView&&) = delete;
-    ADSBRxAircraftDetailsView& operator=(const ADSBRxAircraftDetailsView&) = delete;
-    ADSBRxAircraftDetailsView& operator=(ADSBRxAircraftDetailsView&&) = delete;
 
     void focus() override;
-
+    std::string title() const override { return "AC Details"; }
-    void update(const AircraftRecentEntry& entry);
-
-    std::string title() const override { return "AC Details"; };
-
-    AircraftRecentEntry get_current_entry() { return entry_copy; }
-
-    std::database::AircraftDBRecord aircraft_record = {};
 
    private:
-    AircraftRecentEntry entry_copy{0};
-    std::function<void(void)> on_close_{};
-    bool send_updates{false};
-    std::database db = {};
-    std::string icao_code = "";
-    int return_code = 0;
-
     Labels labels{
         {{0 * 8, 1 * 16}, "ICAO:", Color::light_grey()},
         {{0 * 8, 2 * 16}, "Registration:", Color::light_grey()},
@@ -237,36 +250,34 @@ class ADSBRxAircraftDetailsView : public View {
         "Back"};
 };
 
+/* Shows detailed information about an aircraft's flight. */
 class ADSBRxDetailsView : public View {
    public:
-    ADSBRxDetailsView(NavigationView&, const AircraftRecentEntry& entry, const std::function<void(void)> on_close);
+    ADSBRxDetailsView(NavigationView&, const AircraftRecentEntry& entry);
-    ~ADSBRxDetailsView();
 
     ADSBRxDetailsView(const ADSBRxDetailsView&) = delete;
-    ADSBRxDetailsView(ADSBRxDetailsView&&) = delete;
     ADSBRxDetailsView& operator=(const ADSBRxDetailsView&) = delete;
-    ADSBRxDetailsView& operator=(ADSBRxDetailsView&&) = delete;
 
     void focus() override;
-
     void update(const AircraftRecentEntry& entry);
 
-    std::string title() const override { return "Details"; };
+    /* Calls forwarded to map view if shown. */
+    bool map_active() const { return geomap_view_; }
+    void clear_map_markers();
+    /* Adds a marker for the entry to the map. Returns true on success. */
+    bool add_map_marker(const AircraftRecentEntry& entry);
 
-    AircraftRecentEntry get_current_entry() { return entry_copy; }
+    std::string title() const override { return "Details"; }
-
-    std::database::AirlinesDBRecord airline_record = {};
-
-    GeoMapView* geomap_view{nullptr};
 
    private:
-    AircraftRecentEntry entry_copy{0};
+    void refresh_ui();
-    std::function<void(void)> on_close_{};
+
-    ADSBRxAircraftDetailsView* aircraft_details_view{nullptr};
+    GeoMapView* geomap_view_{nullptr};
-    bool send_updates{false};
+    ADSBRxAircraftDetailsView* aircraft_details_view_{nullptr};
-    std::database db = {};
+
-    std::string airline_code = "";
+    // NB: Keeping a copy so that it doesn't end up dangling
-    int return_code = 0;
+    // if removed from the recent entries list.
+    AircraftRecentEntry entry_{AircraftRecentEntry::invalid_key};
 
     Labels labels{
         {{0 * 8, 1 * 16}, "ICAO:", Color::light_grey()},
@@ -321,6 +332,7 @@ class ADSBRxDetailsView : public View {
         "See on map"};
 };
 
+/* Main ADSB application view and message dispatch. */
 class ADSBRxView : public View {
    public:
     ADSBRxView(NavigationView& nav);
@@ -332,46 +344,52 @@ class ADSBRxView : public View {
     ADSBRxView& operator=(ADSBRxView&&) = delete;
 
     void focus() override;
-
     std::string title() const override { return "ADS-B RX"; };
 
-    void replace_entry(AircraftRecentEntry& entry);
-    void remove_old_entries();
-    AircraftRecentEntry find_or_create_entry(uint32_t ICAO_address);
-    void sort_entries_by_state();
-
    private:
     RxRadioState radio_state_{
-        1090000000 /* frequency */,
+        1'090'000'000 /* frequency */,
-        2500000 /* bandwidth */,
+        2'500'000 /* bandwidth */,
-        2000000 /* sampling rate */,
+        2'000'000 /* sampling rate */,
         ReceiverModel::Mode::SpectrumAnalysis};
     app_settings::SettingsManager settings_{
         "rx_adsb", app_settings::Mode::RX};
 
     std::unique_ptr<ADSBLogger> logger{};
+
+    /* Event Handlers */
     void on_frame(const ADSBFrameMessage* message);
     void on_tick_second();
-    int updateState = {0};
+    void refresh_ui();
-    void updateRecentEntries();
-    void updateDetailsAndMap(int ageStep);
-
-#define MARKER_UPDATE_SECONDS (5)
-    int ticksSinceMarkerRefresh{MARKER_UPDATE_SECONDS - 1};
-
-    const RecentEntriesColumns columns{{{"ICAO/Call", 9},
-                                        {"Lvl", 3},
-                                        {"Spd", 3},
-                                        {"Amp", 3},
-                                        {"Hit", 3},
-                                        {"Age", 4}}};
-    AircraftRecentEntries recent{};
-    RecentEntriesView<RecentEntries<AircraftRecentEntry>> recent_entries_view{columns, recent};
 
     SignalToken signal_token_tick_second{};
+    uint8_t tick_count = 0;
+    uint16_t ticks_since_marker_refresh{MARKER_UPDATE_SECONDS};
+
+    /* Max number of entries that can be updated in a single pass.
+     * 16 is one screen of recent entries. */
+    static constexpr uint8_t max_update_entries = 16;
+
+    /* Recent Entries */
+    const RecentEntriesColumns columns{
+        {{"ICAO/Call", 9},
+         {"Lvl", 3},
+         {"Spd", 3},
+         {"Amp", 3},
+         {"Hit", 3},
+         {"Age", 4}}};
+    AircraftRecentEntries recent{};
+    RecentEntriesView<AircraftRecentEntries> recent_entries_view{columns, recent};
+
+    /* Entry Management */
+    void update_recent_entries(int age_delta);
+    AircraftRecentEntry& find_or_create_entry(uint32_t ICAO_address);
+    void sort_entries_by_state();
+    void remove_expired_entries();
+
+    /* The key of the entry in the details view if shown. */
+    AircraftRecentEntry::Key detail_key{AircraftRecentEntry::invalid_key};
     ADSBRxDetailsView* details_view{nullptr};
-    uint32_t detailed_entry_key{0};
-    bool send_updates{false};
 
     Labels labels{
         {{0 * 8, 0 * 8}, "LNA:   VGA:   AMP:", Color::light_grey()}};
@@ -386,7 +404,17 @@ class ADSBRxView : public View {
         {18 * 8, 0 * 16}};
 
     RSSI rssi{
-        {20 * 8, 4, 10 * 8, 8},
+        {20 * 8, 4, 10 * 7, 8},
+    };
+
+    ActivityDot status_frame{
+        {screen_width - 3, 5, 2, 2},
+        Color::white(),
+    };
+
+    ActivityDot status_good_frame{
+        {screen_width - 3, 9, 2, 2},
+        Color::green(),
     };
 
     MessageHandlerRegistration message_handler_frame{

firmware/application/baseband_api.cpp

@@ -278,8 +278,7 @@ void set_pocsag() {
 }
 
 void set_adsb() {
-    const ADSBConfigureMessage message{
+    const ADSBConfigureMessage message{};
-        1};
     send_message(&message);
 }
 

firmware/application/recent_entries.hpp

@@ -24,13 +24,13 @@
 
 #include "ui_widget.hpp"
 
+#include <algorithm>
 #include <cstddef>
 #include <cstdint>
-#include <list>
-#include <utility>
 #include <functional>
 #include <iterator>
-#include <algorithm>
+#include <list>
+#include <utility>
 
 template <class Entry>
 using RecentEntries = std::list<Entry>;
@@ -42,6 +42,13 @@ typename ContainerType::const_iterator find(const ContainerType& entries, const
         [key](typename ContainerType::const_reference e) { return e.key() == key; });
 }
 
+template <typename ContainerType, typename Key>
+typename ContainerType::iterator find(ContainerType& entries, const Key key) {
+    return std::find_if(
+        std::begin(entries), std::end(entries),
+        [key](typename ContainerType::const_reference e) { return e.key() == key; });
+}
+
 template <typename ContainerType>
 static void truncate_entries(ContainerType& entries, const size_t entries_max = 64) {
     while (entries.size() > entries_max) {

firmware/common/message.hpp

@@ -1089,13 +1089,9 @@ class APRSPacketMessage : public Message {
 
 class ADSBConfigureMessage : public Message {
    public:
-    constexpr ADSBConfigureMessage(
+    constexpr ADSBConfigureMessage()
-        const uint32_t test)
+        : Message{ID::ADSBConfigure} {
-        : Message{ID::ADSBConfigure},
-          test(test) {
     }
-
-    const uint32_t test;
 };
 
 class JammerConfigureMessage : public Message {

firmware/common/ui_widget.cpp

@@ -587,6 +587,28 @@ void ProgressBar::paint(Painter& painter) {
     painter.draw_rectangle(sr, s.foreground);
 }
 
+/* ActivityDot ***********************************************************/
+
+ActivityDot::ActivityDot(
+    Rect parent_rect,
+    Color color)
+    : Widget{parent_rect},
+      _color{color} {}
+
+void ActivityDot::paint(Painter& painter) {
+    painter.fill_rectangle(screen_rect(), _on ? _color : Color::grey());
+}
+
+void ActivityDot::toggle() {
+    _on = !_on;
+    set_dirty();
+}
+
+void ActivityDot::reset() {
+    _on = false;
+    set_dirty();
+}
+
 /* Console ***************************************************************/
 
 Console::Console(

firmware/common/ui_widget.hpp

@@ -317,6 +317,20 @@ class ProgressBar : public Widget {
     uint32_t _max = 100;
 };
 
+/* A simple status indicator that can be used to indicate activity. */
+class ActivityDot : public Widget {
+   public:
+    ActivityDot(Rect parent_rect, Color color);
+
+    void paint(Painter& painter) override;
+    void toggle();
+    void reset();
+
+   private:
+    bool _on{false};
+    Color _color;
+};
+
 class Console : public Widget {
    public:
     Console(Rect parent_rect);