Unified firmware: runtime Bluetooth Proxy switch + Stable/Beta channel OTA#111
Unified firmware: runtime Bluetooth Proxy switch + Stable/Beta channel OTA#111bharvey88 wants to merge 2 commits into
Conversation
…-only OTA One firmware image per channel replaces the build-time _BLE split: bluetooth_proxy + esp32_ble_tracker compile into every variant and a "Bluetooth Proxy" switch (default off, persisted) starts/stops scanning at runtime. The Firmware Channel select swaps the OTA manifest URL directly - no ble_firmware selector, no manifest-matching guard, no pre-OTA BLE disable (ESPHome's OTA quiesces BLE itself). - Firmware Update button holds prevent_deep_sleep for the download and re-arms sleep on the not-started path (unless ota_mode/Prevent Sleep is holding the device awake) - http_request consolidated into Core.yaml with the proven buffer sizes (rx 5120 for GitHub's ~3.6KB CSP header line, tx 2048 for the ~850-char signed-redirect query) - apply_ota_source is channel-only; AIR-1_BLE.yaml keeps legacy devices on the firmware-ble manifests via substitution overrides - beta-channel/ wrappers default the select to Beta; build-beta.yml publishes manifest-standard.json + manifest-ble.json to the rolling beta-fw pre-release (absolute URLs) - build.yml now serves the end-user AIR-1.yaml image at firmware/ and moves the improv Factory image to firmware-factory/ (installer page repointed) - version 26.7.12.1 Ported from MSR-1 26.7.9.1 (ApolloAutomation/MSR-1 #100, #103, #104). Supersedes #107 and #110. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe PR adds selectable Stable/Beta OTA channels, BLE proxy controls, beta ESPHome wrappers, and a GitHub Actions workflow that builds and publishes rolling beta firmware assets. The installer now references the factory firmware manifest. ChangesOTA channels and device controls
Beta firmware publishing
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant ESPHomeBuildWorkflow
participant BetaRelease
GitHubActions->>GitHubActions: Extract version from Core.yaml
GitHubActions->>ESPHomeBuildWorkflow: Build AIR-1.yaml and AIR-1_BLE.yaml
ESPHomeBuildWorkflow-->>GitHubActions: Return firmware artifacts
GitHubActions->>BetaRelease: Rewrite manifests and upload beta-fw assets
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Integrations/ESPHome/AIR-1_BLE.yaml (1)
53-53: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winKeep passive BLE scanning explicit here AIR-1_BLE now inherits ESPHome’s default
scan_parameters.active: true, so removing the override switches these legacy devices from passive to active scanning. If that isn’t intentional, restorescan_parameters.active: falseinIntegrations/ESPHome/AIR-1_BLE.yaml.🤖 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 `@Integrations/ESPHome/AIR-1_BLE.yaml` at line 53, Restore the explicit passive BLE scanning configuration for AIR-1_BLE by setting scan_parameters.active to false, preserving legacy devices’ existing passive-scanning behavior.
🧹 Nitpick comments (3)
Integrations/ESPHome/AIR-1_BLE.yaml (1)
1-4: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded manifest URLs duplicate Core.yaml's base substitutions.
Core.yaml defines
stable_manifest_base/beta_manifest_basespecifically so per-variant files only need to append a path. This file instead hardcodes the full URLs, so a future change to the base URLs in Core.yaml won't propagate here.♻️ Reuse the shared base substitutions
substitutions: # Legacy BLE devices keep updating from the firmware-ble manifest. - ota_stable_manifest: "https://apolloautomation.github.io/AIR-1/firmware-ble/manifest.json" - ota_beta_manifest: "https://github.com/ApolloAutomation/AIR-1/releases/download/beta-fw/manifest-ble.json" + ota_stable_manifest: "${stable_manifest_base}/firmware-ble/manifest.json" + ota_beta_manifest: "${beta_manifest_base}/manifest-ble.json"🤖 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 `@Integrations/ESPHome/AIR-1_BLE.yaml` around lines 1 - 4, Update the substitutions in the AIR-1 BLE configuration to reuse Core.yaml’s stable_manifest_base and beta_manifest_base symbols, appending only the BLE manifest paths instead of hardcoding full URLs. Preserve the existing stable and beta manifest targets..github/workflows/build-beta.yml (1)
30-34: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor: version extraction relies on positional text parsing of Core.yaml.
awkhere trusts that the firstversion:line aftersubstitutions:in Core.yaml is the firmware version and thatsubstitutions:appears once. This works today but is fragile to reordering; a proper YAML query (e.g.yq) would be less brittle if this file grows.🤖 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 @.github/workflows/build-beta.yml around lines 30 - 34, Update the version extraction step in the workflow to query the YAML structure of Core.yaml with a YAML-aware tool such as yq, targeting the firmware version field under the intended substitutions mapping. Preserve the existing GITHUB_OUTPUT assignment and Beta version logging while removing the positional awk parsing.Integrations/ESPHome/Core.yaml (1)
497-524: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winFixed delays race the async manifest-check task; confirm timing assumptions hold under slow networks.
apply_ota_source's last step (component.update: update_http_request) triggersHttpRequestUpdate::update(), which spawns a separate FreeRTOS task to fetch/parse the manifest and populateupdate_info_— it returns almost immediately, soscript.wait: apply_ota_source(line 509) provides no real synchronization guarantee for the fetch itself. The subsequentdelay: 5s(line 512) is the only thing standing between "manifest fetch kicked off" andid(update_http_request).perform(true)(line 513) actually being called with the correct firmware URL. On a slow/lossy network this task may not finish in 5s, andperform(true)could act on stale/emptyupdate_info_. This is already acknowledged in the comment ("give it a fixed window to land") but is worth hardening with a bounded poll (e.g.wait_untilonupdate_http_request.is_availableor a short retry loop) instead of a blind delay. The unrelateddelay: 3sat line 507 also has no comment explaining its purpose.♻️ Sketch of a bounded-wait alternative
- script.execute: apply_ota_source - script.wait: apply_ota_source - # The manifest fetch runs in its own task; give it a fixed window to land - # (update.is_available stays false for same-version switches). - - delay: 5s + # Poll briefly instead of blindly waiting a fixed window, so slow + # networks still get a chance to finish the async manifest fetch. + - repeat: + count: 10 + then: + - if: + condition: + lambda: "return id(update_http_request).state != update::UPDATE_STATE_UNKNOWN;" + then: + - script.stop: update_firmware_wait_loop + - delay: 500ms - lambda: id(update_http_request).perform(true);Also applies to: 647-659
🤖 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 `@Integrations/ESPHome/Core.yaml` around lines 497 - 524, Replace the fixed 5-second delay after script.wait in the update_firmware flow with a bounded wait that polls update_http_request.is_available, allowing perform(true) to run only after the manifest is ready while still timing out safely on slow or failed networks. Apply the same synchronization change to the corresponding flow around the second update handler at lines 647-659; leave the unrelated 3-second delay unchanged.
🤖 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.
Outside diff comments:
In `@Integrations/ESPHome/AIR-1_BLE.yaml`:
- Line 53: Restore the explicit passive BLE scanning configuration for AIR-1_BLE
by setting scan_parameters.active to false, preserving legacy devices’ existing
passive-scanning behavior.
---
Nitpick comments:
In @.github/workflows/build-beta.yml:
- Around line 30-34: Update the version extraction step in the workflow to query
the YAML structure of Core.yaml with a YAML-aware tool such as yq, targeting the
firmware version field under the intended substitutions mapping. Preserve the
existing GITHUB_OUTPUT assignment and Beta version logging while removing the
positional awk parsing.
In `@Integrations/ESPHome/AIR-1_BLE.yaml`:
- Around line 1-4: Update the substitutions in the AIR-1 BLE configuration to
reuse Core.yaml’s stable_manifest_base and beta_manifest_base symbols, appending
only the BLE manifest paths instead of hardcoding full URLs. Preserve the
existing stable and beta manifest targets.
In `@Integrations/ESPHome/Core.yaml`:
- Around line 497-524: Replace the fixed 5-second delay after script.wait in the
update_firmware flow with a bounded wait that polls
update_http_request.is_available, allowing perform(true) to run only after the
manifest is ready while still timing out safely on slow or failed networks.
Apply the same synchronization change to the corresponding flow around the
second update handler at lines 647-659; leave the unrelated 3-second delay
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a7667359-26ad-4587-ba7a-1742a58254d2
📒 Files selected for processing (9)
.github/workflows/build-beta.yml.github/workflows/build.ymlIntegrations/ESPHome/AIR-1.yamlIntegrations/ESPHome/AIR-1_BLE.yamlIntegrations/ESPHome/AIR-1_Factory.yamlIntegrations/ESPHome/Core.yamlIntegrations/ESPHome/beta-channel/AIR-1.yamlIntegrations/ESPHome/beta-channel/AIR-1_BLE.yamlstatic/index.html
💤 Files with no reviewable changes (2)
- Integrations/ESPHome/AIR-1_Factory.yaml
- Integrations/ESPHome/AIR-1.yaml
Add an empty encryption: key to the api: block so ESPHome/HA provisions a per-device API key on adoption, matching MSR-1. Bump firmware version to 26.7.14.1. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
1 participant
Version: 26.7.12.1
Adds:
bluetooth_proxy+esp32_ble_trackernow compile into every variant; a "Bluetooth Proxy" switch (default off, persisted) starts/stops scanning at runtime. On this deep-sleep device the proxy is only useful with "Prevent Sleep" on — the switches stay independent.apply_ota_source: a directset_source_urlswap. Noble_firmwareselector, no manifest-matching guard, no pre-OTA BLE disable (ESPHome's OTA quiesces BLE itself).prevent_deep_sleep()for the download and re-arms sleep on the not-started path.http_requestconsolidated into Core.yaml withbuffer_size_rx: 5120/buffer_size_tx: 2048— GitHub release redirects overflow the 512-byte defaults (~3.6 KB CSP header line, ~850-char signed query).beta-channel/wrappers (select defaults to Beta) + build-beta.yml publishingmanifest-standard.json/manifest-ble.jsonto a rollingbeta-fwpre-release (tag deliberately not namedbeta: a tag sharing a branch name shadows it for git fetch and ESPHome remote packages).Fixes:
firmware/currently serves the improv Factory image for OTA; the end-user AIR-1.yaml image takes overfirmware/, Factory moves tofirmware-factory/, andstatic/index.htmlis repointed. install.apolloautomation.com needs the matching repoint when this ships to main (mirrors Point MSR-1 installer at the factory image (restores Wi-Fi setup step) installer#16).Breaks:
AIR-1_BLE.yamlstays for legacy BLE devices via manifest substitution overrides. Two pre-existing_BLEquirks are NOT addressed here (they were only fixed in Add Firmware Channel and Bluetooth Proxy switching from HA #107 and deserve their own small PR): its on_boot sleep duration is computed in hours instead of minutes, and it never publishesapollo_firmware_version.Supersedes #107 and #110.
Checks:
🤖 Generated with Claude Code
Summary by CodeRabbit