docs(features): refresh event firmware spec to shipped behavior - #132
Conversation
|
Warning Review limit reached
Next review available in: 55 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe firmware edition event specification now uses a hosted metadata manifest with offline fallback, defines branding and theme behavior, simplifies notification auto-disable rules, adds a time-zone-aware post-event update nudge, and revises implementation tracking. ChangesEvent firmware client behavior
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 @.github/features/firmware-edition-event-behavior.md:
- Around line 176-180: Clarify the Post-Event Nudge rule so clients prompt only
when the current local calendar date in the event’s IANA timeZone is later than
the date-only eventEnd; do not prompt on eventEnd itself, and retain the
existing behavior for missing or unparseable eventEnd values.
- Line 84: Update the connected-state behavior for MyNodeInfo.firmware_edition
so a transition between different non-vanilla editions replaces the active
branding metadata with the new manifest entry. Preserve activation for mapped
editions and deactivation for vanilla firmware or disconnects, ensuring no stale
artwork, names, or themes remain.
- Around line 36-43: Clarify the “data-driven, not hardcoded in clients”
statement to distinguish hosted metadata from the bundled offline fallback:
clients that can fetch GET /resource/eventFirmware receive new events without a
release, while offline clients continue using the release-bundled manifest and
require a client release to recognize additions. Apply the same clarification to
the corresponding section referenced around lines 184–189.
- Line 74: Update the firmware-edition behavior specification around the
optional fields and device-info behavior to define a non-empty fallback when
displayName is absent, such as the raw edition enum name; preserve the
documented minimal-entry degradation behavior while ensuring firmware labels are
never blank.
- Around line 40-45: Define validation and trust-boundary requirements for the
hosted eventFirmware manifest before clients consume entries: validate the
schema and required fields, sanitize rendered text, restrict links and font/icon
URLs to approved schemes and hosts, and enforce image type and size limits.
Require clients to reject invalid or untrusted entries before display,
deep-linking, font loading, or icon fetching, while preserving the offline
fallback behavior.
🪄 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: 0c7c7112-ce24-48a5-98c4-6324d7af36fc
📒 Files selected for processing (1)
.github/features/firmware-edition-event-behavior.md
… eventEnd rule Addresses CodeRabbit review on #132.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 @.github/features/firmware-edition-event-behavior.md:
- Around line 214-216: Update the fenced code block containing the ended
expression in firmware-edition-event-behavior.md to include an explicit text
language identifier, using a ```text fence while preserving the block’s
contents.
- Around line 49-60: Update the manifest-cache refresh flow to re-evaluate the
currently connected firmware edition whenever cached data changes, not only when
firmware_edition changes. Atomically refresh the associated branding, theme, and
sheet contents so newly fetched events become visible without requiring
reconnection.
- Around line 64-70: Extend the trust-boundary requirements to firmware.zipUrl
in the post-event update flow: require the URL to match an approved firmware
origin, and verify the downloaded artifact’s signature before presenting or
consuming it. Preserve the existing behavior of dropping invalid manifest
entries and treating untrusted firmware as unavailable.
🪄 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: 893e2d9a-c050-455a-81fb-530dec264d42
📒 Files selected for processing (1)
.github/features/firmware-edition-event-behavior.md
…x MD040 Addresses the second CodeRabbit review on #132.
🌟 Why
firmware-edition-event-behavior.mdhasn't been touched since #73 (May 2026). Since then the Android implementation moved substantially past it, and the spec now describes a design that no client actually builds — which makes it actively misleading for the iOS and Web implementations it exists to guide.The largest drift: the spec hardcodes an event registry table in the client, while events are now data-driven off
meshtastic/api. Adding an event is a metadata change, not a client release. Anyone implementing from the current text would build the wrong architecture.🛠️ What changed
Corrected
FAB = 20was missing from the enum.resource/eventFirmware), backing repo, icon hosting, offline fallback, and a commented example of the shape."Welcome to DEFCON! 💀📡") no longer match the manifest and are no longer duplicated here — the manifest is the source.meshtastic/api.welcomeMessage/taglineare explicitly not localized. Moving them into data traded translation for per-event editability; the old text asked implementers to localize them.New sections for behavior that shipped but was never specified
Behavior 3 (notifications) — now describes what the code does: keyed off any non-vanilla edition, including
DIY_EDITION, which the old text said was "not treated as event". The state key is named after Android'snodeEventsAutoDisabledForEvent.Two implementation divergences are called out under Known divergences rather than silently written into the rules — a user who had already turned new-node notifications off gets force-re-enabled on returning to vanilla, and a manual re-enable never clears the flag. Both look like bugs worth fixing rather than behavior to copy, so they're flagged for a deliberate cross-platform decision and added as a sub-task.
Added a Platform Status matrix so the gap between platforms is visible at a glance.
🧪 Verification
Every claim was checked against the Android implementation rather than the PR descriptions:
2.7.26.130-g642aae4) and cross-checked againstmeshtastic/protobufsmaster.MeshConfigFlowManagerImpl.applyEventFirmwareNotificationDefaultsand theNotificationPrefsAPI — this is where both divergences surfaced.data/eventFirmware.jsonandGET resource/eventFirmware.EventBrandingIcon,EventInfoSheet,MainAppBar, andConnectionsScreen.🔗 Related
iconUrl.🤖 Generated with Claude Code
Summary by CodeRabbit