fix: deviceUpdateBootloader suppoort use emmcFileWrite #580
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughRemoves the emmcFileWrite API and its types, refactors DeviceUpdateBootloader to use FirmwareUpdateBaseMethod with an EMMC update path, relaxes reboot error handling, updates SPA redirect restoration and 404 target, tweaks UI/i18n and firmware presets, changes default connect src, and bumps many package versions. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App
participant DeviceUpdate as DeviceUpdateBootloader
participant Base as FirmwareUpdateBaseMethod
participant Device
App->>DeviceUpdate: deviceUpdateBootloader({ binary? })
DeviceUpdate->>DeviceUpdate: if no binary -> fetch resource
DeviceUpdate->>Device: get features
alt features.bootloader_mode == true
DeviceUpdate->>App: Tip(StartTransferData)
DeviceUpdate->>Device: emmcCommonUpdateProcess(binary)
DeviceUpdate->>App: Tip(ConfirmOnDevice)
DeviceUpdate->>Base: reboot(normal)
Note right of Base: reboot errors from disconnect may be caught and treated as success
DeviceUpdate-->>App: UpdateBootloaderSuccess
else
DeviceUpdate->>Device: updateBootloader(binary) (non-emmc path)
DeviceUpdate->>Base: reboot(normal)
DeviceUpdate-->>App: UpdateBootloaderSuccess
end
sequenceDiagram
autonumber
participant Browser
participant Entry as entry.client.tsx
Browser->>Entry: page load
Entry->>Entry: read sessionStorage.spa_redirect_url
alt redirectUrl exists and != current location
Entry->>Browser: history.replaceState(..., redirectUrl)
Entry->>Entry: remove spa_redirect_url
else
Entry-->>Browser: no redirect
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro Cache: Disabled due to data retention organization setting Knowledge base: Disabled due to data retention organization setting Disabled knowledge base sources:
📒 Files selected for processing (1)
Comment |
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) ✅ license/snyk check is complete. No issues have been found. (View Details) |
wabicai
requested review from
originalix and
ByteZhang1024
and removed request for
originalix
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/core/src/api/device/DeviceUpdateBootloader.ts (2)
64-79: Make routing robust when bootloader_mode is undefinedIf
features.bootloader_modeis undefined, the function falls through without returning. Default to the normal path.Apply:
- // Check if device is in bootloader mode - if (features && features.bootloader_mode) { + // Route by mode (default to normal path) + if (features?.bootloader_mode) { // Use emmcFileWrite + reboot logic for bootloader mode this.postTipMessage(FirmwareUpdateTipMessage.UpdateBootloader); - return this.updateBootloaderWithEmmcFileWrite(device, binary); + return this.updateBootloaderWithEmmcFileWrite(binary); } - if (features && !features.bootloader_mode) { + { // Use original updateBootloader logic for normal mode await updateBootloader( this.device.getCommands().typedCall.bind(this.device.getCommands()), this.postMessage, device, binary ); - return Promise.resolve(true); + return true; }
78-79: Nit: avoid Promise.resolve for static valuesReturn the boolean directly. It’s clearer.
Apply:
- return Promise.resolve(true); + return true;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (31)
.github/templates/404.html(1 hunks)packages/connect-examples/electron-example/package.json(2 hunks)packages/connect-examples/expo-example/package.json(2 hunks)packages/connect-examples/expo-playground/app/components/common/ParameterInput.tsx(1 hunks)packages/connect-examples/expo-playground/app/components/global/WebUsbAuthorizeDialog.tsx(4 hunks)packages/connect-examples/expo-playground/app/data/methods/firmware.ts(1 hunks)packages/connect-examples/expo-playground/app/entry.client.tsx(1 hunks)packages/connect-examples/expo-playground/app/i18n/locales/en.ts(2 hunks)packages/connect-examples/expo-playground/package.json(2 hunks)packages/connect-examples/expo-playground/public/_redirects(1 hunks)packages/connect-examples/shared-constants/constants.js(1 hunks)packages/core/package.json(2 hunks)packages/core/src/api/EmmcFileWrite.ts(0 hunks)packages/core/src/api/device/DeviceUpdateBootloader.ts(1 hunks)packages/core/src/api/firmware/FirmwareUpdateBaseMethod.ts(1 hunks)packages/core/src/api/index.ts(0 hunks)packages/core/src/inject.ts(0 hunks)packages/core/src/types/api/emmcFileWrite.ts(0 hunks)packages/core/src/types/api/export.ts(0 hunks)packages/core/src/types/api/index.ts(0 hunks)packages/hd-ble-sdk/package.json(2 hunks)packages/hd-common-connect-sdk/package.json(2 hunks)packages/hd-transport-electron/package.json(2 hunks)packages/hd-transport-emulator/package.json(2 hunks)packages/hd-transport-http/package.json(2 hunks)packages/hd-transport-lowlevel/package.json(2 hunks)packages/hd-transport-react-native/package.json(2 hunks)packages/hd-transport-web-device/package.json(2 hunks)packages/hd-transport/package.json(1 hunks)packages/hd-web-sdk/package.json(2 hunks)packages/shared/package.json(1 hunks)
💤 Files with no reviewable changes (6)
- packages/core/src/api/index.ts
- packages/core/src/types/api/export.ts
- packages/core/src/types/api/index.ts
- packages/core/src/inject.ts
- packages/core/src/types/api/emmcFileWrite.ts
- packages/core/src/api/EmmcFileWrite.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: lint (22)
- GitHub Check: build (22)
🔇 Additional comments (9)
packages/connect-examples/electron-example/package.json (1)
5-26: Version alignment looks good.The example app version and the electron transport dependency stay in lockstep with the alpha release train. Ship it.
packages/hd-transport-http/package.json (1)
3-28: HTTP transport bump synced correctly.The package version and its shared/transport deps all point to 1.1.15-alpha.0, so consumers won’t hit mixed builds.
packages/connect-examples/expo-playground/app/i18n/locales/en.ts (2)
69-69: New common copy is fine.Adding
common.selectmatches existing phrasing and keeps the namespace consistent.
599-612: WebUSB authorize strings read clean.The new firmware namespace mirrors the dialog content and keeps the guidance crisp.
packages/hd-transport-react-native/package.json (1)
3-24: React Native transport stays in sync.Version and internal deps now target 1.1.15-alpha.0, so the BLE stack pulls the right bits.
packages/hd-transport/package.json (1)
3-3: Core transport version bump looks solid.No other manifest shifts, just the alpha tag—exactly what we need for the prerelease.
packages/connect-examples/expo-playground/app/entry.client.tsx (1)
42-56: Nice SPA restore flow.Replacing the history entry before we mount keeps the router in sync with the intended deep link. Clean and effective.
packages/core/src/api/device/DeviceUpdateBootloader.ts (1)
16-16: Good move: unify on FirmwareUpdateBaseMethodExtending FirmwareUpdateBaseMethod centralizes EMMC write, progress, and reboot handling. Cleaner surface.
packages/core/src/api/firmware/FirmwareUpdateBaseMethod.ts (1)
412-430: Safe to broaden reboot disconnect handling. Search shows only one call inDeviceUpdateBootloader.ts, and it ignoresreboot’s return value—merging the proposed diff won’t break existing usage.
packages/connect-examples/expo-playground/app/components/common/ParameterInput.tsx
Show resolved
Hide resolved
Summary by CodeRabbit
New Features
Improvements
Chores