feat(firmware): add verified event OTA installer - #2223
Draft
RCGV1 wants to merge 9 commits into
Draft
Conversation
Contributor
📝 WalkthroughWalkthroughEvent firmware handling now uses resilient manifest merging, HTTPS and image validation, policy-based notifications, event-specific presentation, and signed OTA contracts with verified artifact downloads. The UI exposes event branding and installation flows while preserving device targeting and cache behavior. ChangesEvent firmware metadata and presentation
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant FirmwareScreen
participant AvailabilityResolver
participant ContractVerifier
participant ArtifactDownloader
participant Installer
FirmwareScreen->>AvailabilityResolver: Resolve event OTA availability
AvailabilityResolver->>ContractVerifier: Verify signed OTA contract
ContractVerifier-->>AvailabilityResolver: Return verified contract
AvailabilityResolver-->>FirmwareScreen: Return compatible artifact
FirmwareScreen->>ArtifactDownloader: Prepare artifact
ArtifactDownloader-->>FirmwareScreen: Return verified local file
FirmwareScreen->>Installer: Start installation for expected device
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This was referenced Jul 29, 2026
Member
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Meshtastic/Views/Settings/Firmware/Firmware.swift (1)
411-421: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winPass
expectedNodeNumto local-file firmware install sheets.
Firmware.swiftpassesnode.numfor row installs, but the Catalyst file picker still opensNRFDFUSheetandESP32OTAIntroSheetwithout the target node. WhenexpectedNodeNumis nil, these sheets fall back to treating any connected device as valid, while row installs require the correct node.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Meshtastic/Views/Settings/Firmware/Firmware.swift` around lines 411 - 421, Update the local-file firmware sheet switch in the showInstallationSheet flow to pass the selected target node’s number as expectedNodeNum to NRFDFUSheet and ESP32OTAIntroSheet, matching the row-install paths; leave UF2MassStorageView unchanged unless it already supports the same parameter.
🧹 Nitpick comments (7)
Meshtastic/Views/Connect/EventFirmwareInfoView.swift (1)
278-288: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid byte-at-a-time accumulation for the icon download.
for try await byte in bytesruns one async-sequence iteration per byte — up to ~2M iterations for a max-size icon. Since the cap is already enforced,URLSession.data(for:)plus a length check is both simpler and far cheaper; if you want streaming, iteratebytes.chunksstyle buffers instead.♻️ Simpler bounded fetch
- let (bytes, response) = try await URLSession.shared.bytes(for: request) + let (data, response) = try await URLSession.shared.data(for: request) guard let response = response as? HTTPURLResponse, (200..<300).contains(response.statusCode), response.expectedContentLength <= Int64(EventFirmwareImageValidator.maximumEncodedBytes), - response.mimeType == "image/png" || response.mimeType == "image/jpeg" else { + response.mimeType == "image/png" || response.mimeType == "image/jpeg", + data.count <= EventFirmwareImageValidator.maximumEncodedBytes else { return nil } - var data = Data() - if response.expectedContentLength > 0 { - data.reserveCapacity(Int(response.expectedContentLength)) - } - for try await byte in bytes { - guard data.count < EventFirmwareImageValidator.maximumEncodedBytes else { - return nil - } - data.append(byte) - } return EventFirmwareImageValidator.image(from: data)Note this buffers the full body before the cap check; keep the streaming form if you must abort mid-transfer, but buffer in chunks rather than per byte.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Meshtastic/Views/Connect/EventFirmwareInfoView.swift` around lines 278 - 288, Update the icon download logic around the byte iteration to avoid per-byte async accumulation. Prefer a bounded URLSession data fetch followed by a maximum-size check before calling EventFirmwareImageValidator.image(from:), or, if early streaming cancellation is required, consume buffered chunks while preserving the existing cap enforcement and validation behavior.Meshtastic/Model/EventFirmwareOTASelector.swift (1)
1-2: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing file-level header/MARK comment on new Model files. Both new files start directly with their
importstatements with no// MARK: FileNameor copyright header, per repo convention.
Meshtastic/Model/EventFirmwareOTASelector.swift#L1-L2: add a// MARK: EventFirmwareOTASelector.swift(or copyright) header beforeimport Foundation.Meshtastic/Model/EventFirmwareOTAService.swift#L1-L2: add a// MARK: EventFirmwareOTAService.swift(or copyright) header beforeimport CryptoKit.As per coding guidelines, "Add
// MARK: FileNameor a file-level copyright comment at the top of files."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Meshtastic/Model/EventFirmwareOTASelector.swift` around lines 1 - 2, Add a file-level MARK or copyright header before the imports in both EventFirmwareOTASelector.swift (lines 1-2) and EventFirmwareOTAService.swift (lines 1-2), using each file’s name for the MARK comment.Source: Coding guidelines
Meshtastic/Model/EventFirmwareArtifactDownloader.swift (1)
1-3: 📐 Maintainability & Code Quality | 🔵 TrivialMissing file-level header comment.
This new file has no
// MARK: FileNameor copyright header at the top, unlikeEventFirmwareNotificationPolicy.swiftin the same PR. As per coding guidelines, "Add// MARK: FileNameor a file-level copyright comment at the top of files."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Meshtastic/Model/EventFirmwareArtifactDownloader.swift` around lines 1 - 3, Add the project-standard file-level header at the top of EventFirmwareArtifactDownloader.swift, using either a // MARK: FileName comment or the same copyright-header format established by EventFirmwareNotificationPolicy.swift. Keep it before the import statements.Source: Coding guidelines
Meshtastic/Model/EventFirmwareOTAContract.swift (1)
1-3: 📐 Maintainability & Code Quality | 🔵 TrivialMissing file-level header comment.
Same as
EventFirmwareArtifactDownloader.swift— no// MARK: FileNameor copyright header at the top of this new file. As per coding guidelines, "Add// MARK: FileNameor a file-level copyright comment at the top of files."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Meshtastic/Model/EventFirmwareOTAContract.swift` around lines 1 - 3, Add a file-level header at the top of EventFirmwareOTAContract.swift, using either the project’s standard // MARK: FileName format or the established copyright header pattern from EventFirmwareArtifactDownloader.swift, before the imports.Source: Coding guidelines
Meshtastic/Views/Settings/Firmware/EventFirmwareInstallerView.swift (2)
302-309: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLog the underlying error before collapsing it to a generic message.
Download/checksum/signature failures are exactly the cases that need diagnostics, and the caught
erroris discarded. Add a typedLoggercall (per the project'sLoggerextensions) while keeping the user-facing text generic.🪵 Proposed fix
} catch { + Logger.services.error("Event firmware artifact preparation failed: \(error.localizedDescription, privacy: .public)") await MainActor.run { preparationState = .failed( "The firmware package could not be downloaded and verified." )As per coding guidelines, "Use
OSLog/Loggerfor all logging... Prefer the typed loggers defined inMeshtastic/Extensions/Logger.swift."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Meshtastic/Views/Settings/Firmware/EventFirmwareInstallerView.swift` around lines 302 - 309, In the catch block of the firmware preparation flow, log the caught error using the project’s typed Logger extension before updating preparationState. Keep the existing generic user-facing failure message and preparationTask cleanup unchanged.Source: Coding guidelines
1-27: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd a file header and
// MARK: -separators.This file bundles the policy enum, the main view, a DI helper, a format extension, and two simulator-only types with no section markers or top-of-file comment.
As per coding guidelines, "Use
// MARK: -comments to separate logical sections within a file" and "Add// MARK: FileNameor a file-level copyright comment at the top of files."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Meshtastic/Views/Settings/Firmware/EventFirmwareInstallerView.swift` around lines 1 - 27, Add a file-level header comment at the top of EventFirmwareInstallerView.swift, then organize the policy enum, main view, dependency-injection helper, format extension, and simulator-only types into logical sections using // MARK: - separators. Keep the existing declarations and behavior unchanged.Source: Coding guidelines
Meshtastic/Views/Settings/Firmware/ESP32 OTA/ESP32OTAIntroSheet.swift (1)
38-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winOptional-
expectedNodeNumfallback is reimplemented in both OTA sheets. Both views wrapEventFirmwareInstallerPolicy.isExpectedDeviceActivewith an identicalguard letthat degrades to "any device is connected" whenexpectedNodeNumisnil. Move that fallback into the policy so the weaker semantics are defined once and can be tested.
Meshtastic/Views/Settings/Firmware/ESP32 OTA/ESP32OTAIntroSheet.swift#L38-L46: replace the computed property body with a single call to a policy overload acceptingInt64?.Meshtastic/Views/Settings/Firmware/NRF DFU/NRFDFUSheet.swift#L20-L28: apply the same replacement.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Meshtastic/Views/Settings/Firmware/ESP32` OTA/ESP32OTAIntroSheet.swift around lines 38 - 46, Move the optional expectedNodeNum fallback into EventFirmwareInstallerPolicy by adding or using an overload accepting Int64?, preserving “any device connected” behavior when nil. Replace the computed property bodies in Meshtastic/Views/Settings/Firmware/ESP32 OTA/ESP32OTAIntroSheet.swift lines 38-46 and Meshtastic/Views/Settings/Firmware/NRF DFU/NRFDFUSheet.swift lines 20-28 with a single policy call, leaving the non-optional policy logic centralized and reusable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Meshtastic/API/MeshtasticAPI.swift`:
- Around line 1073-1126: Update the manifest-to-entity assignments in the
payload mapping block so every remaining plain string field only overwrites
cached values when its payload value is non-empty. Apply this to displayName,
welcomeMessage, tag, eventStart, eventEnd, timeZone, location, domain, theme
name/tagline, theme fonts, and firmware slug/version/id/title/releaseNotes,
while preserving the existing URL and color validation behavior.
In `@Meshtastic/Assets.xcassets/EventFirmwareHAMVENTION.imageset/Contents.json`:
- Around line 3-7: Remove the "scale" entry from the hamvention.png image record
in the EventFirmwareHAMVENTION asset metadata, leaving the filename and idiom
unchanged so this single-scale imageset matches EventFirmwareDEFCON and
EventFirmwareFAB.
In `@Meshtastic/Views/Settings/Firmware/EventFirmwareInstallerView.swift`:
- Around line 204-219: Update the String-returning helpers actionTitle,
actionIcon, actionFooter, and unavailableMessage so each user-facing literal is
localized with String(localized:), preserving the existing conditional and
switch behavior. Ensure the corresponding Text and Label call sites receive the
localized strings rather than raw untranslated literals.
---
Outside diff comments:
In `@Meshtastic/Views/Settings/Firmware/Firmware.swift`:
- Around line 411-421: Update the local-file firmware sheet switch in the
showInstallationSheet flow to pass the selected target node’s number as
expectedNodeNum to NRFDFUSheet and ESP32OTAIntroSheet, matching the row-install
paths; leave UF2MassStorageView unchanged unless it already supports the same
parameter.
---
Nitpick comments:
In `@Meshtastic/Model/EventFirmwareArtifactDownloader.swift`:
- Around line 1-3: Add the project-standard file-level header at the top of
EventFirmwareArtifactDownloader.swift, using either a // MARK: FileName comment
or the same copyright-header format established by
EventFirmwareNotificationPolicy.swift. Keep it before the import statements.
In `@Meshtastic/Model/EventFirmwareOTAContract.swift`:
- Around line 1-3: Add a file-level header at the top of
EventFirmwareOTAContract.swift, using either the project’s standard // MARK:
FileName format or the established copyright header pattern from
EventFirmwareArtifactDownloader.swift, before the imports.
In `@Meshtastic/Model/EventFirmwareOTASelector.swift`:
- Around line 1-2: Add a file-level MARK or copyright header before the imports
in both EventFirmwareOTASelector.swift (lines 1-2) and
EventFirmwareOTAService.swift (lines 1-2), using each file’s name for the MARK
comment.
In `@Meshtastic/Views/Connect/EventFirmwareInfoView.swift`:
- Around line 278-288: Update the icon download logic around the byte iteration
to avoid per-byte async accumulation. Prefer a bounded URLSession data fetch
followed by a maximum-size check before calling
EventFirmwareImageValidator.image(from:), or, if early streaming cancellation is
required, consume buffered chunks while preserving the existing cap enforcement
and validation behavior.
In `@Meshtastic/Views/Settings/Firmware/ESP32` OTA/ESP32OTAIntroSheet.swift:
- Around line 38-46: Move the optional expectedNodeNum fallback into
EventFirmwareInstallerPolicy by adding or using an overload accepting Int64?,
preserving “any device connected” behavior when nil. Replace the computed
property bodies in Meshtastic/Views/Settings/Firmware/ESP32
OTA/ESP32OTAIntroSheet.swift lines 38-46 and
Meshtastic/Views/Settings/Firmware/NRF DFU/NRFDFUSheet.swift lines 20-28 with a
single policy call, leaving the non-optional policy logic centralized and
reusable.
In `@Meshtastic/Views/Settings/Firmware/EventFirmwareInstallerView.swift`:
- Around line 302-309: In the catch block of the firmware preparation flow, log
the caught error using the project’s typed Logger extension before updating
preparationState. Keep the existing generic user-facing failure message and
preparationTask cleanup unchanged.
- Around line 1-27: Add a file-level header comment at the top of
EventFirmwareInstallerView.swift, then organize the policy enum, main view,
dependency-injection helper, format extension, and simulator-only types into
logical sections using // MARK: - separators. Keep the existing declarations and
behavior unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7a32c34d-3e5c-4dd0-93a2-d1a1c1de68a7
⛔ Files ignored due to path filters (3)
Meshtastic/Assets.xcassets/EventFirmwareDEFCON.imageset/defcon34.pngis excluded by!**/*.pngMeshtastic/Assets.xcassets/EventFirmwareFAB.imageset/fab26.pngis excluded by!**/*.pngMeshtastic/Assets.xcassets/EventFirmwareHAMVENTION.imageset/hamvention.pngis excluded by!**/*.png
📒 Files selected for processing (81)
Meshtastic/API/MeshtasticAPI.swiftMeshtastic/Accessory/Accessory Manager/AccessoryManager+FromRadio.swiftMeshtastic/Assets.xcassets/EventFirmwareDEFCON.imageset/Contents.jsonMeshtastic/Assets.xcassets/EventFirmwareFAB.imageset/Contents.jsonMeshtastic/Assets.xcassets/EventFirmwareHAMVENTION.imageset/Contents.jsonMeshtastic/Enums/FirmwareEditionEnum.swiftMeshtastic/Extensions/UserDefaults.swiftMeshtastic/MeshtasticApp.swiftMeshtastic/Model/EventFirmwareArtifactDownloader.swiftMeshtastic/Model/EventFirmwareEntity.swiftMeshtastic/Model/EventFirmwareNotificationPolicy.swiftMeshtastic/Model/EventFirmwareOTAContract.swiftMeshtastic/Model/EventFirmwareOTASelector.swiftMeshtastic/Model/EventFirmwareOTAService.swiftMeshtastic/Model/EventFirmwarePresentation.swiftMeshtastic/Persistence/MarketingCapture.swiftMeshtastic/Persistence/Persistence.swiftMeshtastic/Persistence/UpdateSwiftData.swiftMeshtastic/Resources/docs/developer/architecture.htmlMeshtastic/Resources/docs/developer/swiftdata.htmlMeshtastic/Resources/docs/index.jsonMeshtastic/Resources/docs/markdown/developer/architecture.mdMeshtastic/Resources/docs/markdown/developer/swiftdata.mdMeshtastic/Resources/docs/markdown/user/firmware.mdMeshtastic/Resources/docs/user/firmware.htmlMeshtastic/Resources/event_firmware.jsonMeshtastic/Views/Connect/Connect.swiftMeshtastic/Views/Connect/EventFirmwareInfoView.swiftMeshtastic/Views/ContentView.swiftMeshtastic/Views/Helpers/Compact Widgets/ParticulateMatterCompactWidget.swiftMeshtastic/Views/Helpers/Compact Widgets/RadiationCompactWidget.swiftMeshtastic/Views/Helpers/MeshtasticLogo.swiftMeshtastic/Views/Nodes/Helpers/Map/MapSettingsForm.swiftMeshtastic/Views/Nodes/Helpers/Map/WaypointForm.swiftMeshtastic/Views/Nodes/Helpers/NodeListFilter.swiftMeshtastic/Views/Onboarding/DeviceOnboarding.swiftMeshtastic/Views/Provisioning/WifiProvisioningView.swiftMeshtastic/Views/Settings/AppData.swiftMeshtastic/Views/Settings/AppSettings.swiftMeshtastic/Views/Settings/Channels/ChannelForm.swiftMeshtastic/Views/Settings/Config/BluetoothConfig.swiftMeshtastic/Views/Settings/Config/DeviceConfig.swiftMeshtastic/Views/Settings/Config/DisplayConfig.swiftMeshtastic/Views/Settings/Config/LoRaConfig.swiftMeshtastic/Views/Settings/Config/Module/AmbientLightingConfig.swiftMeshtastic/Views/Settings/Config/Module/AudioConfig.swiftMeshtastic/Views/Settings/Config/Module/CannedMessagesConfig.swiftMeshtastic/Views/Settings/Config/Module/DetectionSensorConfig.swiftMeshtastic/Views/Settings/Config/Module/ExternalNotificationConfig.swiftMeshtastic/Views/Settings/Config/Module/MQTTConfig.swiftMeshtastic/Views/Settings/Config/Module/MeshBeaconConfig.swiftMeshtastic/Views/Settings/Config/Module/NeighborInfoConfig.swiftMeshtastic/Views/Settings/Config/Module/PaxCounterConfig.swiftMeshtastic/Views/Settings/Config/Module/RangeTestConfig.swiftMeshtastic/Views/Settings/Config/Module/SerialConfig.swiftMeshtastic/Views/Settings/Config/Module/StoreForwardConfig.swiftMeshtastic/Views/Settings/Config/Module/TelemetryConfig.swiftMeshtastic/Views/Settings/Config/Module/TrafficManagementConfig.swiftMeshtastic/Views/Settings/Config/NetworkConfig.swiftMeshtastic/Views/Settings/Config/PositionConfig.swiftMeshtastic/Views/Settings/Config/PowerConfig.swiftMeshtastic/Views/Settings/Config/SecurityConfig.swiftMeshtastic/Views/Settings/Discovery/DiscoveryScanView.swiftMeshtastic/Views/Settings/Firmware/ESP32 OTA/ESP32OTAIntroSheet.swiftMeshtastic/Views/Settings/Firmware/EventFirmwareInstallerView.swiftMeshtastic/Views/Settings/Firmware/Firmware.swiftMeshtastic/Views/Settings/Firmware/NRF DFU/NRFDFUSheet.swiftMeshtastic/Views/Settings/Routes.swiftMeshtastic/Views/Settings/ShareChannels.swiftMeshtastic/Views/Settings/TAKServerConfig.swiftMeshtastic/Views/Settings/UserConfig.swiftMeshtasticTests/EventFirmwareArtifactDownloaderTests.swiftMeshtasticTests/EventFirmwareInstallerViewTests.swiftMeshtasticTests/EventFirmwareMetadataTests.swiftMeshtasticTests/EventFirmwareNotificationTests.swiftMeshtasticTests/EventFirmwareOTAContractTests.swiftMeshtasticTests/EventFirmwareOTASelectorTests.swiftMeshtasticTests/EventFirmwareOTAServiceTests.swiftdocs/developer/architecture.mddocs/developer/swiftdata.mddocs/user/firmware.md
💤 Files with no reviewable changes (26)
- Meshtastic/Views/Helpers/Compact Widgets/ParticulateMatterCompactWidget.swift
- Meshtastic/Views/Settings/Config/Module/NeighborInfoConfig.swift
- Meshtastic/Views/Settings/Config/Module/AmbientLightingConfig.swift
- Meshtastic/Views/Settings/AppData.swift
- Meshtastic/Views/Settings/Config/Module/AudioConfig.swift
- Meshtastic/Views/Settings/ShareChannels.swift
- Meshtastic/Views/Settings/Config/Module/MeshBeaconConfig.swift
- Meshtastic/Views/Settings/Config/Module/DetectionSensorConfig.swift
- Meshtastic/Views/Helpers/Compact Widgets/RadiationCompactWidget.swift
- Meshtastic/Views/Settings/Config/Module/RangeTestConfig.swift
- Meshtastic/Views/Settings/Config/Module/StoreForwardConfig.swift
- Meshtastic/Views/Settings/Config/SecurityConfig.swift
- Meshtastic/Views/Settings/Config/DisplayConfig.swift
- Meshtastic/Views/Settings/Config/Module/ExternalNotificationConfig.swift
- Meshtastic/Views/Provisioning/WifiProvisioningView.swift
- Meshtastic/Views/Settings/Config/Module/SerialConfig.swift
- Meshtastic/Views/Settings/Config/Module/TrafficManagementConfig.swift
- Meshtastic/Views/Settings/Config/Module/CannedMessagesConfig.swift
- Meshtastic/Views/Settings/Discovery/DiscoveryScanView.swift
- Meshtastic/Views/Settings/Config/NetworkConfig.swift
- Meshtastic/Views/Settings/Config/LoRaConfig.swift
- Meshtastic/Views/Settings/Config/Module/MQTTConfig.swift
- Meshtastic/Views/Settings/Config/Module/TelemetryConfig.swift
- Meshtastic/Views/Settings/Config/PositionConfig.swift
- Meshtastic/Views/Settings/AppSettings.swift
- Meshtastic/Views/Settings/Config/PowerConfig.swift
Comment on lines
+1073
to
+1126
| if let value = payload.displayName { entity.displayName = value } | ||
| if let value = payload.welcomeMessage { entity.welcomeMessage = value } | ||
| if let value = payload.tag { entity.tag = value } | ||
| if let value = payload.eventStart { entity.eventStart = value } | ||
| if let value = payload.eventEnd { entity.eventEnd = value } | ||
| if let value = payload.timeZone { entity.timeZone = value } | ||
| if let value = payload.location { entity.location = value } | ||
| if let value = EventFirmwareURLPolicy.httpsURL(from: payload.iconUrl)?.absoluteString { | ||
| entity.iconUrl = value | ||
| } | ||
| if let value = payload.accentColor, | ||
| EventFirmwareEntity.color(fromHex: value) != nil { | ||
| entity.accentColor = value | ||
| } | ||
| if let value = payload.domain { entity.domain = value } | ||
| if let links = payload.links { | ||
| let safeLinks = links.map { | ||
| EventFirmwareEntity.Link(label: $0.label, url: $0.url) | ||
| }.filter { | ||
| EventFirmwareURLPolicy.httpsURL(from: $0.url) != nil | ||
| } | ||
| if !safeLinks.isEmpty { | ||
| entity.setLinks(safeLinks) | ||
| } | ||
| } | ||
| if let value = payload.theme?.name { entity.themeName = value } | ||
| if let value = payload.theme?.tagline { entity.themeTagline = value } | ||
| if let value = payload.theme?.colors?.primary, | ||
| EventFirmwareEntity.color(fromHex: value) != nil { | ||
| entity.themePrimaryColor = value | ||
| } | ||
| if let value = payload.theme?.colors?.secondary, | ||
| EventFirmwareEntity.color(fromHex: value) != nil { | ||
| entity.themeSecondaryColor = value | ||
| } | ||
| if let value = payload.theme?.colors?.accent, | ||
| EventFirmwareEntity.color(fromHex: value) != nil { | ||
| entity.themeAccentColor = value | ||
| } | ||
| if let palette = payload.theme?.palette { | ||
| let validPalette = palette.filter { | ||
| EventFirmwareEntity.color(fromHex: $0) != nil | ||
| } | ||
| if !validPalette.isEmpty { | ||
| entity.themePalette = validPalette | ||
| } | ||
| } | ||
| if let value = payload.theme?.fonts?.heading { entity.themeFontHeading = value } | ||
| if let value = payload.theme?.fonts?.body { entity.themeFontBody = value } | ||
| if let value = payload.firmware?.slug { entity.firmwareSlug = value } | ||
| if let value = payload.firmware?.version { entity.firmwareVersion = value } | ||
| if let value = payload.firmware?.id { entity.firmwareId = value } | ||
| if let value = payload.firmware?.title { entity.firmwareTitle = value } | ||
| if let value = payload.firmware?.releaseNotes { entity.firmwareReleaseNotes = value } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Empty strings from the manifest still overwrite cached copy.
if let value = payload.displayName treats "" as a supplied value, so an edition entry with blank strings blanks out good cached branding. The URL/color fields already validate; the plain string fields don't.
🛠️ Suggested guard
- if let value = payload.displayName { entity.displayName = value }
- if let value = payload.welcomeMessage { entity.welcomeMessage = value }
+ if let value = payload.displayName, !value.isEmpty { entity.displayName = value }
+ if let value = payload.welcomeMessage, !value.isEmpty { entity.welcomeMessage = value }(apply the same non-empty check to the remaining plain string fields)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Meshtastic/API/MeshtasticAPI.swift` around lines 1073 - 1126, Update the
manifest-to-entity assignments in the payload mapping block so every remaining
plain string field only overwrites cached values when its payload value is
non-empty. Apply this to displayName, welcomeMessage, tag, eventStart, eventEnd,
timeZone, location, domain, theme name/tagline, theme fonts, and firmware
slug/version/id/title/releaseNotes, while preserving the existing URL and color
validation behavior.
Comment on lines
+3
to
+7
| { | ||
| "filename" : "hamvention.png", | ||
| "idiom" : "universal", | ||
| "scale" : "1x" | ||
| } |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Drop the 1x scale to match the other event imagesets.
EventFirmwareDEFCON and EventFirmwareFAB use scale-less (single-scale) entries; pinning this one to 1x makes the artwork upscale on 2x/3x displays.
🎨 Proposed fix
{
"filename" : "hamvention.png",
- "idiom" : "universal",
- "scale" : "1x"
+ "idiom" : "universal"
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| { | |
| "filename" : "hamvention.png", | |
| "idiom" : "universal", | |
| "scale" : "1x" | |
| } | |
| { | |
| "filename" : "hamvention.png", | |
| "idiom" : "universal" | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Meshtastic/Assets.xcassets/EventFirmwareHAMVENTION.imageset/Contents.json`
around lines 3 - 7, Remove the "scale" entry from the hamvention.png image
record in the EventFirmwareHAMVENTION asset metadata, leaving the filename and
idiom unchanged so this single-scale imageset matches EventFirmwareDEFCON and
EventFirmwareFAB.
Comment on lines
+204
to
+219
| private var actionTitle: String { | ||
| installPurpose == .event ? "Install Event Firmware" : "Return to Standard Firmware" | ||
| } | ||
|
|
||
| private var actionIcon: String { | ||
| installPurpose == .event ? "calendar.badge.checkmark" : "arrow.uturn.backward.circle" | ||
| } | ||
|
|
||
| private var actionFooter: String { | ||
| switch availability { | ||
| case .available: | ||
| return "The download is verified before the existing firmware installer is opened. Keep the app open and your device nearby during installation." | ||
| case .unavailable: | ||
| return "This app cannot verify a compatible in-app package for this exact device. The web flasher provides the supported installation path." | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
These String-returning helpers bypass localization.
Text(actionFooter), Label(actionTitle, ...) and Label(unavailableMessage(for:), ...) take a String, not a LocalizedStringKey, so these strings ship untranslated while the adjacent inline literals (Lines 111–112) are localized. Wrap each literal in String(localized:) as done elsewhere in the codebase.
🌐 Proposed fix (pattern applies to all three helpers)
private var actionTitle: String {
- installPurpose == .event ? "Install Event Firmware" : "Return to Standard Firmware"
+ installPurpose == .event
+ ? String(localized: "Install Event Firmware")
+ : String(localized: "Return to Standard Firmware")
} case .contractUnavailable:
- return "No trusted installation contract is currently published for this event."
+ return String(localized: "No trusted installation contract is currently published for this event.")Also applies to: 234-253
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Meshtastic/Views/Settings/Firmware/EventFirmwareInstallerView.swift` around
lines 204 - 219, Update the String-returning helpers actionTitle, actionIcon,
actionFooter, and unavailableMessage so each user-facing literal is localized
with String(localized:), preserving the existing conditional and switch
behavior. Ensure the corresponding Text and Label call sites receive the
localized strings rather than raw untranslated literals.
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
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.
Status
Warning
Currently untested with real event firmware on physical hardware. This is a draft for contract, security, and UX review. Production in-app event installation remains disabled until Meshtastic provisions the API release signing key and the app embeds its corresponding trusted public key.
Stacked on and dependent on #2222. Until that PR lands, GitHub will also show its firmware-edition branding commits in this draft.
Cross-repository dependencies:
What changed?
pioEnv, hardware model, architecture, source-version floor, artifact version/format, byte count, SHA-256, and architecture-specific OTA compatibility.flasher.meshtastic.orgwhenever the app cannot prove an exact compatible in-app path.Why did it change?
Event metadata is display-oriented and cannot safely authorize firmware installation. A separate signed contract is needed so the app can select one immutable artifact for the exact connected target and verify it before entering the sensitive OTA flow. The contract also carries the standard-firmware return artifact so leaving an event edition is an explicit supported transition.
How is this tested?
Screenshots/Videos (when applicable)
Simulator-only UX is available in this branch. Real-device screenshots and recordings should be added after the server contract and hardware test matrix are approved.
Checklist
docs/user/ordocs/developer/, and updated accordingly. Documentation remains pending while the contract is under review.