Skip to content

Conversation

@haslinghuis
Copy link
Member

@haslinghuis haslinghuis commented Nov 25, 2025

  • initial configuration for capacitor builds
  • probably needs change in vite.config.js

Summary by CodeRabbit

  • Chores
    • CI pipeline now builds, signs and uploads a release Android APK as an additional artifact alongside the web distribution.
  • Bug Fixes
    • Production build asset paths set to a relative base, improving asset resolution when serving files from non-root locations.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 25, 2025

Walkthrough

Adds Capacitor Android build steps to /.github/workflows/deploy.yml: installs Capacitor packages, generates Capacitor config, syncs the Android project, builds and signs a release APK via Narottam04/action-capacitor-android@v1 using keystore secrets, and uploads the signed APK artifact before the existing dist upload. Also sets base: './' in vite.config.js to change production asset base path.

Changes

Cohort / File(s) Summary
Android Capacitor Build Integration
/.github/workflows/deploy.yml
Adds steps after yarn build to install Capacitor packages (@capacitor/core, @capacitor/cli, @capacitor/android), run node capacitor.config.generator.mjs, run npx cap sync android, run Narottam04/action-capacitor-android@v1 to build & sign a release APK using keystore secrets, and upload the signed APK artifact named betaflight-configurator-apk from android/app/build/outputs/apk/release/*.apk prior to the existing dist artifact upload.
Vite config change
vite.config.js
Adds base: './' to the exported Vite configuration to adjust production asset base path resolution.

Sequence Diagram

sequenceDiagram
    participant GH as GitHub Actions
    participant Build as yarn build
    participant Npm as npm/yarn install
    participant Gen as capacitor.config.generator.mjs
    participant Cap as Capacitor CLI (npx cap)
    participant Action as action-capacitor-android@v1
    participant Art as Artifact Storage

    GH->>Build: yarn build
    Build-->>GH: web assets (dist)

    rect rgb(230,245,255)
    Note over GH,Npm: New Android Capacitor flow
    GH->>Npm: install `@capacitor/core`, `@capacitor/cli`, `@capacitor/android`
    Npm-->>GH: deps installed
    GH->>Gen: node capacitor.config.generator.mjs
    Gen-->>GH: capacitor config generated
    GH->>Cap: npx cap sync android
    Cap-->>GH: android/ project prepared
    GH->>Action: run Narottam04/action-capacitor-android@v1 (build & sign using secrets)
    Action-->>GH: signed APK produced
    end

    GH->>Art: upload signed APK artifact (betaflight-configurator-apk)
    Art-->>GH: APK stored
    GH->>Art: upload dist artifact
    Art-->>GH: dist stored
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Verify keystore secret names and that no credentials are hardcoded in the workflow.
  • Confirm APK output path (android/app/build/outputs/apk/release/*.apk) matches produced artifact.
  • Inspect capacitor.config.generator.mjs for CI suitability and required environment assumptions (Android SDK/Gradle).
  • Check that base: './' in vite.config.js doesn't break existing deployment targets.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Description check ⚠️ Warning The description is minimal and incomplete, lacking details about the changes, rationale, and missing required sections from the template (such as issue references or comprehensive explanation). Expand the description to include what was changed and why; add issue references using 'Fixes #' if applicable; provide more context about the Capacitor build configuration.
Title check ❓ Inconclusive The title 'Add capacitor action' is vague and generic, referring only to adding a Capacitor action without specifying what the action does or its purpose. Make the title more specific and descriptive, e.g., 'Add Android APK build and deployment workflow for Capacitor' to clearly indicate the main changes.
✅ Passed checks (1 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 38cbc0d and df417d4.

📒 Files selected for processing (1)
  • .github/workflows/deploy.yml (1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.8)
.github/workflows/deploy.yml

69-69: the runner of "actions/upload-artifact@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
.github/workflows/deploy.yml (1)

40-42: Remove the redundant npm install step; Capacitor packages are already in package.json.

All three Capacitor packages (@capacitor/core, @capacitor/cli, @capacitor/android) are already declared in package.json and will be installed during the yarn install step (line 37). The inline npm install on lines 40-42 is redundant and introduces unnecessary npm/yarn mixing that can cause lock file conflicts.

Remove this step:

-      - name: Install Capacitor Dependencies
-        run: |
-          npm install @capacitor/core @capacitor/cli @capacitor/android
-
       - name: Sync Web Assets to Android Project
         run: npx cap sync android
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between df417d4 and b7485fc.

📒 Files selected for processing (1)
  • .github/workflows/deploy.yml (1 hunks)
🔇 Additional comments (1)
.github/workflows/deploy.yml (1)

44-56: LGTM on Capacitor setup sequence and Android SDK configuration.

The JDK setup (Java 17, temurin), Android SDK filter (including build-tools;34.0.0), and Capacitor sync steps are well-configured. The npx cap sync android command is correct for updating an existing Capacitor Android project without re-initializing.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b7485fc and 95d726d.

📒 Files selected for processing (1)
  • .github/workflows/deploy.yml (1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.8)
.github/workflows/deploy.yml

44-44: specifying action "action-capacitor-android@v1" in invalid format because owner is missing. available formats are "{owner}/{repo}@{ref}" or "{owner}/{repo}/{path}@{ref}"

(action)

🔇 Additional comments (1)
.github/workflows/deploy.yml (1)

43-55: Setup looks solid after fixing the action reference.

The Capacitor build, signing, and artifact upload steps follow good practices. The action-capacitor-android generates signed APK files for testing, and the APK output path aligns with standard Gradle release build structure. The artifact upload uses @v4 consistently with the rest of the workflow.

@haslinghuis
Copy link
Member Author

@coderabbitai one more question - do we need changes in vite.config.js to how the Android app consumes the PWA

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 25, 2025

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Read the vite.config.js file
echo "=== vite.config.js ==="
cat vite.config.js

echo -e "\n=== capacitor.config.generator.mjs ==="
cat capacitor.config.generator.mjs

Length of output: 6056


🏁 Script executed:

#!/bin/bash
# Check the capacitor base config
echo "=== capacitor.config.base.json ==="
cat capacitor.config.base.json

echo -e "\n=== Checking if there's an explicit outDir in vite config ==="
grep -n "outDir" vite.config.js || echo "No explicit outDir found"

Length of output: 227


@haslinghuis, based on my analysis of your configuration, the webDir alignment is correct (src/dist matches your Vite build output), but you may need to make a few adjustments to vite.config.js for optimal Capacitor Android app behavior:

Recommended Changes:

1. Base Path for Assets (Important)

Add base: './' to ensure asset paths work correctly in the Capacitor native app:

export default defineConfig({
    base: './',  // Add this for Capacitor
    define: {
        __APP_VERSION__: JSON.stringify(pkg.version),
        // ...
    },
    // ... rest of config
});

Without this, Capacitor may have issues resolving assets since they'll use absolute paths from root (/) rather than relative paths.

2. PWA Plugin Consideration (Optional)

Since Capacitor provides its own native app shell, you might want to conditionally disable or adjust the PWA plugin for Capacitor builds. The service worker and PWA manifest are primarily for web deployment:

const isCapacitorBuild = process.env.CAPACITOR_BUILD === 'true';

// In plugins array:
!isCapacitorBuild && VitePWA({
    // ... existing config
})

Then set the env var in your workflow: CAPACITOR_BUILD=true npm run build

3. Multi-page App Entry Points

Your current setup has two HTML entry points (main and receiver_msp). Verify that both pages are needed in the Android app, as Capacitor typically starts with a single entry point.

Current Config Status:

webDir: "src/dist" correctly matches Vite output
⚠️ Missing base: './' for proper asset paths in native app
⚠️ PWA plugin may be redundant in Capacitor context

The base: './' change is the most critical one to ensure your Android app can properly load assets.

@haslinghuis
Copy link
Member Author

@coderabbitai - to verify used https://github.com/betaflight/betaflight-configurator/pull/4706/files#diff-58e6f63d87181b1c6a8cb6e5f1691df04aa32854456efcd52ca71c8541375d26R123 while developing locally - having yarn dev to provide the web part.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 25, 2025

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Fetch details about PR #4706 to understand the referenced change
gh pr view 4706 --json title,body,files --repo betaflight/betaflight-configurator

# Get the specific file changes from that PR
gh pr diff 4706 --repo betaflight/betaflight-configurator | head -200

Length of output: 9543


🏁 Script executed:

#!/bin/bash
# Look for capacitor.config in PR #4706 to see what line 123 refers to
gh pr view 4706 --json files --jq '.files[].path' --repo betaflight/betaflight-configurator | grep -i capacitor

Length of output: 70


🏁 Script executed:

#!/bin/bash
# Get more details about the capacitor config files in the current PR
cat capacitor.config.base.json
cat capacitor.config.generator.mjs 2>/dev/null || echo "File not found"

Length of output: 1956


🏁 Script executed:

#!/bin/bash
# Get the specific vite.config.js changes from PR #4706
gh pr diff 4706 --repo betaflight/betaflight-configurator | grep -A 20 -B 5 "vite.config.js"

Length of output: 1305


🏁 Script executed:

#!/bin/bash
# Check the current vite.config.js to compare
cat vite.config.js | head -150

Length of output: 4141


@haslinghuis, perfect! That clarifies the development workflow. Looking at PR #4706, I see the critical change at line 123 of vite.config.js:

server: {
    port: 8000,
    strictPort: true,
    host: "0.0.0.0", // Listen on all network interfaces for Android device access
},

This enables the standard Capacitor live-reload dev workflow where the Android app loads content from your dev server over the network.

Current Status

For Development (with yarn dev):

  • capacitor.config.generator.mjs --dev correctly generates server config pointing to your Vite dev server
  • ⚠️ Missing in this PR: The host: "0.0.0.0" change from PR Custom android serial plugin #4706 should be included here too, otherwise the dev server won't be accessible from the Android device

For Production Builds:

  • webDir: "src/dist" correctly matches Vite output
  • ⚠️ Still recommend adding base: './' to ensure asset paths work correctly when loading from bundled files (not dev server)

Recommended Changes to vite.config.js

export default defineConfig({
    base: './',  // Important for production APK asset paths
    define: {
        __APP_VERSION__: JSON.stringify(pkg.version),
        // ...
    },
    // ... rest of config
    server: {
        port: 8000,
        strictPort: true,
        host: "0.0.0.0", // From PR #4706 - enables Android dev workflow
    },

The host: "0.0.0.0" is essential for the dev workflow you're using, and base: './' ensures the production APK works correctly with relative asset paths.


🧠 Learnings used
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4379
File: src-tauri/tauri.conf.json:9-9
Timestamp: 2025-10-26T19:24:00.800Z
Learning: In Betaflight Configurator's vite.config.js, the root is set to "./src" and outDir is "dist", which results in build output at src/dist (relative to repository root). Therefore, src-tauri/tauri.conf.json correctly references this as "../src/dist" for the frontendDist path.

Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4601
File: package.json:125-125
Timestamp: 2025-09-10T18:26:10.136Z
Learning: In betaflight-configurator, dependency updates are handled incrementally - Vue dependencies are updated separately from Vite dependencies for better isolation and maintainability.

Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4601
File: package.json:125-125
Timestamp: 2025-09-10T18:26:10.136Z
Learning: The betaflight-configurator project uses Node 20.19.4, which satisfies Vite 7's minimum Node requirement of 20.19+.

Learnt from: blckmn
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-09-01T11:01:38.695Z
Learning: The Betaflight Configurator implements analytics through src/js/Analytics.js with the URL "https://analytics.betaflight.com" at line 68, which serves as a proxy frontend for the actual telemetry provider (currently Telemetry Deck), allowing vendor flexibility without code changes.

Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4532
File: src/js/VirtualFC.js:234-234
Timestamp: 2025-06-27T22:06:49.210Z
Learning: In the betaflight-configurator codebase, the VirtualFC.js AUX_CONFIG array must stay synchronized with the betaflight firmware's msp_box.c definitions to ensure proper auxiliary mode functionality. Changes to mode names should follow the firmware source code even if it potentially breaks backward compatibility with saved presets.

Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4532
File: src/js/VirtualFC.js:234-234
Timestamp: 2025-06-27T22:06:49.210Z
Learning: In the betaflight-configurator codebase, the VirtualFC.js AUX_CONFIG array must stay synchronized with the betaflight firmware's msp_box.c definitions to ensure proper auxiliary mode functionality. Changes to mode names should follow the firmware source code even if it potentially breaks backward compatibility with saved presets.

Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4708
File: src/js/tabs/firmware_flasher.js:0-0
Timestamp: 2025-11-19T17:55:51.714Z
Learning: In the betaflight-configurator project, only current Chrome and Edge-based browsers are officially supported, and users are expected to keep their browsers up-to-date for security reasons. This means ES2023 features like Array.prototype.toSorted() and Object.groupBy() can be safely used without polyfills or transpilation.

Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4597
File: src/js/utils/common.js:95-127
Timestamp: 2025-09-09T20:02:33.475Z
Learning: In the Betaflight Configurator codebase, the Chromium v140 compatibility issue with sortSelect was resolved by rewriting the implementation to use native DOM APIs (Array.from, select.options, select.remove, select.add) instead of jQuery DOM manipulation methods (this.children, this.empty().append). The fix still performs DOM operations but avoids the specific jQuery methods that were causing issues in Chromium v140.

Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4579
File: src/js/data_storage.js:9-9
Timestamp: 2025-08-27T22:07:26.353Z
Learning: In the betaflight-configurator CalVer refactoring, the generateVirtualApiVersions function in src/js/utils/common.js works correctly despite the apparent mathematical issue with bounds logic where compareVersions.minor() returns different ranges for API_VERSION_MAX_SUPPORTED (25.12.0) vs API_VERSION_ACCEPTED (1.44.0).

Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4379
File: src-tauri/Cargo.toml:0-0
Timestamp: 2025-10-24T00:41:16.419Z
Learning: The Betaflight Configurator project uses CalVer (calendar versioning) format for versioning (e.g., "2025.12.0") rather than traditional SemVer, while still using SemVer for internal API versioning.

@haslinghuis
Copy link
Member Author

@coderabbitai full review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 25, 2025

✅ Actions performed

Full review triggered.

Comment on lines +40 to +41
- name: Generate Capacitor Config
run: node capacitor.config.generator.mjs
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai - this was not needed in development. Please clarify

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash

# Check if capacitor.config files are gitignored
echo "=== Checking .gitignore for capacitor.config ===="
if [ -f .gitignore ]; then
  rg -n "capacitor.config" .gitignore || echo "No capacitor.config entries found in .gitignore"
fi

echo -e "\n=== Checking for capacitor.config.generator.mjs ===="
if [ -f capacitor.config.generator.mjs ]; then
  echo "Generator script exists. Content:"
  cat capacitor.config.generator.mjs
else
  echo "Generator script not found"
fi

echo -e "\n=== Checking for existing capacitor.config files ===="
fd -t f "^capacitor\.config\." -d 1

echo -e "\n=== Checking git tracking status of capacitor.config files ===="
git ls-files | grep "capacitor.config" || echo "No capacitor.config files are tracked in git"

Length of output: 2250


@haslinghuis The key difference is that capacitor.config.json is gitignored (line 43 in .gitignore), so it doesn't exist in CI's fresh checkout.

In development:

  • You ran the generator manually once (likely with --dev flag for live reload)
  • The generated capacitor.config.json persisted locally on your machine
  • You didn't need to regenerate it unless your IP changed or you cleaned the directory

In CI:

  • Fresh checkout on every run
  • capacitor.config.json doesn't exist (gitignored)
  • Must be generated before npx cap sync android runs
  • CI runs in standard mode (no --dev flag), which simply copies capacitor.config.base.json to capacitor.config.json without the dev server configuration

The generator script has two modes:

  • --dev: Adds server URL for Vite dev server live reload (development)
  • Standard: Just uses base config as-is (production/CI)

Without this step in CI, the npx cap sync android command would fail because it requires capacitor.config.json to exist.


🧠 Learnings used
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4706
File: src/js/protocols/CapacitorSerial.js:47-47
Timestamp: 2025-11-24T15:07:25.227Z
Learning: In betaflight-configurator's CapacitorSerial protocol (src/js/protocols/CapacitorSerial.js), the fire-and-forget this.loadDevices() call in the constructor is intentional background pre-loading. The actual critical device list loading for UI population is orchestrated by port_handler.js via serial.getDevices("serial"), which properly awaits CapacitorSerial.getDevices() → loadDevices() in a complete async chain. The constructor's async call does not cause race conditions.

@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
1 Security Hotspot

See analysis details on SonarQube Cloud

@github-actions
Copy link
Contributor

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
vite.config.js (1)

86-112: Consider conditionally disabling VitePWA for Capacitor builds.

Based on PR discussion, the VitePWA plugin generates service workers and manifests that aren't needed for native Capacitor apps. The bot recommended conditionally disabling this plugin for Capacitor builds to avoid unnecessary overhead and potential conflicts.

Apply this pattern to conditionally include VitePWA:

     plugins: [
         vue(),
         serveLocalesPlugin(),
         copy({
             targets: [
                 { src: "locales/**/*", dest: "src/dist/locales" },
                 { src: "resources/**/*", dest: "src/dist/resources" },
                 { src: "src/tabs/**/*", dest: "src/dist/tabs" },
                 { src: "src/images/**/*", dest: "src/dist/images" },
                 { src: "src/components/**/*", dest: "src/dist/components" },
             ],
             hook: "writeBundle",
         }),
-        VitePWA({
+        ...(process.env.CAPACITOR_BUILD ? [] : [VitePWA({
             registerType: "prompt",
             workbox: {
                 globPatterns: ["**/*.{js,css,html,ico,png,svg,json,mcm}"],
                 // 5MB
                 maximumFileSizeToCacheInBytes: 5 * 1024 * 1024,
             },
             includeAssets: ["favicon.ico", "apple-touch-icon.png"],
             manifest: {
                 name: pkg.displayName,
                 short_name: pkg.productName,
                 description: pkg.description,
                 theme_color: "#ffffff",
                 icons: [
                     {
                         src: "/images/pwa/pwa-192-192.png",
                         sizes: "192x192",
                         type: "image/png",
                     },
                     {
                         src: "/images/pwa/pwa-512-512.png",
                         sizes: "512x512",
                         type: "image/png",
                     },
                 ],
             },
-        }),
+        })]),
     ],

Then set CAPACITOR_BUILD=true in your Capacitor build workflow.

Based on PR discussion and bot recommendations.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6e242f0 and dc88c63.

📒 Files selected for processing (2)
  • .github/workflows/deploy.yml (1 hunks)
  • vite.config.js (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • .github/workflows/deploy.yml
🔇 Additional comments (2)
vite.config.js (2)

53-53: I'll help you verify and rewrite this review comment. Let me start by examining the repository structure and build configuration.
<function_calls>

#!/bin/bash

Check if package.json exists and examine build scripts

if [ -f "package.json" ]; then
echo "=== Build Scripts ==="
jq '.scripts' package.json 2>/dev/null || cat package.json | grep -A 20 '"scripts"'
else
echo "package.json not found"
fi

List root files to understand project structure

echo -e "\n=== Root files ==="
ls -la | grep -E '.(json|js|config)$'


</function_calls>
<function_calls>

#!/bin/bash

Read vite.config.js to understand full configuration

echo "=== vite.config.js ==="
wc -l vite.config.js
echo ""
cat -n vite.config.js


</function_calls>


61-64: I'm unable to complete the verification due to repository access issues in the sandbox environment. However, based on the context provided:

Unable to verify entry point necessity due to sandbox limitations.

The codebase search cannot be executed to confirm whether both main and receiver_msp entry points are necessary for the Capacitor Android app. The naming convention suggests receiver_msp (likely MultiWii Serial Protocol receiver) is a specific feature module, but verification requires:

  • Confirming receiver_msp.html is actively referenced in the app
  • Checking if it's conditionally loaded or always required
  • Verifying it's not dead code from a previous refactor

This should be manually verified by checking:

  1. Whether receiver_msp.html is imported or navigated to anywhere in the codebase
  2. If the entry point is actually bundled and loaded by the Android app

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant