-
Notifications
You must be signed in to change notification settings - Fork 2
feat: firmware-v7, sdk 1.1.11; #78
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughSwitched firmware field usage from firmware-v6 to firmware-v7 across types, UI, and hardware logic. Updated dependency versions in package.json. Adjusted V3 update path and docs to reference firmware-v7. No public API signature changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as V3UpdateTable (UI)
participant HW as Hardware Index
participant RM as releaseMap
User->>UI: Open V3 firmware update
UI->>RM: Get field info for firmware-v7
alt v7 exists
RM-->>UI: Bootloader & Firmware entries
UI-->>User: Display v7 options
else v7 missing
RM-->>UI: No entries
UI-->>User: Show limited options (BLE only)
end
User->>HW: Start remote V3 update (pro)
HW->>RM: Select release info using firmware-v7
RM-->>HW: v7 release data
HW-->>User: Proceed with update
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
🎉 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) |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 (6)
src/components/Firmware/V3UpdateTable.tsx (2)
136-144: Wrong source when toggling in v3-local tabYou always pass "remote". In v3-local, selections won’t apply and updates will noop.
Apply:
+ const currentTab = useSelector((state: RootState) => state.runtime.currentTab); + const currentSource = currentTab === 'v3-local' ? 'local' : 'remote'; ... - checked={isSelected && selection.source === 'remote'} + checked={isSelected && selection.source === currentSource} onChange={() => { toggleComponentSelection( component.type, - 'remote', + currentSource, component.version ); }}
155-166: Unsanitized HTML from marked → XSS riskYou render remote changelogs via dangerouslySetInnerHTML without sanitizing. Use DOMPurify (or similar) before injecting.
Example:
+import DOMPurify from 'dompurify'; ... - dangerouslySetInnerHTML={{ - __html: marked.parse(component.changelog), - }} + dangerouslySetInnerHTML={{ + __html: DOMPurify.sanitize(marked.parse(component.changelog)), + }}Also applies to: 195-207
src/hardware/index.ts (4)
569-573: Loop control bug: break should be continueBreak aborts processing remaining components. You likely want to skip only this one.
Apply:
- // 如果没有版本信息且不是资源组件,跳过此组件 - break; + // 如果没有版本信息且不是资源组件,跳过此组件 + continue;
581-586: Honor the selected version, not always index 0If the server ever exposes multiple entries, this ignores user choice.
Apply:
- if (fwReleaseInfo && fwReleaseInfo.length > 0) { - updateParams.firmwareVersion = fwReleaseInfo[0].version; - } + if (fwReleaseInfo && fwReleaseInfo.length > 0) { + const picked = + selection?.version + ? fwReleaseInfo.find( + x => x.version.join('.') === selection.version + ) || fwReleaseInfo[0] + : fwReleaseInfo[0]; + updateParams.firmwareVersion = picked.version; + }
589-593: Same for BLE: pick the selected versionMirror the firmware logic for BLE.
Apply:
- if (bleReleaseInfo && bleReleaseInfo.length > 0) { - updateParams.bleVersion = bleReleaseInfo[0].version; - } + if (bleReleaseInfo && bleReleaseInfo.length > 0) { + const picked = + selection?.version + ? bleReleaseInfo.find( + x => x.version.join('.') === selection.version + ) || bleReleaseInfo[0] + : bleReleaseInfo[0]; + updateParams.bleVersion = picked.version; + }
601-607: Bootloader: also honor the selected entryBe consistent with version picking.
Apply:
- if ( - fwReleaseInfo && - fwReleaseInfo.length > 0 && - fwReleaseInfo[0].bootloaderVersion - ) { - updateParams.bootloaderVersion = - fwReleaseInfo[0].bootloaderVersion; - } + if (fwReleaseInfo && fwReleaseInfo.length > 0) { + const picked = + selection?.version + ? fwReleaseInfo.find( + x => x.version.join('.') === selection.version + ) || fwReleaseInfo[0] + : fwReleaseInfo[0]; + if (picked.bootloaderVersion) { + updateParams.bootloaderVersion = picked.bootloaderVersion; + } + }
📜 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
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (4)
package.json(1 hunks)src/components/Firmware/V3UpdateTable.tsx(1 hunks)src/hardware/index.ts(2 hunks)src/types.ts(1 hunks)
🔇 Additional comments (2)
package.json (1)
12-14: Lockfile updated—no old SDK versions remain. Commit yarn.lock and smoke-test all flows using firmwareUpdateV3 to confirm compatibility.src/hardware/index.ts (1)
509-514: Doc matches behavior (v7).Comment aligns with the new v7 flow. No action needed.
| const fieldInfo = releaseMap[deviceType]?.['firmware-v7']?.[0]; | ||
| const bleFieldInfo = releaseMap[deviceType]?.ble?.[0]; | ||
| // Bootloader (boot) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Graceful fallback if firmware-v7 is missing
Prefer v7, but fall back to v6 to avoid empty lists during rollout.
Apply:
- const fieldInfo = releaseMap[deviceType]?.['firmware-v7']?.[0];
+ const fieldInfo =
+ releaseMap[deviceType]?.['firmware-v7']?.[0] ??
+ releaseMap[deviceType]?.['firmware-v6']?.[0];📝 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.
| const fieldInfo = releaseMap[deviceType]?.['firmware-v7']?.[0]; | |
| const bleFieldInfo = releaseMap[deviceType]?.ble?.[0]; | |
| // Bootloader (boot) | |
| const fieldInfo = | |
| releaseMap[deviceType]?.['firmware-v7']?.[0] ?? | |
| releaseMap[deviceType]?.['firmware-v6']?.[0]; | |
| const bleFieldInfo = releaseMap[deviceType]?.ble?.[0]; | |
| // Bootloader (boot) |
🤖 Prompt for AI Agents
In src/components/Firmware/V3UpdateTable.tsx around lines 72 to 74, the code
currently reads const fieldInfo = releaseMap[deviceType]?.['firmware-v7']?.[0];
const bleFieldInfo = releaseMap[deviceType]?.ble?.[0]; — update fieldInfo to
prefer firmware-v7 but fall back to firmware-v6 when v7 is missing: check
releaseMap[deviceType]['firmware-v7'] first and if falsy use
releaseMap[deviceType]['firmware-v6'] before taking [0]; keep bleFieldInfo
as-is. Ensure the fallback uses optional chaining to avoid runtime errors.
| const firmwareField = 'firmware-v7'; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add v6 fallback to avoid blocking updates on older configs
If releaseMap lacks v7, the flow should try v6.
Apply:
- const firmwareField = 'firmware-v7';
+ const firmwareField =
+ releaseInfo['firmware-v7'] && releaseInfo['firmware-v7']!.length > 0
+ ? 'firmware-v7'
+ : ('firmware-v6' as keyof typeof releaseInfo);📝 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.
| const firmwareField = 'firmware-v7'; | |
| const firmwareField = | |
| releaseInfo['firmware-v7'] && releaseInfo['firmware-v7']!.length > 0 | |
| ? 'firmware-v7' | |
| : ('firmware-v6' as keyof typeof releaseInfo); |
| 'firmware-v7'?: IFirmwareReleaseInfo[]; | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Keep backward-compat type until all configs publish v7
Add a deprecated v6 field so code can safely fall back while config rolls out.
Apply:
'firmware-v2'?: IFirmwareReleaseInfo[];
- 'firmware-v7'?: IFirmwareReleaseInfo[];
+ // DEPRECATED: remove after all environments ship firmware-v7
+ 'firmware-v6'?: IFirmwareReleaseInfo[];
+ 'firmware-v7'?: IFirmwareReleaseInfo[];To confirm the remote config already exposes v7, run:
🏁 Script executed:
#!/bin/bash
curl -s "https://data.onekey.so/config.json?noCache=$(date +%s)" \
| jq '{pro_v7: .pro["firmware-v7"]|type, pro_v6: .pro["firmware-v6"]|type}'Length of output: 198
Add deprecated firmware-v6 type for backward compatibility
src/types.ts Lines 45–46
- 'firmware-v7'?: IFirmwareReleaseInfo[];
+ // DEPRECATED: remove after all environments ship firmware-v7
+ 'firmware-v6'?: IFirmwareReleaseInfo[];
+ 'firmware-v7'?: IFirmwareReleaseInfo[];📝 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.
| 'firmware-v7'?: IFirmwareReleaseInfo[]; | |
| }; | |
| // DEPRECATED: remove after all environments ship firmware-v7 | |
| 'firmware-v6'?: IFirmwareReleaseInfo[]; | |
| 'firmware-v7'?: IFirmwareReleaseInfo[]; | |
| }; |
🤖 Prompt for AI Agents
In src/types.ts around lines 45 to 46, add a deprecated backward-compatible
optional property 'firmware-v6'?: IFirmwareReleaseInfo[] to the exported type
definition alongside 'firmware-v7' so older consumers still recognize v6
entries; mark it with a comment like "// deprecated: v6 compatibility" if
desired and keep the type identical to IFirmwareReleaseInfo[] to avoid changing
runtime behavior.
Summary by CodeRabbit
New Features
Refactor
Documentation
Chores