Refactor: Use new protobuf to compatible with different versions of protocols #458
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis update removes the old firmware submodule and adds a new onekey-protocol submodule. It refactors device info extraction, unifies feature handling, and updates protobuf message schemas and types. Several classes now use a new base for firmware updates, and device communication uses new message types and commands. Many type definitions are expanded or renamed for clarity and consistency. Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant Device
participant FirmwareUpdateBaseMethod
participant TransportManager
App->>Device: StartSession({ ok_dev_info_req })
Device->>Device: fixFeaturesBetweenProtocol()
Device->>TransportManager: reconfigure(features)
Device-->>App: features
App->>FirmwareUpdateBaseMethod: enterBootloaderMode()
FirmwareUpdateBaseMethod->>Device: reboot(BootLoader)
Device-->>FirmwareUpdateBaseMethod: Success/Error (logs error, continues)
FirmwareUpdateBaseMethod->>Device: check bootloader status
FirmwareUpdateBaseMethod->>TransportManager: clear device cache
FirmwareUpdateBaseMethod-->>App: Bootloader mode entered
App->>FirmwareUpdateBaseMethod: reboot(OneKeyRebootType)
FirmwareUpdateBaseMethod->>Device: typedCall (command depends on message version)
Device-->>FirmwareUpdateBaseMethod: Success/Error (logs error, continues)
sequenceDiagram
participant App
participant Device
participant onekeyInfoUtils
App->>Device: getFeatures()
Device->>Device: StartSession({ ok_dev_info_req: { factory, normal } })
Device->>Device: fixFeaturesBetweenProtocol()
Device->>onekeyInfoUtils: getHardwareInfoFromFeatures()
Device->>onekeyInfoUtils: getFirmwareInfoFromFeatures()
Device->>onekeyInfoUtils: getSeInfoFromFeatures()
Device-->>App: features (normalized)
sequenceDiagram
participant App
participant DataManager
App->>DataManager: get messages for version
DataManager-->>App: messages (latest, v1, or v2)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
wabicai
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 32
🔭 Outside diff range comments (5)
packages/core/src/types/api/cardano.ts (3)
141-162: 🛠️ Refactor suggestionNaming inconsistency needs attention
While you updated the
formatproperty type, other related names remain inconsistent. The interface is still calledCardanoCVoteRegistrationParametersand usesCardanoCVoteRegistrationDelegation. These should be renamed to match the newGovernanceprefix pattern.-export interface CardanoCVoteRegistrationDelegation { +export interface CardanoGovernanceRegistrationDelegation { votePublicKey: string; weight: number; } -export interface CardanoCVoteRegistrationParameters { +export interface CardanoGovernanceRegistrationParameters { votePublicKey?: string; stakingPath: string | number[]; paymentAddressParameters: CardanoAddressParameters; nonce: string; format?: PROTO.CardanoGovernanceRegistrationFormat; - delegations?: CardanoCVoteRegistrationDelegation[]; + delegations?: CardanoGovernanceRegistrationDelegation[]; votingPurpose?: number; paymentAddress?: string; }
164-167: 🛠️ Refactor suggestionAdditional naming update needed
Update this interface to use the new governance naming pattern.
export interface CardanoAuxiliaryData { hash?: string; - cVoteRegistrationParameters?: CardanoCVoteRegistrationParameters; + governanceRegistrationParameters?: CardanoGovernanceRegistrationParameters; }
202-206: 🛠️ Refactor suggestionRename field to match naming pattern
Update
cVoteRegistrationSignaturetogovernanceRegistrationSignaturefor consistency.export interface CardanoAuxiliaryDataSupplement { type: PROTO.CardanoTxAuxiliaryDataSupplementType; auxiliaryDataHash: string; - cVoteRegistrationSignature?: string; + governanceRegistrationSignature?: string; }packages/core/src/device/Device.ts (1)
360-384: 🧹 Nitpick (assertive)Swap
console.errorfor the project loggerRaw
console.errorcalls pollute stdout and can hurt performance on mobile. UseLog.debug/Log.error(already imported) or remove the statement.-console.error('======> StartSession', options); +Log.debug('StartSession', options);packages/core/src/api/FirmwareUpdateV2.ts (1)
126-166:⚠️ Potential issueUndeclared methods – compile will fail
_promptDeviceInBootloaderForWebDeviceand_checkDeviceInBootloaderModeno longer exist after the refactor toFirmwareUpdateBaseMethod.
Calls here will break the build.- const confirmed = await this._promptDeviceInBootloaderForWebDevice(); + const confirmed = await this.promptDeviceInBootloaderForWebDevice(); ... - await this._checkDeviceInBootloaderMode(connectId, intervalTimer, timeoutTimer); + await this.checkDeviceInBootloaderMode(connectId, intervalTimer, timeoutTimer);Do the same replacement for the call in the
elsebranch below.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (43)
.gitmodules(1 hunks)packages/connect-examples/expo-example/src/components/BaseTestRunner/Context/TestRunnerProvider.tsx(3 hunks)packages/connect-examples/expo-example/src/components/BaseTestRunner/useRunnerTest.ts(0 hunks)packages/connect-examples/expo-example/src/utils/deviceUtils.ts(4 hunks)packages/connect-examples/expo-example/src/views/FirmwareScreen/DeviceField.tsx(3 hunks)packages/connect-examples/expo-example/src/views/FirmwareScreen/ExportDeviceInfo.tsx(6 hunks)packages/connect-examples/expo-example/src/views/FirmwareScreen/index.tsx(2 hunks)packages/core/src/api/FirmwareUpdateV2.ts(7 hunks)packages/core/src/api/FirmwareUpdateV3.ts(3 hunks)packages/core/src/api/GetOnekeyFeatures.ts(2 hunks)packages/core/src/api/device/DeviceRebootToBoardloader.ts(2 hunks)packages/core/src/api/device/DeviceRebootToBootloader.ts(2 hunks)packages/core/src/api/device/DeviceUpdateBootloader.ts(1 hunks)packages/core/src/api/device/DeviceUpdateReboot.ts(1 hunks)packages/core/src/api/firmware/FirmwareUpdateBaseMethod.ts(5 hunks)packages/core/src/api/firmware/bootloaderHelper.ts(2 hunks)packages/core/src/api/firmware/uploadFirmware.ts(2 hunks)packages/core/src/api/test/TestInitializeDeviceDuration.ts(2 hunks)packages/core/src/data-manager/DataManager.ts(3 hunks)packages/core/src/data-manager/MessagesConfig.ts(1 hunks)packages/core/src/data-manager/TransportManager.ts(2 hunks)packages/core/src/data/messages/messages_legacy_v1.json(2 hunks)packages/core/src/device/Device.ts(6 hunks)packages/core/src/device/utils.ts(1 hunks)packages/core/src/types/api/cardano.ts(1 hunks)packages/core/src/types/api/deviceRebootToBoardloader.ts(1 hunks)packages/core/src/types/api/deviceRecovery.ts(2 hunks)packages/core/src/types/device.ts(1 hunks)packages/core/src/utils/deviceFeaturesUtils.ts(3 hunks)packages/core/src/utils/deviceInfoUtils.ts(3 hunks)packages/core/src/utils/deviceVersionUtils.ts(2 hunks)packages/core/src/utils/findDefectiveBatchDevice.ts(1 hunks)packages/core/src/utils/index.ts(1 hunks)packages/core/src/utils/onekeyInfoUtils.ts(1 hunks)packages/hd-transport-http/src/index.ts(2 hunks)packages/hd-transport/__tests__/encode-decode.test.js(1 hunks)packages/hd-transport/__tests__/messages.test.js(3 hunks)packages/hd-transport/scripts/protobuf-build.sh(3 hunks)packages/hd-transport/scripts/protobuf-types.js(1 hunks)packages/hd-transport/src/serialization/protobuf/decode.ts(2 hunks)packages/hd-transport/src/types/messages.ts(75 hunks)submodules/firmware(0 hunks)submodules/onekey-protocol(1 hunks)
💤 Files with no reviewable changes (2)
- packages/connect-examples/expo-example/src/components/BaseTestRunner/useRunnerTest.ts
- submodules/firmware
🧰 Additional context used
🧬 Code Graph Analysis (13)
packages/hd-transport-http/src/index.ts (3)
packages/hd-transport/src/serialization/send.ts (1)
buildOne(13-22)packages/shared/src/HardwareError.ts (1)
HardwareErrorCode(46-402)packages/hd-transport/src/serialization/receive.ts (1)
receiveOne(8-18)
packages/core/src/api/firmware/bootloaderHelper.ts (2)
packages/core/src/utils/deviceVersionUtils.ts (1)
getDeviceBoardloaderVersion(52-58)packages/core/src/utils/index.ts (1)
getDeviceBoardloaderVersion(15-15)
packages/core/src/types/device.ts (1)
packages/hd-transport/src/types/messages.ts (1)
Features(2406-2463)
packages/core/src/api/GetOnekeyFeatures.ts (4)
packages/core/src/utils/deviceFeaturesUtils.ts (1)
getSupportMessageVersion(14-56)packages/core/src/types/device.ts (1)
Features(79-79)packages/hd-transport/src/types/messages.ts (2)
Features(2406-2463)MessageKey(5028-5028)packages/core/src/device/utils.ts (1)
cherryPickFeaturesParams(4-25)
packages/hd-transport/scripts/protobuf-build.sh (2)
packages/hd-transport/__tests__/messages.test.js (1)
json(8-58)packages/hd-transport/scripts/protobuf-types.js (1)
json(9-9)
packages/core/src/utils/findDefectiveBatchDevice.ts (3)
packages/core/src/types/device.ts (1)
Features(79-79)packages/hd-transport/src/types/messages.ts (1)
Features(2406-2463)packages/core/src/utils/onekeyInfoUtils.ts (2)
getHardwareInfoFromFeatures(148-169)getSeInfoFromFeatures(171-280)
packages/core/src/utils/deviceFeaturesUtils.ts (1)
packages/core/src/data-manager/DataManager.ts (1)
DataManager(33-357)
packages/connect-examples/expo-example/src/views/FirmwareScreen/DeviceField.tsx (2)
packages/connect-examples/expo-example/src/views/FirmwareScreen/DeviceFieldContext.ts (1)
useDeviceFieldContext(17-19)packages/connect-examples/expo-example/src/provider/MediaProvider.tsx (1)
useMedia(20-20)
packages/connect-examples/expo-example/src/views/FirmwareScreen/index.tsx (3)
packages/connect-examples/expo-example/src/utils/deviceUtils.ts (1)
getDeviceBasicInfo(50-99)packages/hd-transport/src/types/messages.ts (1)
Features(2406-2463)packages/connect-examples/expo-example/src/views/FirmwareScreen/DeviceField.tsx (1)
DeviceField(74-77)
packages/core/src/device/utils.ts (2)
packages/core/src/device/Device.ts (1)
IFeaturesType(55-58)packages/hd-transport/src/types/messages.ts (2)
OneKeyInfoTargets(3612-3621)OneKeyInfoTypes(3624-3629)
packages/core/src/utils/deviceInfoUtils.ts (3)
packages/core/src/utils/onekeyInfoUtils.ts (2)
getHardwareInfoFromFeatures(148-169)getFirmwareInfoFromFeatures(69-147)packages/core/src/types/device.ts (1)
Features(79-79)packages/hd-transport/src/types/messages.ts (1)
Features(2406-2463)
packages/core/src/api/device/DeviceUpdateReboot.ts (3)
packages/core/src/api/firmware/FirmwareUpdateBaseMethod.ts (1)
FirmwareUpdateBaseMethod(32-451)packages/hd-transport/src/types/messages.ts (1)
DeviceBackToBoot(827-827)packages/core/src/events/ui-request.ts (1)
UI_REQUEST(8-44)
packages/connect-examples/expo-example/src/utils/deviceUtils.ts (3)
packages/core/src/types/device.ts (1)
Features(79-79)packages/hd-transport/src/types/messages.ts (1)
Features(2406-2463)packages/core/src/utils/onekeyInfoUtils.ts (3)
getFirmwareInfoFromFeatures(69-147)getHardwareInfoFromFeatures(148-169)getSeInfoFromFeatures(171-280)
🪛 Biome (1.9.4)
packages/core/src/data-manager/TransportManager.ts
[error] 62-62: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
packages/connect-examples/expo-example/src/views/FirmwareScreen/DeviceField.tsx
[error] 45-45: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
packages/hd-transport/src/types/messages.ts
[error] 143-143: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 245-245: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 788-788: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 824-824: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 827-827: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 830-830: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 887-887: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 1586-1586: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 1589-1589: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 2466-2466: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 2695-2695: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 2714-2714: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 2717-2717: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 2870-2870: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 3119-3119: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 3179-3179: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 4388-4388: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
🪛 Shellcheck (0.10.0)
packages/hd-transport/scripts/protobuf-build.sh
[info] 46-46: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 47-47: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 50-50: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 58-58: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 70-70: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 70-70: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 71-71: Double quote to prevent globbing and word splitting.
(SC2086)
🔇 Additional comments (64)
packages/core/src/types/api/cardano.ts (1)
158-158: Type update looks goodThe change from
PROTO.CardanoCVoteRegistrationFormattoPROTO.CardanoGovernanceRegistrationFormataligns with the broader renaming of Cardano governance types mentioned in the PR summary.submodules/onekey-protocol (1)
1-1: Pin onekey-protocol submodule to correct commit
The subproject file now references commit 8f7a537fc0950866d43da08d4fca76f4635ccdd3, matching the updated.gitmodules.packages/core/src/api/device/DeviceUpdateBootloader.ts (1)
9-9: Imports reduced to only what's needed.Good job simplifying the import statement to include only the necessary
checkBootloaderLengthfunction. This improves code clarity and reduces bundle size.packages/hd-transport/scripts/protobuf-types.js (1)
24-24: Support for protobuf float type added.Adding the
floattonumbermapping completes the protobuf primitive type conversion set. This enables proper handling of floating-point values in protobuf definitions.packages/hd-transport/__tests__/encode-decode.test.js (1)
94-97: Message name updated from "Initialize" to "StartSession".The fixture update aligns with the protocol-level message rename. This change maintains consistency across the codebase.
packages/hd-transport/__tests__/messages.test.js (3)
10-10: Protocol message renamed from "Initialize" to "StartSession"The message schema has been renamed to improve clarity about its purpose - starting a session rather than general initialization.
54-54: MessageType enum value updated to match renamed messageConsistent renaming of the enum value to match the message type name change.
63-63: Test case updated to use new message nameTest case properly updated to use the new message name.
packages/core/src/types/api/deviceRecovery.ts (2)
1-1: Import updated to use the new RecoveryTypeImport correctly updated to use the new type name.
11-11: Type definition updated to match imported typeParameter type properly updated to use RecoveryType.
packages/core/src/utils/index.ts (1)
38-42:✅ Verification successful
New utility functions exported for device info extraction
Good addition of utility functions that centralize device feature extraction.
🏁 Script executed:
#!/bin/bash # Check if the onekeyInfoUtils file exists cat packages/core/src/utils/onekeyInfoUtils.ts | head -20Length of output: 576
🏁 Script executed:
#!/bin/bash set -e file="packages/core/src/utils/onekeyInfoUtils.ts" echo "Searching for utility function definitions in $file" grep -n "getHardwareInfoFromFeatures" $file || echo "getHardwareInfoFromFeatures not found" grep -n "getFirmwareInfoFromFeatures" $file || echo "getFirmwareInfoFromFeatures not found" grep -n "getSeInfoFromFeatures" $file || echo "getSeInfoFromFeatures not found"Length of output: 872
Verification Complete – Exports Match Definitions
All three utilities—getHardwareInfoFromFeatures, getFirmwareInfoFromFeatures, and getSeInfoFromFeatures—are defined in onekeyInfoUtils.ts and correctly re-exported in utils/index.ts. No further action needed.packages/hd-transport-http/src/index.ts (1)
121-121: Protocol update from 'Initialize' to 'StartSession'.The timeout condition correctly reflects the protocol update where 'Initialize' was renamed to 'StartSession'.
packages/core/src/api/test/TestInitializeDeviceDuration.ts (1)
18-18: Protocol update from 'Initialize' to 'StartSession'.The command name update is correct, and the empty object parameter matches the new API signature.
packages/core/src/data-manager/MessagesConfig.ts (1)
13-13: Explicit version labeling improves clarity.Changing from 'latest' to 'v2' makes the version requirements more explicit and less ambiguous.
Also applies to: 17-17
packages/core/src/data/messages/messages_legacy_v1.json (2)
4831-4849: Great rename ofInitializetoStartSession.
You’ve preserved the original numeric ID (0) and deprecated the old field. The structure is concise and clear.
9235-9236:✅ Verification successful
Verify no leftover
Initializereferences.
Ensure all code and tests now useStartSession. Run this script to catch any stray mentions:
🏁 Script executed:
#!/bin/bash # Find any remaining "Initialize" in the repo rg -n '"Initialize\b"'Length of output: 24
No leftover “Initialize” references found
RG returned no matches. All code and tests now useStartSession.packages/core/src/data-manager/DataManager.ts (3)
6-6: Import added for the new v2 legacy messages.This import brings in the new protobuf message definitions needed for the v2 protocol support.
31-31: Type definition updated to include the new v2 message version.The MessageVersion type now correctly includes 'v2' alongside existing options.
68-68: Added v2 message version to the static messages dictionary.Good implementation - follows the existing pattern for message version support.
packages/hd-transport/src/serialization/protobuf/decode.ts (4)
48-49: Improved type safety for enum fields.Added explicit property check before accessing
valuesByIdonfield.resolvedType.
50-51: Enhanced type check for message fields.The code now verifies that
field.resolvedTypeexists and has a 'fields' property before attempting to access it.
59-60: Better type checking for enum resolution.Similar to the repeated fields section, this improves safety when handling enum types.
63-66: Added comprehensive type checks and null handling.This change improves error prevention by:
- Checking for the existence of
resolvedTypeand itsfieldsproperty- Adding explicit handling for null resolved types
Good defensive programming that prevents potential runtime errors.
packages/core/src/api/firmware/uploadFirmware.ts (2)
27-28: Added imports for new feature parameter utilities.These utility functions support the updated device session initialization.
245-247: Replaced race pattern with direct StartSession call.This change:
- Replaces the
Initializecommand with the newStartSessioncommand- Removes the timeout-based race pattern
- Explicitly requests specific features with
cherryPickFeaturesParamsThe code is now cleaner and uses the modern protocol command.
packages/core/src/utils/findDefectiveBatchDevice.ts (3)
2-2: Updated imports to use the new utility functions.Good move to import the centralized feature extraction utilities.
6-7: Refactored to use utility functions instead of direct property access.Now uses
getHardwareInfoFromFeaturesandgetSeInfoFromFeaturesto extract device data. This approach better handles differences between protocol versions.
11-11: Updated condition to use extracted SE version property.Now references
se01Versionfrom the utility function rather than directly accessing feature properties.packages/core/src/api/firmware/bootloaderHelper.ts (1)
4-4: Good move: centralizing version parsingSwitching to
getDeviceBoardloaderVersionandgetDeviceTypekeeps all version/feature parsing logic in one place, which cuts duplication and future‐proofs maintenance.packages/core/src/api/device/DeviceRebootToBootloader.ts (3)
1-2: Import updates align with new class inheritanceThe imports now include
OneKeyRebootTypefor specifying reboot types andFirmwareUpdateBaseMethodas the new parent class.
5-5: Class now extends FirmwareUpdateBaseMethodThe class inheritance change unifies reboot handling with other firmware update methods, improving consistency.
23-23: Standardized reboot method callUsing the base class's
rebootmethod withOneKeyRebootType.BootLoadercentralizes reboot logic and improves maintainability.packages/hd-transport/scripts/protobuf-build.sh (3)
9-10: Updated source path to use onekey-protocolThe source directory correctly points to the new submodule path.
33-42: Added optimization to reuse existing messages.jsonSmart performance improvement that checks if a pre-built messages.json already exists in the submodule.
82-82: Added completion messageHelpful confirmation for users that the build process finished.
packages/core/src/utils/deviceFeaturesUtils.ts (3)
23-28: Added shortcut for new protocol version detectionSmart optimization that returns the latest message version immediately when the device supports the new protocol.
231-232: Enhanced version fallback handlingThe
fixVersionfunction now safely handles undefined inputs by returning a default version.
257-259: Consistent version normalization for onekey_versionSame fix applied to the onekey_version field for complete consistency.
packages/connect-examples/expo-example/src/views/FirmwareScreen/DeviceField.tsx (3)
13-17: Strengthened empty value detectionThe
isNilfunction now checks both null and undefined values, and safely callstrim()only when available.
24-24: Safer type casting for features accessUsing
unknownas an intermediate type offers better type safety when accessing dynamic feature properties.
58-58: Added null fallback for text renderingUsing
fieldValue ?? ''ensures we always render a valid string, preventing rendering issues.packages/connect-examples/expo-example/src/views/FirmwareScreen/ExportDeviceInfo.tsx (8)
2-2: Great addition of explicit type import.Adding the Features type import improves type safety throughout the component.
8-9: Uncommented first array entry properly.Fixed the previously commented-out device_id and label fields in deviceInfoKeys.
28-62: Good expansion of device SE info keys.Adding the type fields (onekey_se0X_type) to each SE device array improves the completeness of exported device information.
82-82: Simplified device context usage.Removing reliance on separate onekeyFeatures object streamlines the code.
84-84: Improved type safety with keyof operator.The getFieldValue function now accepts keys strictly of type keyof Features, preventing potential runtime errors.
96-96: Simplified API call.Updated getDeviceBasicInfo call to only use features parameter, aligning with the unified feature handling approach.
134-135: Added necessary type casting.Proper type casting ensures type safety when accessing dynamic properties from the features object.
147-148: Consistent type casting approach.Maintaining the same type casting pattern for SE info keys as used for device info keys.
packages/core/src/device/utils.ts (2)
1-3: Good imports for feature parameter building.Importing the necessary types for feature parameter cherry-picking.
4-25:✅ Verification successful
Well-structured feature parameter builder.
The cherryPickFeaturesParams function cleanly builds feature request parameters based on the factory flag:
- Always includes basic targets (status, hw, fw, bt) and types (version, specific)
- Conditionally adds advanced targets (SE devices) and types (build_id, hash) when in factory mode
This supports the new protocol's selective feature request approach.
🏁 Script executed:
#!/bin/bash # Check usage of this function to verify proper integration rg -A 3 "cherryPickFeaturesParams" --type tsLength of output: 2614
🏁 Script executed:
#!/bin/bash # Locate the definition of IFeaturesType rg -n "IFeaturesType" --type tsLength of output: 291
🏁 Script executed:
#!/bin/bash # Show full IFeaturesType definition rg -A10 -B2 "export interface IFeaturesType" --type ts packages/core/src/device/Device.tsLength of output: 345
✅ Approve cherryPickFeaturesParams integration
The function correctly builds feature request parameters:
- Always includes basic targets:
status,hw,fw,bt- Always includes basic types:
version,specific- Adds SE targets (
se1–se4) and extra types (build_id,hash) whenfactory = trueWe verified its usage in
Device.ts,GetOnekeyFeatures.ts, anduploadFirmware.ts. No issues found.packages/core/src/utils/deviceInfoUtils.ts (4)
6-6: Good import of centralized utility functions.Importing the helper functions from onekeyInfoUtils centralizes feature extraction logic.
16-17: Improved hardware info extraction.Refactored to use the centralized getHardwareInfoFromFeatures helper instead of direct property access, supporting both legacy and new feature formats.
86-88: Simplified BLE name retrieval.Refactored to use getFirmwareInfoFromFeatures, which handles both old and new property formats consistently.
95-96: Streamlined device UUID extraction.Using the centralized helper function improves maintainability and ensures consistent handling of both old and new serial number formats.
packages/core/src/api/device/DeviceUpdateReboot.ts (4)
1-3: Good imports for reboot functionality.Added OneKeyRebootType enum and switched to FirmwareUpdateBaseMethod for centralized reboot handling.
6-6: Improved class inheritance.Extending FirmwareUpdateBaseMethod gives access to centralized firmware update utilities.
10-10: Better UI control with BOOTLOADER restriction.Adding UI_REQUEST.BOOTLOADER to notAllowDeviceMode prevents using this method when device is already in bootloader mode.
14-14: Simplified reboot implementation.Using the base class's reboot method with OneKeyRebootType.BootLoader parameter:
- Supports different message versions
- Provides better error handling
- Follows the unified device reboot pattern
This is more robust than directly calling device commands.
packages/connect-examples/expo-example/src/views/FirmwareScreen/index.tsx (1)
402-405: Check merge order of feature objectsYou spread
featuresfirst andonekeyFeaturessecond, so any overlapping keys from OneKey will silently override the originalfeatures. This is probably fine, but please double-check that this is the intended precedence – especially for critical flags such asbootloader_mode.packages/core/src/utils/deviceVersionUtils.ts (1)
1-58: Looks goodRefactor to
getFirmwareInfoFromFeaturesremoves duplication and increases consistency.packages/core/src/api/FirmwareUpdateV3.ts (1)
388-395: Ensure every device firmware supportsStartSessionOlder firmware used
Initialize. Swapping toStartSessionunconditionally may break reconnect on legacy devices. Consider a feature-based fallback.await Promise.race([ typedCall(supportsStartSession ? 'StartSession' : 'Initialize', 'Features', {}), // … ]);packages/core/src/device/Device.ts (1)
381-384: Check for undefined before asking for message version
getSupportMessageVersion(this.features)receivesundefinedon the very first call toinitialize, which may break inside that helper. Guard with a null-check or default.Would you confirm that
getSupportMessageVersionhandlesundefinedsafely? If not, wrap the call:const { messageVersion } = getSupportMessageVersion(this.features ?? ({} as any));packages/core/src/utils/onekeyInfoUtils.ts (1)
148-170: Guard empty hardware version
oldHardwareVersionis set to an empty string, then passed intofixVersion.
IffixVersion('')returns"0.0.0"it may overwrite a valid new-protocol version with a bogus default.Consider:
const rawVersion = newHardwareVersion ?? oldHardwareVersion; hardwareVersion: rawVersion ? fixVersion(rawVersion) : undefined,
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
|
@coderabbitai resolve |
✅ Actions performedComments resolved. |
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores
Documentation