Adjust fingerprint sensor sensitivity.#291
Conversation
WalkthroughThis update introduces new functionality for fingerprint sensor configuration and testing. It adds methods to set and get fingerprint sensitivity and area, exposes these controls in the user interface, and makes them available only in non-production environments. A new constant, Changes
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
core/src/trezor/lvglui/scrs/homescreen.py (3)
2760-2787: Hardcoded Chinese text for UI labels.Labels aren't using the internationalization system.
- self.app_drawer_up.set_text("按压阈值:") + self.app_drawer_up.set_text(_(i18n_keys.FINGERPRINT_PRESSURE_THRESHOLD))
2798-2824: Another hardcoded Chinese label.Similar internationalization issue.
- self.app_drawer_up_delay.set_text("面积:") + self.app_drawer_up_delay.set_text(_(i18n_keys.FINGERPRINT_AREA))
2834-2847: Missing error handling in fingerprint parameter setting.No try/except blocks to catch potential errors.
def on_value_changed(self, event_obj): from trezorio import fingerprint target = event_obj.get_target() if target == self.slider: value = target.get_value() self.sensitivity = value self.percent.set_text(f"{value}") - fingerprint.set_sensitivity_and_area(value, self.area) + try: + fingerprint.set_sensitivity_and_area(value, self.area) + except Exception as e: + print(f"Error setting sensitivity: {e}") elif target == self.slider1: value = target.get_value() self.area = value self.percent1.set_text(f"{value}") - fingerprint.set_sensitivity_and_area(self.sensitivity, value) + try: + fingerprint.set_sensitivity_and_area(self.sensitivity, value) + except Exception as e: + print(f"Error setting area: {e}")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
core/mocks/generated/trezorio/fingerprint.pyiis excluded by!**/generated/**
📒 Files selected for processing (8)
core/embed/boardloader/main.c(1 hunks)core/embed/extmod/modtrezorio/modtrezorio-fingerprint.h(4 hunks)core/embed/extmod/modtrezorutils/modtrezorutils.c(1 hunks)core/embed/fp_sensor_wrapper/fingerprint.c(1 hunks)core/embed/fp_sensor_wrapper/fpsensor_common.c(1 hunks)core/embed/fp_sensor_wrapper/fpsensor_driver.h(1 hunks)core/src/trezor/lvglui/scrs/homescreen.py(3 hunks)core/src/trezor/utils.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
core/embed/fp_sensor_wrapper/fingerprint.c (2)
core/embed/fp_sensor_wrapper/fpsensor_common.c (1)
fpsensor_set_config_param(197-207)core/embed/fp_sensor_wrapper/fpsensor_driver.h (1)
fpsensor_set_config_param(262-262)
core/embed/fp_sensor_wrapper/fpsensor_common.c (1)
core/embed/fp_sensor_wrapper/fpsensor_driver.h (1)
fpsensor_get_config_param(263-263)
core/embed/extmod/modtrezorio/modtrezorio-fingerprint.h (2)
core/embed/fp_sensor_wrapper/fpsensor_common.c (2)
fpsensor_set_config_param(197-207)fpsensor_get_config_param(209-213)core/embed/fp_sensor_wrapper/fpsensor_driver.h (2)
fpsensor_set_config_param(262-262)fpsensor_get_config_param(263-263)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Style check
- GitHub Check: Gen check
🔇 Additional comments (10)
core/embed/fp_sensor_wrapper/fpsensor_driver.h (1)
263-263: New getter function looks good.The function signature correctly uses pointers for output parameters and matches the parameter naming in the setter function.
core/embed/extmod/modtrezorutils/modtrezorutils.c (1)
635-639: PRODUCTION flag implementation is correct.This follows the same pattern as other conditional flags (EMULATOR, BITCOIN_ONLY) and properly exposes the build configuration to Python code.
core/src/trezor/utils.py (1)
22-22: Import addition is appropriate.Adding PRODUCTION to the imports makes the flag accessible throughout the Python codebase.
core/embed/fp_sensor_wrapper/fingerprint.c (1)
76-76:Details
✅ Verification successful
Default finger area parameter adjustment looks reasonable.
Changing the finger area parameter from 10 to 8 aligns with the new fingerprint sensor configuration capabilities being added.
🏁 Script executed:
#!/bin/bash # Check if this parameter is used elsewhere in the codebase rg --type c "fpsensor_set_config_param\([^,]+,[^)]+\)" -A 1 -B 1Length of output: 1588
Approved: Finger area change verified.
The update from 10 to 8 is consistent across the codebase.core/embed/boardloader/main.c (1)
267-267:Details
❓ Verification inconclusive
Verify the added parameter's impact on bootloader validation.
The new boolean parameter
trueadded tocheck_image_contents_ADV()affects critical bootloader validation code. Ensure this change is thoroughly tested.
🏁 Script executed:
#!/bin/bash # Check for other usages of check_image_contents_ADV to ensure consistent parameter usage rg "check_image_contents_ADV" --type cLength of output: 748
Validate new parameter in bootloader tests.
- The parameter is consistently used.
- Confirm bootloader tests fully cover this update.
core/src/trezor/lvglui/scrs/homescreen.py (3)
1555-1559: Conditionally adds Fingerprint Test option to settings menu.The feature is properly gated behind a production flag.
1597-1598: Properly handles event routing to new test screen.
2731-2748: New class to test fingerprint sensor parameters.Class structure looks correct and imports the needed fingerprint module.
core/embed/extmod/modtrezorio/modtrezorio-fingerprint.h (2)
21-21: Good include added for driver
335-338: Registration looks good
Summary by CodeRabbit
New Features
Improvements
Bug Fixes