Skip to content

Varius changes, check individual commit for details#271

Merged
424778940z merged 12 commits intoOneKeyHQ:mainfrom
424778940z:main
Mar 18, 2025
Merged

Varius changes, check individual commit for details#271
424778940z merged 12 commits intoOneKeyHQ:mainfrom
424778940z:main

Conversation

@424778940z
Copy link
Copy Markdown
Contributor

@424778940z 424778940z commented Mar 18, 2025

Summary by CodeRabbit

  • New Features

    • Introduced dynamic, formatted text output for display messages.
    • Added new modules to provide enhanced hardware and NFC diagnostics.
    • Extended support for floating-point data in protocol communications.
    • Added a method to check the initialization state of the touch system.
    • Added new constants for power management and error handling in the BLE module.
  • Refactor

    • Standardized bootloader text alignment and error message rendering for a more consistent UI.
    • Unified firmware versioning and updated cryptographic keys to improve security.
    • Simplified state retrieval logic in the touch handling code.
    • Updated command definitions and charging state checks for improved clarity.
    • Enhanced error handling and display output in critical functions.
  • Chores

    • Simplified module initialization by removing deprecated code.
    • Improved touch interface initialization feedback for smoother interactions.
    • Streamlined import statements for better code organization.

Signed-off-by: Adam BZH <adam@onekey.so>
Signed-off-by: Adam BZH <adam@onekey.so>
Signed-off-by: Adam BZH <adam@onekey.so>
Signed-off-by: Adam BZH <adam@onekey.so>
@424778940z 424778940z requested a review from a team as a code owner March 18, 2025 05:26
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 18, 2025

Walkthrough

This pull request updates multiple components across bootloader, firmware, extmod, rust, and Python modules. It standardizes display positioning and version definitions, removes outdated configuration functions, and improves error handling. New modules for hardware info and float support in the protobuf and MicroPython bindings are added. The changes also update cryptographic keys and refine touch initialization. Minor style cleanups and revised debug logging in Python modules are included.

Changes

File(s) Change Summary
core/embed/bootloader/bootui.c, core/embed/bootloader/version.h, core/embed/firmware/bootloader_hashes.py, core/embed/firmware/mpconfigport.h, core/embed/firmware/version.h, core/embed/trezorhal/thd89_boot.c Adjust text positioning; update fix version macros; remove fallback for blake2s; update board/MCU names; simplify state conversion and add version retrieval.
core/embed/extmod/modtrezorconfig/modtrezorconfig.c, core/embed/extmod/modtrezorio/modtrezorio-hwinfo.h, core/embed/extmod/modtrezorio/modtrezorio-motor.h, core/embed/extmod/modtrezorio/modtrezorio-nfc.h, core/embed/extmod/modtrezorio/modtrezorio.c, core/embed/extmod/modtrezorutils/modtrezorutils.c Remove legacy modtrezorconfig functions; add hwinfo module functions; update module metadata (name entries & type renaming); streamline module table and remove model-specific conditionals.
core/embed/extmod/modtrezorui/display.c, core/embed/extmod/modtrezorui/display.h, core/embed/extmod/modtrezorui/fonts/font_plusjakartasans_regular_26.c Remove obsolete display macros; add display_text_printf; introduce new display constants; update glyph advance width.
core/embed/trezorhal/common.c, core/embed/trezorhal/touch.c, core/embed/trezorhal/touch.h Update error functions to use display_text_printf and improve formatting; add touch_inited flag and touch_is_inited function; revise touch initialization.
core/embed/rust/build.rs, core/embed/rust/src/micropython/obj.rs, core/embed/rust/src/protobuf/decode.rs, core/embed/rust/src/protobuf/defs.rs, core/embed/rust/src/protobuf/encode.rs Add float allowlist in bindings; implement TryFrom for f32 and Obj; add support for reading and encoding 32-bit floats; update FieldType with new Float variant and constants.
core/src/apps/base.py, core/src/trezor/protobuf.py, core/src/trezor/wire/__init__.py Update import structure; add print_message for recursive message printing; enhance debug logging with additional message details.
crypto/fuzzer/extract_fuzzer_dictionary.py, python/src/trezorlib/firmware.py Minor code style cleanup; replace old cryptographic keys with new hexadecimal keys in firmware lists.

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Copy Markdown

@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: 2

🧹 Nitpick comments (8)
core/embed/extmod/modtrezorio/modtrezorio-hwinfo.h (1)

27-27: Remove stray semicolon

The function has an extra semicolon after the return statement.

- return mp_obj_new_int_from_uint(get_hw_ver_adc_raw());
- ;
+ return mp_obj_new_int_from_uint(get_hw_ver_adc_raw());
core/src/trezor/protobuf.py (2)

57-57: Unprofessional comment in code.

The comment contains unprofessional language.

-                ):  # can't figure out what exact type due to shitty ffi designs from upstream
+                ):  # can't determine exact type due to FFI design limitations

82-82: Unnecessary pass statement.

The pass statement after print_exception is redundant.

-        pass
core/embed/firmware/bootloader_hashes.py (1)

3-3: Import simplified.

The import has been changed from a try-except fallback to a direct import.

If this code runs on older Python versions where blake2s might not be in hashlib, this could break.

core/embed/trezorhal/touch.h (1)

54-54: Missing void parameter.

The function declaration lacks (void).

-bool touch_is_inited();
+bool touch_is_inited(void);
core/embed/extmod/modtrezorui/display.h (1)

83-83: Typo in macro name.

-#define DISPLAY_CHAR_HIGHT 26
+#define DISPLAY_CHAR_HEIGHT 26
core/embed/trezorhal/common.c (2)

84-86: Typo in comment.

-// print line hight (DISPLAY_CHAR_HIGHT + 2)
+// print line height (DISPLAY_CHAR_HEIGHT + 2)

87-87: Typo in macro usage.

-#define DISP_LINE_TO_Y(LINE) (8 + (DISPLAY_CHAR_HIGHT + 2) * (LINE))
+#define DISP_LINE_TO_Y(LINE) (8 + (DISPLAY_CHAR_HEIGHT + 2) * (LINE))

Also applies to: 111-111, 121-121, 123-123, 125-125, 131-131, 135-135, 139-139, 143-143, 147-147, 163-163

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 14bf8b6 and 689564b.

⛔ Files ignored due to path filters (3)
  • core/mocks/generated/trezorconfig.pyi is excluded by !**/generated/**
  • core/mocks/generated/trezorio/__init__.pyi is excluded by !**/generated/**
  • core/mocks/generated/trezorio/hwinfo.pyi is excluded by !**/generated/**
📒 Files selected for processing (28)
  • core/embed/bootloader/bootui.c (1 hunks)
  • core/embed/bootloader/version.h (1 hunks)
  • core/embed/extmod/modtrezorconfig/modtrezorconfig.c (0 hunks)
  • core/embed/extmod/modtrezorio/modtrezorio-hwinfo.h (1 hunks)
  • core/embed/extmod/modtrezorio/modtrezorio-motor.h (2 hunks)
  • core/embed/extmod/modtrezorio/modtrezorio-nfc.h (2 hunks)
  • core/embed/extmod/modtrezorio/modtrezorio.c (2 hunks)
  • core/embed/extmod/modtrezorui/display.c (1 hunks)
  • core/embed/extmod/modtrezorui/display.h (2 hunks)
  • core/embed/extmod/modtrezorui/fonts/font_plusjakartasans_regular_26.c (1 hunks)
  • core/embed/extmod/modtrezorutils/modtrezorutils.c (1 hunks)
  • core/embed/firmware/bootloader_hashes.py (1 hunks)
  • core/embed/firmware/mpconfigport.h (1 hunks)
  • core/embed/firmware/version.h (1 hunks)
  • core/embed/rust/build.rs (1 hunks)
  • core/embed/rust/src/micropython/obj.rs (2 hunks)
  • core/embed/rust/src/protobuf/decode.rs (4 hunks)
  • core/embed/rust/src/protobuf/defs.rs (3 hunks)
  • core/embed/rust/src/protobuf/encode.rs (1 hunks)
  • core/embed/trezorhal/common.c (6 hunks)
  • core/embed/trezorhal/thd89_boot.c (2 hunks)
  • core/embed/trezorhal/touch.c (1 hunks)
  • core/embed/trezorhal/touch.h (1 hunks)
  • core/src/apps/base.py (1 hunks)
  • core/src/trezor/protobuf.py (1 hunks)
  • core/src/trezor/wire/__init__.py (1 hunks)
  • crypto/fuzzer/extract_fuzzer_dictionary.py (0 hunks)
  • python/src/trezorlib/firmware.py (1 hunks)
💤 Files with no reviewable changes (2)
  • crypto/fuzzer/extract_fuzzer_dictionary.py
  • core/embed/extmod/modtrezorconfig/modtrezorconfig.c
🧰 Additional context used
🧬 Code Definitions (2)
core/embed/trezorhal/common.c (8)
core/embed/trezorhal/common.h (3) (3)
  • __fatal_error (82:83)
  • error_shutdown (85:86)
  • error_reset (88:89)
core/embed/extmod/modtrezorui/display.c (7) (7)
  • display_print_clear (583:586)
  • display_clear (169:171)
  • display_print_color (576:579)
  • display_image (288:323)
  • display_text (844:849)
  • display_text_printf (922:935)
  • display_bar (173:175)
core/embed/trezorhal/touch.h (2) (2)
  • touch_is_inited (54:54)
  • touch_click (61:61)
core/embed/trezorhal/touch.c (3) (3)
  • touch_is_inited (33:33)
  • touch_click (42:53)
  • touch_click (302:313)
core/embed/trezorhal/common.h (3) (3)
  • __fatal_error (82:83)
  • error_shutdown (85:86)
  • error_reset (88:89)
core/embed/extmod/modtrezorui/display.c (7) (7)
  • display_print_clear (583:586)
  • display_clear (169:171)
  • display_print_color (576:579)
  • display_image (288:323)
  • display_text (844:849)
  • display_text_printf (922:935)
  • display_bar (173:175)
core/embed/trezorhal/touch.h (2) (2)
  • touch_is_inited (54:54)
  • touch_click (61:61)
core/embed/trezorhal/touch.c (3) (3)
  • touch_is_inited (33:33)
  • touch_click (42:53)
  • touch_click (302:313)
core/embed/bootloader/bootui.c (6)
core/embed/extmod/modtrezorui/display.c (1) (1)
  • display_text_center (851:857)
core/embed/extmod/modtrezorui/display.h (1) (1)
  • display_text_center (132:133)
core/embed/extmod/modtrezorui/display.c (1) (1)
  • display_text_center (851:857)
core/embed/extmod/modtrezorui/display.h (1) (1)
  • display_text_center (132:133)
core/embed/extmod/modtrezorui/display.c (1) (1)
  • display_text_center (851:857)
core/embed/extmod/modtrezorui/display.h (1) (1)
  • display_text_center (132:133)
⏰ Context from checks skipped due to timeout of 90000ms (3)
  • GitHub Check: Defs check
  • GitHub Check: Style check
  • GitHub Check: Gen check
🔇 Additional comments (34)
core/embed/extmod/modtrezorio/modtrezorio-nfc.h (2)

67-67: Module name added


78-78: Fixed module name

core/embed/extmod/modtrezorio/modtrezorio-motor.h (2)

110-110: Module name added


119-119: Renamed type to module

core/embed/extmod/modtrezorio/modtrezorio-hwinfo.h (1)

1-46: New hardware info module

core/embed/extmod/modtrezorio/modtrezorio.c (3)

48-48: Added hwinfo header.

New hardware info module inclusion. Matches with the new module entry at line 109.


54-54: Fixed header name.

Changed from moto to motor. Better naming.


107-109: Updated module references.

MOTOR changed from type to module. Added NFC and hwinfo modules. Consistent with header changes.

core/src/apps/base.py (1)

5-5: Added protobuf import to support return types.

The protobuf module is now properly imported from trezor package, which is needed for the return type annotations throughout the file.

core/embed/rust/src/protobuf/encode.rs (1)

115-119: Added support for float encoding in protobuf.

The implementation correctly:

  1. Converts the input value to f32
  2. Uses little-endian byte representation
  3. Writes bytes to the output stream
core/embed/rust/build.rs (2)

146-146: Added float creation function to allowlist.

Function is needed for the new float encoding support in the protobuf implementation.


151-151: Added float extraction function to allowlist.

Function is needed for decoding float values from MicroPython objects.

core/src/trezor/wire/__init__.py (2)

342-349: Enhanced debug logging with wire type information.

Added message wire type to debug output for better traceability.


354-356: Added detailed message content debugging.

New debug statement prints the full message structure while omitting None fields.

core/src/trezor/protobuf.py (1)

46-87: Function added for printing message contents.

The function provides useful debugging capabilities for protocol buffer messages.

core/embed/rust/src/protobuf/decode.rs (3)

159-161: Added support for 32-bit floating point values.

The implementation correctly handles the new primitive type.


200-202: Added support for 32-bit floating point values in defaults.

The implementation correctly handles float values in defaults.


327-335: New read_float method implementation.

The implementation looks solid but contains an unwrap() that could panic.

Consider using a safe pattern like:

-        Ok(f32::from_be_bytes(buf[0..4].try_into().unwrap()))
+        let array: [u8; 4] = match buf[0..4].try_into() {
+            Ok(arr) => arr,
+            Err(_) => return Err(error::end_of_buffer()),
+        };
+        Ok(f32::from_be_bytes(array))
core/embed/rust/src/protobuf/defs.rs (4)

51-51: Added Float type handling to get_type method.

The implementation correctly maps type code 7 to the new Float variant.


85-85: Added Float variant to FieldType enum.

The new variant properly extends the type system.


89-91: Added new primitive type constants.

The constants are well-defined for the fixed-width types.


102-102: Updated primitive_type method for Float type.

The implementation correctly maps Float to PRIMITIVE_TYPE_FIXED32BITS.

core/embed/rust/src/micropython/obj.rs (2)

135-148: Added f32 conversion from MicroPython objects.

The implementation correctly handles the conversion using mp_obj_get_float_maybe.


203-209: Added f32 conversion to MicroPython objects.

The implementation correctly handles the conversion using mp_obj_new_float.

core/embed/trezorhal/thd89_boot.c (2)

26-26: Function calls simplified.

Each se0X_get_state function now calls _se_get_state directly without conversion.

Also applies to: 30-30, 34-34, 38-38


67-80: Version retrieval function reintroduced.

The se_get_version_ex function has been added back to retrieve version information.

python/src/trezorlib/firmware.py (1)

46-48:

Details

✅ Verification successful

Cryptographic keys updated.

All three key lists now use identical keys.

Also applies to: 55-57, 64-66


🏁 Script executed:

#!/bin/bash
# Verify if these keys are referenced elsewhere in the codebase
grep -r "57114f0aa669d2f837e040ab9bb51c00991209f84bfd7bf0f893676246fba24a" --include="*.py" .
grep -r "dcae8e37df5c246027c03aa951bd6ec6caa7ad32c166b1f548a4efcd88ca3ca5" --include="*.py" .
grep -r "772912ab61d1dc4f9133325e57e146ab9fac17a4572c6fcdf355f80036100004" --include="*.py" .

Length of output: 1280


Verified key update in firmware.py.

  • The keys on lines 46-48, 55-57, and 64-66 are identical.
core/embed/firmware/mpconfigport.h (1)

216-217: Board and MCU names updated.

Changed hardware labels from TREZORv2/STM3h747xx to OneKeyPro/STM32H7.

core/embed/extmod/modtrezorui/fonts/font_plusjakartasans_regular_26.c (1)

9-9: Space character width increased.

Space advance width changed from 4 to 6 units.

core/embed/bootloader/bootui.c (2)

891-893: Improved code consistency by using TITLE_OFFSET_Y constant.

The code now uses a defined constant instead of a hardcoded value (277), making the vertical positioning consistent with other UI elements.


897-899: Standardized vertical positioning using TITLE_OFFSET_Y.

Replaced magic number with the standard title offset constant, improving consistency.

core/embed/firmware/version.h (1)

6-9:

Details

❓ Verification inconclusive

Version definitions now use existing constants instead of hardcoded values.

This change links fixed version numbers to the current version numbers, allowing for dynamic updates. Note that both FIX_VERSION_MAJOR and FIX_VERSION_MINOR reference VERSION_MINOR.


🏁 Script executed:

#!/bin/bash
# Check if both FIX_VERSION_MAJOR and FIX_VERSION_MINOR referencing VERSION_MINOR is intentional
grep -rn "FIX_VERSION_" --include="*.c" --include="*.h" core/

Length of output: 999


Inconsistent version mapping detected.

  • In core/embed/boardloader_reflash_dev/version.h, FIX_VERSION_MAJOR maps to VERSION_MAJOR.
  • In both core/embed/bootloader/version.h and core/embed/firmware/version.h, FIX_VERSION_MAJOR maps to VERSION_MINOR.

Confirm if this inconsistency is intentional.

core/embed/bootloader/version.h (1)

9-12: Version definitions now reference existing constants instead of hardcoded values.

The change ensures consistency with firmware version.h by using the same pattern of referencing VERSION constants.

core/embed/extmod/modtrezorui/display.c (1)

922-935: Added useful formatted text output function.

The new display_text_printf function provides a convenient way to output formatted text to the display, supporting both simple strings and printf-style formatting.

Signed-off-by: Adam BZH <adam@onekey.so>
Signed-off-by: Adam BZH <adam@onekey.so>
Copy link
Copy Markdown

@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 (5)
core/embed/extmod/modtrezorui/display.c (1)

614-617: Typo in comment. 'hight' => 'height'.

core/embed/trezorhal/common.c (1)

84-84: Spelling error

core/embed/extmod/modtrezorio/modtrezorio-hwinfo.h (1)

15-16: Fix redundant function call.

Calling hw_ver_to_str(hwver) twice wastes CPU cycles.

-  return mp_obj_new_str(hw_ver_to_str(hwver),
-                        MIN(strlen(hw_ver_to_str(hwver)), 32));
+  const char* ver_str = hw_ver_to_str(hwver);
+  return mp_obj_new_str(ver_str, MIN(strlen(ver_str), 32));
core/embed/extmod/modtrezorui/display.h (2)

80-85: Add comments for these macros.

These display constants lack explanatory comments.


140-140: Missing documentation.

Function lacks docstring unlike others in file.

+/// Prints formatted text at given position.
 void display_text_printf(int x, int y, const char *fmt, ...);
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)

📥 Commits

Reviewing files that changed from the base of the PR and between 689564b and 7bd0778.

⛔ Files ignored due to path filters (2)
  • core/mocks/generated/trezorio/__init__.pyi is excluded by !**/generated/**
  • core/mocks/generated/trezorio/hwinfo.pyi is excluded by !**/generated/**
📒 Files selected for processing (13)
  • core/embed/bootloader/bootui.c (1 hunks)
  • core/embed/extmod/modtrezorio/modtrezorio-hwinfo.h (1 hunks)
  • core/embed/extmod/modtrezorio/modtrezorio.c (2 hunks)
  • core/embed/extmod/modtrezorui/display.c (3 hunks)
  • core/embed/extmod/modtrezorui/display.h (2 hunks)
  • core/embed/extmod/modtrezorui/fonts/font_plusjakartasans_regular_26.c (1 hunks)
  • core/embed/trezorhal/common.c (6 hunks)
  • core/embed/trezorhal/thd89_boot.c (2 hunks)
  • core/embed/trezorhal/touch.c (1 hunks)
  • core/embed/trezorhal/touch.h (1 hunks)
  • core/src/apps/base.py (1 hunks)
  • core/src/trezor/protobuf.py (1 hunks)
  • core/src/trezor/wire/__init__.py (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
  • core/embed/trezorhal/touch.h
  • core/src/apps/base.py
  • core/src/trezor/wire/init.py
  • core/src/trezor/protobuf.py
  • core/embed/extmod/modtrezorui/fonts/font_plusjakartasans_regular_26.c
  • core/embed/bootloader/bootui.c
🧰 Additional context used
🧬 Code Definitions (2)
core/embed/trezorhal/touch.c (15)
core/embed/trezorhal/touch.h (2) (2)
  • touch_is_inited (54:54)
  • touch_init (55:55)
core/embed/trezorhal/gt911.c (1) (1)
  • gt911_init (138:144)
core/embed/trezorhal/gt911.h (1) (1)
  • gt911_init (13:13)
core/embed/trezorhal/touch.h (2) (2)
  • touch_is_inited (54:54)
  • touch_init (55:55)
core/embed/trezorhal/gt911.c (1) (1)
  • gt911_init (138:144)
core/embed/trezorhal/gt911.h (1) (1)
  • gt911_init (13:13)
core/embed/trezorhal/touch.h (2) (2)
  • touch_is_inited (54:54)
  • touch_init (55:55)
core/embed/trezorhal/gt911.c (1) (1)
  • gt911_init (138:144)
core/embed/trezorhal/gt911.h (1) (1)
  • gt911_init (13:13)
core/embed/trezorhal/touch.h (2) (2)
  • touch_is_inited (54:54)
  • touch_init (55:55)
core/embed/trezorhal/gt911.c (1) (1)
  • gt911_init (138:144)
core/embed/trezorhal/gt911.h (1) (1)
  • gt911_init (13:13)
core/embed/trezorhal/touch.h (2) (2)
  • touch_is_inited (54:54)
  • touch_init (55:55)
core/embed/trezorhal/gt911.c (1) (1)
  • gt911_init (138:144)
core/embed/trezorhal/gt911.h (1) (1)
  • gt911_init (13:13)
core/embed/trezorhal/common.c (12)
core/embed/trezorhal/common.h (3) (3)
  • __fatal_error (82:83)
  • error_shutdown (85:86)
  • error_reset (88:89)
core/embed/extmod/modtrezorui/display.c (7) (7)
  • display_print_clear (583:586)
  • display_clear (169:171)
  • display_print_color (576:579)
  • display_image (288:323)
  • display_text (844:849)
  • display_text_printf (922:935)
  • display_bar (173:175)
core/embed/trezorhal/touch.h (2) (2)
  • touch_is_inited (54:54)
  • touch_click (61:61)
core/embed/trezorhal/touch.c (3) (3)
  • touch_is_inited (33:33)
  • touch_click (42:53)
  • touch_click (302:313)
core/embed/trezorhal/common.h (3) (3)
  • __fatal_error (82:83)
  • error_shutdown (85:86)
  • error_reset (88:89)
core/embed/extmod/modtrezorui/display.c (7) (7)
  • display_print_clear (583:586)
  • display_clear (169:171)
  • display_print_color (576:579)
  • display_image (288:323)
  • display_text (844:849)
  • display_text_printf (922:935)
  • display_bar (173:175)
core/embed/trezorhal/touch.h (2) (2)
  • touch_is_inited (54:54)
  • touch_click (61:61)
core/embed/trezorhal/touch.c (3) (3)
  • touch_is_inited (33:33)
  • touch_click (42:53)
  • touch_click (302:313)
core/embed/trezorhal/common.h (3) (3)
  • __fatal_error (82:83)
  • error_shutdown (85:86)
  • error_reset (88:89)
core/embed/extmod/modtrezorui/display.c (7) (7)
  • display_print_clear (583:586)
  • display_clear (169:171)
  • display_print_color (576:579)
  • display_image (288:323)
  • display_text (844:849)
  • display_text_printf (922:935)
  • display_bar (173:175)
core/embed/trezorhal/touch.h (2) (2)
  • touch_is_inited (54:54)
  • touch_click (61:61)
core/embed/trezorhal/touch.c (3) (3)
  • touch_is_inited (33:33)
  • touch_click (42:53)
  • touch_click (302:313)
⏰ Context from checks skipped due to timeout of 90000ms (3)
  • GitHub Check: Style check
  • GitHub Check: Defs check
  • GitHub Check: Gen check
🔇 Additional comments (11)
core/embed/trezorhal/thd89_boot.c (5)

26-26: No issues


30-30: No issues


34-34: No issues


38-38: No issues


67-80:

Details

✅ Verification successful

Potential concurrency hazard returning static buffer. Confirm usage.


🏁 Script executed:

#!/bin/bash
rg -A 10 -B 10 'se_get_version_ex'

Length of output: 4675


Static buffer usage verified. Sequential execution confirmed.
The function returns a static buffer that is immediately copied in emmc_commands.c with no evidence of concurrent use.

core/embed/trezorhal/touch.c (2)

32-33: No issues


37-40: We ignore potential errors from gt911_init().

core/embed/extmod/modtrezorui/display.c (2)

663-663: No issues


922-935: No issues

core/embed/extmod/modtrezorio/modtrezorio.c (2)

48-54: LGTM!

The header includes are correct and properly ordered.


107-109: LGTM!

The module entries are properly defined and integrated.

Signed-off-by: Adam BZH <adam@onekey.so>
Signed-off-by: Adam BZH <adam@onekey.so>
Signed-off-by: Adam BZH <adam@onekey.so>
Copy link
Copy Markdown

@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 (4)
core/embed/trezorhal/ble.h (1)

33-57: Unnecessary conditional blocks.

These #if blocks will always evaluate to true since they check for symbols defined earlier in this file.

-#if defined(BLE_CMD_POWER_STA)
 #define BLE_INSERT_POWER 0x01
 #define BLE_REMOVE_POWER 0x02
 #define BLE_CHARGING_PWR 0x03
 #define BLE_CHAGE_OVER 0x04

 #define CHARGE_TYPE_USB 0x01
 #define CHARGE_TYPE_WIRELESS 0x02
-#endif

-#if defined(BLE_CMD_KEY_STA)
 #define BLE_KEY_SHORT_PRESS 0x01
 #define BLE_KEY_LONG_PRESS 0x02
 #define BLE_KEY_PRESS 0x20
 #define BLE_KEY_RELEASE 0x40
-#endif

-#if defined(BLE_CMD_POWER_ERR)
 #define BLE_CMD_POWER_ERR__NONE 0x00
 #define BLE_CMD_POWER_ERR__PMU_OVER_TEMP 0x01
 #define BLE_CMD_POWER_ERR__BATT_OVER_TEMP 0x02
 #define BLE_CMD_POWER_ERR__BATT_UNDER_TEMP 0x03
 #define BLE_CMD_POWER_ERR__BATT_OVER_VOLTAGE 0x04
 #define BLE_CMD_POWER_ERR__CHARGE_TIMEOUT 0x05
-#endif
core/embed/extmod/modtrezorui/display.c (2)

614-615: Typo. "hight" is misspelled.


930-931: Potential truncation. No check.

core/embed/trezorhal/common.c (1)

84-84: Typo. "hight" is misspelled.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)

📥 Commits

Reviewing files that changed from the base of the PR and between 7bd0778 and 6b8dcd6.

📒 Files selected for processing (13)
  • core/embed/bootloader/bootui.c (1 hunks)
  • core/embed/extmod/modtrezorui/display.c (3 hunks)
  • core/embed/extmod/modtrezorui/display.h (2 hunks)
  • core/embed/extmod/modtrezorui/fonts/font_plusjakartasans_regular_26.c (1 hunks)
  • core/embed/trezorhal/ble.c (2 hunks)
  • core/embed/trezorhal/ble.h (3 hunks)
  • core/embed/trezorhal/common.c (6 hunks)
  • core/embed/trezorhal/device.c (1 hunks)
  • core/embed/trezorhal/touch.c (1 hunks)
  • core/embed/trezorhal/touch.h (1 hunks)
  • core/src/apps/base.py (1 hunks)
  • core/src/trezor/protobuf.py (1 hunks)
  • core/src/trezor/wire/__init__.py (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • core/embed/trezorhal/device.c
🚧 Files skipped from review as they are similar to previous changes (7)
  • core/embed/trezorhal/touch.h
  • core/src/apps/base.py
  • core/src/trezor/wire/init.py
  • core/embed/bootloader/bootui.c
  • core/embed/extmod/modtrezorui/display.h
  • core/embed/extmod/modtrezorui/fonts/font_plusjakartasans_regular_26.c
  • core/src/trezor/protobuf.py
🧰 Additional context used
🧬 Code Definitions (1)
core/embed/trezorhal/common.c (4)
core/embed/trezorhal/common.h (3) (3)
  • __fatal_error (82:83)
  • error_shutdown (85:86)
  • error_reset (88:89)
core/embed/extmod/modtrezorui/display.c (7) (7)
  • display_print_clear (583:586)
  • display_clear (169:171)
  • display_print_color (576:579)
  • display_image (288:323)
  • display_text (844:849)
  • display_text_printf (922:935)
  • display_bar (173:175)
core/embed/trezorhal/touch.c (3) (3)
  • touch_is_inited (33:33)
  • touch_click (42:53)
  • touch_click (302:313)
core/embed/trezorhal/touch.h (2) (2)
  • touch_is_inited (54:54)
  • touch_click (61:61)
⏰ Context from checks skipped due to timeout of 90000ms (3)
  • GitHub Check: Gen check
  • GitHub Check: Style check
  • GitHub Check: Defs check
🔇 Additional comments (8)
core/embed/trezorhal/ble.c (5)

10-10: New variable added.

A new variable for tracking power errors has been added.


348-352: Command renamed and constants used.

The command has been renamed from BLE_CMD_PLUG_STA to BLE_CMD_POWER_STA, and magic numbers replaced with named constants.


354-357: Charge type constants used.

Magic numbers replaced with named constants for charge types.


367-368: Command renamed.

Command renamed from BLE_CMD_RPESS to BLE_CMD_KEY_STA.


370-371: Command renamed and using new error variable.

Command renamed from BLE_CMD_PWR to BLE_CMD_POWER_ERR, now using new dev_pwr_err variable.

core/embed/trezorhal/ble.h (2)

10-10: New variable declaration.

Declaration for the new power error tracking variable.


22-25: Command defines renamed.

Command names changed to be more descriptive.

core/embed/trezorhal/touch.c (1)

37-40: We ignore init errors from gt911_init().

Signed-off-by: Adam BZH <adam@onekey.so>
Signed-off-by: Adam BZH <adam@onekey.so>
Signed-off-by: Adam BZH <adam@onekey.so>
Copy link
Copy Markdown

@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 (2)
core/embed/extmod/modtrezorui/display.c (1)

614-617: Minor grammar error
Insert "a" before "ghosting issue."

core/embed/trezorhal/common.c (1)

84-88: Unclear phrasing
"up and down padding 8" is unclear.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)

📥 Commits

Reviewing files that changed from the base of the PR and between 6b8dcd6 and c4b8240.

📒 Files selected for processing (8)
  • core/embed/extmod/modtrezorui/display.c (3 hunks)
  • core/embed/extmod/modtrezorui/display.h (2 hunks)
  • core/embed/trezorhal/ble.c (2 hunks)
  • core/embed/trezorhal/ble.h (3 hunks)
  • core/embed/trezorhal/common.c (6 hunks)
  • core/embed/trezorhal/device.c (1 hunks)
  • core/embed/trezorhal/touch.c (1 hunks)
  • core/embed/trezorhal/touch.h (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
  • core/embed/trezorhal/touch.h
  • core/embed/trezorhal/ble.c
  • core/embed/trezorhal/device.c
  • core/embed/extmod/modtrezorui/display.h
  • core/embed/trezorhal/ble.h
⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: Defs check
  • GitHub Check: Style check
🔇 Additional comments (9)
core/embed/trezorhal/touch.c (2)

32-33: No issues


37-40: Ignoring error from gt911_init()

core/embed/extmod/modtrezorui/display.c (1)

922-935: No issues

core/embed/trezorhal/common.c (6)

90-167: No issues


171-172: No issues


221-227: No issues


236-237: No issues


343-344: No issues


356-360: No issues

@424778940z 424778940z enabled auto-merge (squash) March 18, 2025 06:26
@424778940z 424778940z merged commit 4603749 into OneKeyHQ:main Mar 18, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants