Acquisition-readiness sweep: security, dead code, duplicates, UI/UX, structure#53
Open
jfuginay wants to merge 31 commits into
Open
Acquisition-readiness sweep: security, dead code, duplicates, UI/UX, structure#53jfuginay wants to merge 31 commits into
jfuginay wants to merge 31 commits into
Conversation
Toolbar cells, add-palette rows, coachmark, edit hints, and Done/Add buttons now resolve through LocalizationManager (bar.* keys). Adds the 37-key bar.* block to es/fr/de/pl/uk catalogs (en/zh-Hant already had them), terminology-matched to each catalog's existing vocabulary. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…config The Cesium Ion JWT was hardcoded verbatim in two Swift files (MapViewController.swift, ATAKToolsView.swift) of a public repo. Migrate it to the same gitignored-xcconfig pattern already used for the Mapbox token: CESIUM_ION_TOKEN in Config.xcconfig -> Info.plist $(CESIUM_ION_TOKEN) substitution -> single runtime read via CesiumIonConfig (Bundle.main). Both former literals now alias the one config source. An empty token still builds and runs; only Ion-hosted world terrain is skipped. The old token remains in public git history and must be rotated at cesium.com. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
DirectTCPSender's no-CA branch installed a verify block that unconditionally complete(true)d every server certificate, silently exposing those sessions to MITM. Now: - Default (no CA, no opt-in): proper SecTrustEvaluateWithError against the system root store. - New TAKServer.allowUntrustedTLS (default false; custom init(from:) keeps previously-saved servers decoding) gates the old accept-any behavior behind an explicit per-server opt-in, surfaced as a "Trust Untrusted Certificates" toggle in the server edit form under a new Advanced area with a clear MITM warning, localized via LocalizationManager in all 7 catalogs (en/es/fr/de/pl/uk/zh-Hant). - Post-enrollment connects (SimpleEnrollView, DeepLinkHandler token flow, ConnectionStatusWidget reconnect) now pass the enrolled CA truststore so they validate via the pinned-CA path instead of falling into the no-CA branch; the token-enrollment flow also pins caCertificateName to the "alias-ca" chain it already stores, same as the CSR flow. Self-signed servers without a truststore now require the explicit toggle instead of being trusted silently. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…exceptions ATS was globally disabled. Inventory of actual cleartext consumers: TAK streams use NWConnection and RTSP/SRT video uses VLCKit sockets (neither is ATS-governed); enrollment, Marti REST, Cesium CDN and tile downloads are all https; the only http consumers in the URL loading system are the local MBTiles server on 127.0.0.1 and user-entered HTTP/HLS video feeds played by AVPlayer. Tightest working config: drop NSAllowsArbitraryLoads, keep NSAllowsLocalNetworking + localhost/127.0.0.1 exception domains, and add NSAllowsArbitraryLoadsForMedia so field video feeds (routinely plain http) keep playing — the exemption is scoped to AVFoundation media loads only. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
These files (5,098 LOC) have zero entries in project.pbxproj — the project uses explicit PBXFileReference entries with no file-system-synchronized groups, so they have never compiled into any target — and zero inbound references from any source file (re-verified by word-boundary type grep before deletion). They include superseded duplicates of live code: TAKMissionSyncManager/TAKDataPackageService (replaced by MissionSyncManager + TAKRestAPIClient) and CoordinateConverter (a wrong-math MGRS duplicate of the canonical MGRSConverter). Git history preserves them if ever needed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Each cluster was adversarially verified to have zero inbound references (word-boundary grep re-run before deletion); all were compiled into the shipping binary as pure bloat: - CoT-filter UI (5): CoTFilterPanel, CoTUnitListView, CoTFilterManager, CoTFilterCriteria, CoTFilterModel — built-but-never-integrated feature, absent from ToolSheetHost/ToolsLauncherSheet routing - Dead chat backend (3): ChatService, ChatCoTGenerator (incl. private GeoChatXMLParser), ChatStorageManager — superseded by ChatManager + ChatPersistence + ChatXMLGenerator, which every live consumer uses - Dead measurement cluster (3): MeasurementService, MeasurementOverlay, MeasurementButton — live measurement is MeasurementManager + CompactMeasurementOverlay; also corrected the stale comment at MapViewController.swift:4324 that claimed MeasurementPointAnnotation was in use - RadialMenu support (4): RadialMenuButton, RadialMenuItemView, RadialMenuAnimations, RadialMenuGestureHandler — live radial path is RadialMenuView/Models/Presets/ActionExecutor, which stay - Pre-Cesium MapLibre-3D path (4): MapLibre3DSettingsView, MapLibre3DView, Map3DSettingsView, MGRSGridToggleView — verified the live replacements first: Settings mgrsGridEnabled toggle + overlayCoordinator-driven GridOverlayView, and Cesium as the live 3D engine pbxproj PBXBuildFile/PBXFileReference/group/Sources entries removed (76 lines, 4 per file). Build verified: BUILD SUCCEEDED. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ces) From the '12 more compiled files' audit finding (16 itemized files, verified per-type zero inbound references; re-verified by word-boundary grep before deletion): AircraftAnnotation, CertificateSelectionView, CertificateManagementView (transitively dead via CertificateSelectionView), KMLImportView, MarkerAnnotationView (legacy MapKit annotation types never registered), ArcGISTileSource, WaypointListView (incl. WaypointDetailView and its rogue fresh TAKService() instance), ImprovedErrorDialog, OfflineMapModels (parallel unused offline-maps model set), UASVideoPipView, DataPackageButton, TrackRecordingButton, TrackRecordingView (transitively dead via TrackRecordingButton), VideoStreamButton, KMLMapIntegration. Deliberately KEPT despite zero references today: - SelfPositionMarkerImage.swift — the UI/UX audit fix for the dead selfMarkerStyle Settings picker wires it into both map engines - RemoteIdToPointMarkerConverter.swift — staged work on the gyb/RID roadmap, not rot pbxproj entries removed (60 lines, 4 per file). BUILD SUCCEEDED. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…pers at TAKService MultiServerFederation duplicated TAKService.shared's native multi-server support (serverConnections/connectedServerIds/connectToServer) with its own server list, status enum, CoT XML builder, and per-server fresh TAKService() instances that bypassed the singleton and would re-run omnitak_init(). Its addServer was never called, so getConnectedCount() was always 0: sendSelfPosition could never send (and would have minted a new SELF-<uuid> map entity per tick if it had), and the status helpers always reported zero servers. Changes: - Delete Utilities/Network/MultiServerFederation.swift (417 LOC) and its 4 pbxproj entries - Remove the federation @StateObject from ATAKMapView - Delete sendSelfPosition (zero callers; live PPLI is PositionBroadcastService) - Repoint multiServerConnectionStatus/multiServerDisplayName at takService.connectedServerIds + ServerManager.shared so they read the real connection state instead of the always-empty federation BUILD SUCCEEDED. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
CoordinateDisplayView.formatMGRS computed the 100km grid square as a coordinate hash (abs(Int(lat*10+lon*10)) % 60) and scaled degrees linearly for easting/northing — plausible-looking but wrong MGRS shown as the default readout on both map engines. Its formatUTM truncated the meridian-arc series (up to ~16km northing error). Both now delegate to MGRSConverter exactly as formatBNG/formatTWD97 already delegate to their converters; the private getLatitudeBand copy is gone. The callsign panel's formatCoordinates (MapViewController) returned a hardcoded '11T MN' grid zone with raw DMS digits glued on, for every user on earth. It now reads the coordinateDisplayFormat preference and delegates to CoordinateDisplayFormat.format(_:), which routes through MGRSConverter/BNGConverter/TWD97Converter. Operator-facing grid references in a TAK client must be real geodesy — a wrong-but-plausible MGRS string is a safety issue when relayed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…imestamps The codebase had no CoT serializer: ~18 files hand-rolled the <event> envelope with their own ISO8601DateFormatter() (47 instantiations split between fractional/non-fractional, so timestamp precision differed by feature) and 13 private escapeXML copies (some missing entities, some double-escaping). New CoT/CoTXMLBuilder.swift provides: - CoTXMLBuilder.buildEvent(uid:type:how:time:start:staleAfter:lat:lon: hae:ce:le:detail:) — single owner of the XML declaration, <event> attributes, and <point>; generators supply only their <detail> payload - String.xmlEscaped — the one 5-entity escaper (& first) - one static ISO8601 formatter pair (fractional canonical for output, non-fractional for fallback parsing); ISO8601DateFormatter is thread-safe so shared statics are safe Migrated generators: ChatXMLGenerator (GeoChat), LassoCotBuilders (delete tombstone + dest-routed rebuild), PositionBroadcastService (self-SA PPLI), WaypointManager, MarkerCoTGenerator, TeamCoTGenerator (3 events), GeofenceCoTGenerator (2 events), DigitalPointerService, EmergencyBeaconService, MeshtasticCoTConverter (2 events — also fixes a double-escaping bug where pre-escaped names were re-escaped inside remarks). Parsers (CoTMessageParser, ChatXMLParser, TAKRestAPIClient, DigitalPointerService) now parse timestamps via the shared pair — ChatXMLParser previously failed on fractional-seconds timestamps. GPX/KML exporters (TrackRecordingService, BreadcrumbTrailService) use the shared formatters and lost their two private escapeXML copies. CoTEventHandler no longer serializes a parsed event back to XML just to re-parse it into a ChatParticipant (createPresenceXML round-trip deleted; the presence XML never carried an endpoint so the direct mapping is equivalent). Dead zero-caller builders deleted: MapViewController.generateSelfCoT, ChatXMLGenerator .generatePresenceWithChatEndpoint/.generateImageGeoChatXML. Wire format notes: attribute order is uniform now (TAK parsers are attribute-order agnostic); timestamps are uniformly fractional; envelopes that previously omitted how= (WaypointManager) now carry h-g-i-g-o. CAS/MEDEVAC/SPOTREP views and MeshtasticCOTBridge are consolidated in follow-up commits (report-type fixes and bridge-path deletion respectively). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rading as MEDEVAC CASRequestView, MEDEVACRequestView, and SPOTREPView each carried a line-for-line clone of the same envelope builder and send/haptics block, and the copy-paste froze MEDEVAC's CoT type b-r-f-h-c into all three — so a 9-Line CAS request and a SPOTREP rendered and routed as CASEVAC on receiving TAK clients. Divergence had also crept in: SPOTREP escaped its fields, CAS/MEDEVAC interpolated user free-text raw (an & or < in remarks produced malformed CoT on the wire). New Features/Military/Services/MilitaryReportCoT.swift: - buildReportEvent(uid:type:senderCallsign:lat:lon:reportDetail: remarks:) — one envelope (h-g-i-g-o, 1h stale) on CoTXMLBuilder, contact + remarks escaped centrally - sendReport(xml:logTag:) — the shared TAKService send + os.Logger + haptic block; views keep only their alert-state toggles CoT type assignments (documented in MilitaryReportCoT.ReportType): - MEDEVAC: b-r-f-h-c — unchanged; standard casevac request type (ATAK Medline; this repo's federation layer mapped the b-r-f-h-c prefix to casevac) - 9-Line CAS: a-h-G — ATAK's fires workflow attaches the nine-line to the hostile target marker rather than emitting a bespoke request type, so the request plots a hostile ground target at the target coordinates with the <cas> detail riding along (MarkerCoTGenerator.CoTTypes hostile family) - SPOTREP: b-m-p-w — the codebase's own CoTTypes.spotReport constant; receivers (including our CoTMessageParser, which maps b-m-p-w to waypoint) render a map point at the observed location with the <spotrep> detail attached All report fields are now escaped via String.xmlEscaped (numeric fields interpolate directly). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ass delegates Four NSObject URLSessionDelegate classes each independently implemented accept-any-server-trust (TAKAPIURLSessionDelegate in TAKRestAPIClient, CSRSelfSignedCertificateDelegate, DeepLinkHandler's SelfSignedCertDelegate, CertificateEnrollmentService's non-private SelfSignedCertificateDelegate). Security-critical duplicated code: hardening had to be re-implemented four times and the copies had already drifted. New Networking/Services/TAKTLSSessionDelegate(trustMode:certificateId: certificateName:) generalizes the most capable copy (the REST one, which also resolves the mTLS client identity) with an explicit trust policy: - .anchored([SecCertificate]) — chain validation against CA anchors + system roots (basic X509, mirroring the streaming path) - .acceptUntrusted — explicit opt-in only - .system — default system evaluation Wiring (now respects the per-server allowUntrustedTLS flag from the earlier security commit): - TAKRestAPIClient: TAKAPIConfiguration(from:) derives trustMode with the same precedence as the streaming connect path — CA truststore > allowUntrustedTLS opt-in > system roots. Previously Marti REST accepted ANY certificate unconditionally; now a server that streams via its CA truststore gets the same anchored validation on REST. DirectTCPSender.loadCACertificates became static internal so both paths share the keychain truststore loader. - CSREnrollmentService: trustSelfSignedCerts flag maps to .acceptUntrusted/.system (unchanged behavior, shared code) - DeepLinkHandler + CertificateEnrollmentService: .acceptUntrusted — enrollment bootstraps the truststore from the server itself, so the bypass is explicit and documented at the construction site All four duplicate delegate classes deleted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
SecPKCS12Import was invoked in five files (CertificateImportPipeline x2, CertificateManager, CertificateEnrollmentService, DataPackageImportManager, TAKService), each with its own options dict, keychain-write and error conventions. They had demonstrably diverged: only DataPackageImportManager had the SecTrust fallback for truststores where kSecImportItemCertChain isn't populated; only the enrollment path set kSecAttrAccessibleAfterFirstUnlock on identities; chain label schemes differed (-cert-N vs -chain-N vs first-unsuffixed) while the CA loader looks up by exact label. CertificateImportPipeline is now the only file that calls SecPKCS12Import: - static parsePKCS12(_:password:) — the one import invocation, with errSecAuthFailed/errSecDecode diagnostics - static parseIdentity(_:password:) — parse-only helper for the mTLS load paths (no keychain writes) - directImport unified to the superset of the five conventions: delete-before-add, kSecAttrAccessibleAfterFirstUnlock on both identities and certs, SecTrust truststore fallback, and the first-cert-at-bare-label / label-N chain scheme that exact-label lookups (DirectTCPSender.loadCACertificates) depend on - removed a debug print that logged the cert password in plaintext Migrated callers: - CertificateManager.extractIdentity → parseIdentity - TAKService.loadP12Identity → parseIdentity - DataPackageImportManager.importP12Certificate → pipeline (labels unchanged: that scheme is now the pipeline's canonical one) - CertificateEnrollmentService.importP12Certificate → pipeline; the import chain became properly async (no semaphore bridging). CA chain labels move from alias-ca-chain-N to alias-ca/alias-ca-N — nothing looked up the -chain-N labels, and the bare-label form is what the CA loader's exact match expects. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two pipelines ran simultaneously on the same $meshNodes data and gave the same physical radio two identities: MeshtasticCOTBridge (always-on from MeshtasticManager.init) published uid MESHTASTIC-%08X uppercase, type a-f-G-U-C (friendly), ce/le=10 through CoTEventHandler; while enableAutoMapUpdates published uid mesh-%08x lowercase, type a-n-G-U-C (neutral), ce/le=50 via MeshtasticCoTConverter into the unrendered enhancedMarkers store. Mesh strangers rendered friendly on the map because the bridge's (wrong) affiliation was the one that landed in the rendered store. MeshtasticCoTConverter is now the single node→CoTEvent mapper (it owns takUID and the self-node friendly/neutral distinction); publishMeshNodesToMap/publishNodeToMap route its events through CoTEventHandler.shared.handle(.positionUpdate) — the same path inbound CoT takes — so they land in TAKService.cotEvents and render with the converter's deliberate neutral affiliation for non-self nodes. MeshtasticCOTBridge.swift deleted entirely: its only wiring was MeshtasticManager.configureCOTBridge, and with the duplicate mapping path gone nothing else in the file (duplicate XML renderer, statistics, its own $meshNodes subscription) had a caller. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… the single source TAKService published two marker stores: cotEvents (rendered by both map engines via MapViewController.cotMarkers) and enhancedMarkers (a richer UID-keyed dict with position history that NO view rendered). CoTEventHandler wrote both on every position update; writers that touched only enhancedMarkers (the old Meshtastic publish path) silently never appeared on the map, and any future divergence desynced the stores. enhancedMarkers' only consumers were BloodhoundService (which also subscribes to cotEvents and builds its own position history) and the never-assigned onMarkerUpdated callback. Deleted: - TAKService.enhancedMarkers + updateEnhancedMarker/getMarker/ getAllMarkers/removeStaleMarkers/shouldAddToHistory + history config + onMarkerUpdated - CoTEventHandler's dual writes (position + waypoint handlers) and the removeEvent dual delete - BloodhoundService's $enhancedMarkers subscription, processEnhancedMarkers, and the EnhancedCoTMarker updateTrack overload (the cotEvents subscription already feeds the same tracks) - EnhancedCoTMarker.swift (type + UnitAffiliation/UnitType/CoTPosition, all unused elsewhere), plus the dead UI that depended on it: CustomMarkerAnnotation.swift, MarkerInfoPanel's marker panel Live shared helpers that were co-located with the dead UI (InfoRow, cornerRadius(_:corners:)/RoundedCorner, Color.uiColor) moved to Shared/UI/Components/InfoRow.swift — they're used by EmergencyBeaconView, DigitalPointerView, ConnectionStatusWidget, RoutePlanningView, and MapViewController. MeshtasticManager.clearMeshMarkersFromMap now actually works: it removes mesh markers via CoTEventHandler.removeEvent instead of printing that markers 'will expire' against a store the map never read. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…caches The operator callsign lived in five+ places with two different defaults: PositionBroadcastService (the persisted source, 'ALPHA-1'), SettingsView/MapViewController @AppStorage (coherent only by key coincidence), ChatManager's Combine mirror, and GeofenceService / DigitalPointerService / EmergencyBeaconService / TeamService each defaulting to 'OmniTAK-iOS' — settable only via configure/updateCallsign calls that nothing ever made, so those services would stamp the wrong identity into CoT the moment their send paths get wired. PositionBroadcastService.shared is now the single source: - GeofenceService userId/userCallsign, DigitalPointerService and EmergencyBeaconService userCallsign became computed properties that read the source at XML-build time — cached copies, 'OmniTAK-iOS' defaults, and the dead configure(callsign:)/updateCallsign plumbing deleted (zero callers existed) - TeamService.currentUserCallsign seeds from the source and tracks it via Combine, routed through its own updateCallsign so the current team's member entry stays in sync ChatManager's existing Combine mirror of the same source and the view-layer @AppStorage bindings (same key, same default) are unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…status source TAKService published four parallel projections of connection status — connectionStatus: String, isConnected: Bool, connectionState: ConnectionStateSnapshot, connectedServerIds: Set<UUID> — which had to be written in lockstep at every transition site. The codebase already carried evidence of the desync hazard (sendCoT's 'cached state can get out of sync' comment, a DEBUG desync detector, and a resync sweep). Now connectionState is the only @published source: - isConnected and connectionStatus are computed off the snapshot - connectedServerIds is derived from serverConnections under the lock (same freshness as before — the stored set was also only refreshed by updateOverallConnectionState's sync sweep) - all transition sites (connect/connected/failed/disconnect/ updateOverallConnectionState) write exactly one snapshot; the multi-server 'Connected to X / Connected to N servers' strings ride in snapshot.status - .disconnected factory status aligned to the previously-displayed 'Disconnected' (was 'Not Connected') NetworkStatusBar's mirrored @State and WaypointDetailView's fresh TAKService() instance named in the finding were already gone — both files were deleted in the earlier dead-code stage. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The tool catalog was encoded three-plus times keyed by matching string ids with zero enforcement: ATAKTool.allTools plus ATAKToolsView's own 28-case switch (and 28 @State sheet flags + 28 .sheet modifiers), ToolsLauncherSheet's row arrays, ToolSheetHost's switch, and ToolbarCustomization.toolShortcuts (whose own comment admitted each id 'MUST be present in ToolSheetHost' with nothing checking). Drift had already happened: the fallback grid couldn't reach UAS Connect or Go to Coordinate at all. New Core/App/ToolRegistry.swift: ToolDescriptor(id:displayName:icon: description:destination:) — one catalog of every sheet-presented tool, destinations as view closures (a dismiss handle for the two that drive their own dismissal: data packages, point dropper). - ToolSheetHost.sheetView is now a registry lookup — the single dispatcher for the launcher, the customizable toolbar, AND the grid - ATAKToolsView: the parallel switch, 28 sheet states, and duplicate catalog deleted; the grid derives its entries from the registry (its three map-mode commands — drawing/measure/lasso — stay grid-local since they drive map state, not sheets) and opens tools by posting the same .openToolSheet notification the launcher uses. The grid now reaches uas/gotocoord, closing the drift. - TrackRecordingService: one shared instance on the registry replaces the separate instances the grid and host each newed up - ToolsLauncherSheet and the toolbar catalog keep their curated row presentation but are validated against the registry in DEBUG builds (assertionFailure on any id the registry can't dispatch) Adding a tool is now one registry entry plus optional surface styling. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The same great-circle initial-bearing formula (atan2(y,x) + 0-360 normalization) was re-implemented in ~14 places; magnetic-declination or back-azimuth refinements added to one copy could never reach the others. MeasurementCalculator (static, dependency-free) already carried the canonical bearing(from:to:), distance(from:to:), and haversineDistance(from:to:). Delegated the live copies, keeping each call site's local function name so the change is purely an implementation swap: - RangeBearingService (both copies: the measurement init and calculateTrueBearing/calculateDistance; its magnetic-declination layer now builds on the canonical true bearing) - LineOfSightService (calculateBearing + its hand-rolled haversine) - TurnByTurnNavigationService, NavigationService (bearing + distance) - MapLibreService.bearingBetween, TerrainVisualizationService, MapStateManager.RangeBearingState, BloodhoundTrack Not touched: MapViewController's haversineMetres (documented JS-parity requirement with the embedded Cesium code) and BloodhoundTrack's forward-projection geodesic (a destination projection, not a bearing copy). The byte-identical CoTFilterManager/CoTFilterModel pair and the dead CoordinateConverter named in the finding were already removed by the earlier dead-code stage. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Four UI/UX dead-ends from the acquisition audit:
- SimpleEnrollView: the big ATAK-style gray Cancel button next to OK
was a placeholder no-op ({ /* Cancel action */ }) on the first-run
onboarding path. It now calls dismiss(), matching the toolbar Cancel.
- Settings "Name" field removed. @AppStorage("userName") persisted a
value nothing reads — CoT self events, chat, and PLI all key off
userCallsign, which is the operator identity. The settings.name key
is dropped from all 7 catalogs.
- Settings hardcoded English routed through LocalizationManager:
Toolbar section, Customize Toolbar / Build your own, server
Connected / "%d servers", External gyb Detector + description,
Connect gyb Detector, Connected, MISSION section, Create new
mission. New settings.* keys added to all 7 language catalogs.
- MGRS grid is 2D-engine-only but its toggles weren't gated or
annotated: the Settings row and the Overlays panel now carry a
localized "2D map only" caption, and enabling the grid while the
Cesium globe is active auto-switches to the 2D map (prompt-free,
same precedent as kmlZoomToOverlay).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ilian Save works Three radial-menu dead taps from the audit, all on live marker/map surfaces reachable on BOTH engines: - Marker "Info": executeMarkerInfo posted .radialMenuShowMarkerInfo with zero observers (the old MarkerInfoPanel consumer was deleted with the dead EnhancedCoTMarker store). New MarkerRadialFeedback ViewModifier attaches at ATAKMapView.body level — covering Cesium 3D and Mapbox 2D — and presents the new MarkerInfoSheet as a modal sheet (name, affiliation, CoT type, canonical MGRS via MGRSConverter, lat/lon, altitude, created, remarks). - Marker "Share": clipboard-only with an unobserved confirmation notification — to the user, identical to a dead tap. The modifier now presents the system share sheet (UIActivityViewController via the existing LassoShareSheet wrapper) with the share text. - "Copy" / "Get info": pasteboard writes had no visual confirmation. The modifier shows a transient "Copied" HUD capsule (same pattern as the Cesium load-fallback note), localized in all 7 catalogs. - Civilian-mode "Save" (save_location): fell through to the generic .radialMenuCustomAction post whose only observer handles draw_shape/meshtastic. Now drops a neutral favorite marker (heart icon, SAVED-HHmm) through PointDropperService, the same path executeAddWaypoint uses. New keys (radial.copied, markerInfo.*) added to all 7 language catalogs. New views appended to MapContextMenus.swift because the pbxproj uses explicit file references. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…oute on the 3D globe Two dual-engine parity gaps on the DEFAULT (Cesium) engine: - Measurement: CompactMeasurementOverlay was mounted only in mapbox2DBody, while measurement triggers (radial measure actions, Tools grid, toolbar command) fire on both engines. On the globe the tool invisibly activated, swallowed every map tap into measurementManager, and offered no way to finish or cancel. The HUD is now shared chrome (measurementChrome) mounted on BOTH bodies, and the in-progress polyline bridges through CesiumMainMap as a systemYellow "meas-live" measurement with per-segment distance labels — same schema as the saved sessions already mirrored. - Navigation: RouteNavigationPanel and the route geometry were 2D-only (routeOverlayCoordinator renders via MKOverlay, which the WebView can't show). The panel is now shared chrome (routeNavigationChrome) on both bodies, and the active route bridges through CesiumMainMap: polyline in the route's color (calculated segment paths, falling back to straight waypoint legs) plus labelled waypoint billboards. "route-" joins the non-contact uid prefixes so tapping route geometry on the globe doesn't open the marker-context menu. Stale "stays Mapbox-only" comment corrected. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The Settings "Self-position marker" picker persisted selfMarkerStyle but nothing read it — its intended consumer SelfPositionMarkerImage lost its caller in the MapKit→Mapbox migration (the old MKUserLocation delegate consumed it; the Mapbox path hardcoded .puck2D()). - Mapbox 2D: the location puck now renders SelfPositionMarkerImage.milStdFriendlyCombat / .bullseye via Puck2DConfiguration(topImage:) (TacticalMapView.selfPuckType()), re-applied live in updateUIView when the persisted style changes. - Cesium 3D: CesiumMainMap takes selfMarkerStyle; "bullseye" suppresses the self entity's SFGPUCI---- SIDC so the HTML falls back to the friendly green disc + center-dot canvas billboard — the globe's bullseye analog. "milstd" keeps the milsymbol frame. Stale SettingsView comment pointing at the long-gone MKUserLocation handler corrected. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The still-reachable fallback grid ("Full Tools…" passthrough) rendered
ToolRegistry's hardcoded English displayNames next to the fully
localized ToolsLauncherSheet — a zh-Hant user tapping Full Tools
dropped into an all-English screen.
- ToolButton now renders loc.t("tools.<id>.title") (the same keys the
launcher uses) with the subtitle as the accessibility hint; the
header "Tools", "Show disabled" toggle, and "BETA" badge route
through new tools.grid.* keys.
- New keys for the 12 grid-only tool ids (chat, data, video, offline,
drawing, measure, lasso, bloodhound, meshtastic, echelon, uas,
settings) added to ALL 7 catalogs, with the current hardcoded
strings as the en values.
- Side effect: shared ids now use the launcher's curated labels in the
grid too (e.g. "Emergency" → "Emergency Beacon"), unifying the two
surfaces on one catalog.
Note: de/es/fr/uk/pl never had the 19 launcher tools.* keys (i18n v1
scoped en + zh-Hant for tools); those languages keep falling back to
English for shared ids, unchanged. All NEW keys ship in all 7.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…al menu PluginSettingsManager.isToolEnabled was consumed in exactly one place — the legacy 5x4 grid — so "disabling" a feature in Settings → Plugins appeared to work while the tool stayed one tap away on every modern surface. Now: - ToolsLauncherSheet (the primary tools surface) filters its sectioned catalog through isToolEnabled; sections left empty are dropped. - ToolbarConfigStore filters both the rendered bar items and the add-shortcut palette (openTool shortcuts, plus the measure/drawing map commands which map to the measure/drawing plugins). The persisted layout keeps the id, so re-enabling a plugin restores the button where the operator had it. The store re-publishes on plugin-state changes so the bar updates live. - Radial menus filter items via RadialMenuAction.pluginToolID (measure*, drawing actions, navigate/createRoute → routes, quickChat → chat, emergency → alert, custom meshtastic) at all three configuration choke points (showRadialMenu, showContextMenu, and the Mapbox showPointMarkerMenu extension). Core/non-gated actions (drop marker, copy, edit/delete, layers, mode) are untouched; settings/plugins/pointer remain always-enabled per PluginSettingsManager. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ggle + auto-reconnect Tracks — TrackRecordingService swallowed every persistence/export failure in print-only catches, so a failed save/delete/export looked like a silent no-op (and a track could vanish on next launch): - New @published lastError, set (localized via new tracks.error.* keys in all 7 catalogs) in saveTrack, deleteTrack, and the GPX/KML export writers. - TrackListView and TrackDetailView surface it as an alert (detail view needs its own — it presents as a sheet over the list). Note: the audit's suggested TrackRecordingView no longer exists; it was deleted as zero-reference dead code in the earlier sweep, so TrackListView + TrackDetailView are the live surfaces. gyb (audit-gyb findings 3 + 4, iOS side): - reconnectLast() now respects the Settings "External gyb Detector" toggle. GybManager.shared exists from app launch, so the unconditional poweredOn reconnect used to re-link the detector and plot RID markers with the feature OFF — with no purge timer running in that state, so the markers never went stale. handleLine gets a belt-and-braces enabled guard for the same reason. - Auto-reconnect after an unexpected BLE drop: didDisconnectPeripheral / didFailToConnect schedule a delayed reconnectLast() with 1s→30s exponential backoff, cancelled by user disconnect, toggle off, or a successful connect (the firmware already restarts advertising on disconnect — the phone just never tried again, killing the stream mid-demo until an app relaunch). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Mechanical extraction into sibling files in Features/Map/Controllers — no behavior change: - LocationManager.swift — the shared CLLocation source - TacticalMapView.swift — 2D Mapbox representable + coordinator + the radial-menu Mapbox bridge - CesiumMainMap.swift — 3D Cesium WKWebView representable + JS bridge - MapDrawingAnnotations.swift — MKAnnotation classes for drawing shapes - ATAKMapChrome.swift — ATAKStatusBar / ATAKBottomToolbar / ATAKSidePanel / OverlaySettingsPanel + button helpers The embedded ~300-line Cesium scene HTML moves out of the Swift string literal into a bundled resource (OmniTAKMobile/Resources/cesium_scene.html, same __CESIUM_ION_TOKEN__ placeholder convention as the Android asset); CesiumMainMap.html now loads it at runtime and injects the Ion token from CesiumIonConfig (Config.xcconfig → Info.plist), preserving the never-hardcode-the-token invariant from the security pass. ATAKMapView and its body stay in MapViewController.swift (now 2,208 lines). This was the single largest maintainability/bus-factor item in the acquisition audit: one file held both map engines, the GPS source, the chrome, and the scene HTML. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ATAKMapView.body now composes three layers in one ZStack: - the engine switch renders ONLY the bare engine (cesiumEngineView / mapboxEngineView) - engine-specific overlays stay conditional (2D-only: loading splash, cursor mode; the MGRS gridOverlay stays inside mapboxEngineView) - ONE mapChrome @ViewBuilder owns every overlay shared by both engines: point-drop crosshair, toolbars, side panels, status indicators, map overlay components, radial menu, GPS-follow, lasso pill, measurement HUD, route navigation panel interactiveOverlays is dissolved into mapChrome + the 2D conditional. Explicit zIndexes (crosshair 900 … radial 3000) all compete in the same ZStack, preserving the previous stacking. Why: chrome chained onto one engine's ZStack is the recurring bug class here — radial menu, side panels, measurement HUD, and GPS-follow each shipped broken on the other engine at some point. With one mount point a new overlay can no longer be wired into a single engine by accident. Verified in the simulator on both engines (chrome renders over Cesium globe and over the Mapbox Standard map). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A fresh TAKService() re-runs omnitak_init(), re-wires the CoT event handler, registers duplicate app-lifecycle observers, and creates competing connection/PPLI state divorced from the .shared instance the UI observes. The audit found live screens doing this (WaypointDetailView, MultiServerFederation — both already deleted on this branch). Making init private closes the bug class for good; the one remaining caller (ConnectionStatusWidget's DEBUG preview) now uses .shared. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The 10 XCTest files in OmniTAKMobileTests/ (~3,200 LOC of CoT parsing,
TAKService state, ServerValidator, CSR enrollment, mesh codec,
OpenDroneID, PPLI scheduler coverage) were in NO Xcode target: the
project had only app + UITests natives, the scheme's TestAction was
empty, and none of it ever compiled or ran ("a passing build proves
nothing" — acquisition audit).
- add PBXNativeTarget OmniTAKMobileTests (unit-test bundle, TEST_HOST
the app) via the xcodeproj gem; wire into the scheme's Testables
- fix the suite for this branch's consolidations:
- delete tests of deleted code (UnitAffiliation/UnitType enums,
maxHistoryPerUnit/historyRetentionTime — all removed with the
EnhancedCoTMarker store)
- rewrite CertificateHandlingTests against the real API
(CertificateFormatConverter.detectFormat, TAKCertificate,
CertificateImportPipeline()) — the originals targeted helpers that
never existed on this codebase
- update stale assertions to intentional behavior changes
(ConnectionStateSnapshot status "Disconnected"; scheme/path-prefixed
hosts now accepted for reverse-proxied servers; 404 wording)
- gate TAKPacketCodecTests' standalone-script block behind
-D STANDALONE_REPRODUCER (the old `#if swift(>=5.9)` guard was
always true and its top-level entry call broke the target build)
And one real bug the revived suite caught immediately:
- fix(validator): bare IPv6 hosts were always rejected — the
:port-strip heuristic turned "::1" into ":". Only strip when the
colon is a real host:port delimiter, and unwrap bracketed
"[::1]:8089". testValidationWithIPv6Address now passes;
bracketed-form coverage added.
xcodebuild test -scheme OmniTAKMobile: 176 tests, 0 failures.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Build-for-testing then test-without-building on an auto-selected iPhone simulator, for pushes to main / release/* and all PRs. SPM checkouts cached (keyed on the pbxproj — Package.resolved isn't committed; the package pins live in XCRemoteSwiftPackageReference). No secrets needed: Config.base.xcconfig ships empty token defaults so a clean checkout compiles. The repo previously had no CI at all — combined with the orphaned test suite, a green local build was the only signal. Now every PR proves build + 176 unit tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Five-stage sweep driven by the acquisition audit (security → dead code → duplicates → UI/UX → structure/tests/CI). Full app build green at every stage; the suite now runs 176 unit tests with 0 failures.
Security
Dead code
Duplicates / single-source consolidation
UI/UX
Structure / tests / CI
__CESIUM_ION_TOKEN__injection, matching the Android asset convention)Action items for J
Config.xcconfig→ Info.plist, but the exposed token itself must be revoked/reissued at https://ion.cesium.com/tokens.Verification
xcodebuild build— greenxcodebuild test(OmniTAKMobileTests, iPhone 17 Pro sim) — 176 tests, 0 failures🤖 Generated with Claude Code