From b1d664498d093546096e073dd242af19d6b97927 Mon Sep 17 00:00:00 2001 From: tychovrahe Date: Wed, 12 Mar 2025 02:23:33 +0100 Subject: [PATCH] button events refactor --- core/embed/io/button/inc/io/button.h | 21 ++++++----- core/embed/io/button/stm32/button.c | 30 +++++++++++----- .../projects/prodtest/cmd/prodtest_button.c | 13 ++++--- core/embed/rust/src/trezorhal/button.rs | 35 +++++++++++++++++++ core/embed/rust/src/trezorhal/io.rs | 20 ----------- core/embed/rust/src/trezorhal/mod.rs | 6 +++- core/embed/rust/src/trezorhal/touch.rs | 5 +++ core/embed/rust/src/ui/event/button.rs | 17 ++++----- core/embed/rust/src/ui/layout/obj.rs | 19 +++++++--- core/embed/rust/src/ui/layout/simplified.rs | 15 ++++---- .../sys/syscall/stm32/syscall_dispatch.c | 3 +- core/embed/sys/syscall/stm32/syscall_stubs.c | 4 +-- .../sys/syscall/stm32/syscall_verifiers.c | 22 ++++++++++++ .../sys/syscall/stm32/syscall_verifiers.h | 9 +++++ .../upymod/modtrezorio/modtrezorio-poll.h | 9 ++--- core/embed/upymod/modtrezorio/modtrezorio.c | 6 ++-- 16 files changed, 156 insertions(+), 78 deletions(-) create mode 100644 core/embed/rust/src/trezorhal/button.rs delete mode 100644 core/embed/rust/src/trezorhal/io.rs create mode 100644 core/embed/rust/src/trezorhal/touch.rs diff --git a/core/embed/io/button/inc/io/button.h b/core/embed/io/button/inc/io/button.h index 14a2259b11..7db72ff656 100644 --- a/core/embed/io/button/inc/io/button.h +++ b/core/embed/io/button/inc/io/button.h @@ -17,8 +17,7 @@ * along with this program. If not, see . */ -#ifndef TREZORHAL_BUTTON_H -#define TREZORHAL_BUTTON_H +#pragma once #include @@ -31,9 +30,10 @@ // // -// Button events -#define BTN_EVT_DOWN (1U << 24) -#define BTN_EVT_UP (1U << 25) +typedef enum { + BTN_EVENT_UP, + BTN_EVENT_DOWN, +} button_event_type_t; // Button identifiers typedef enum { @@ -42,6 +42,11 @@ typedef enum { BTN_POWER = 2, } button_t; +typedef struct { + button_event_type_t event_type; + button_t button; +} button_event_t; + #ifdef KERNEL_MODE // Initializes button driver @@ -59,8 +64,8 @@ void button_deinit(void); // It's expected there's just one consumer of the button events, // e.g. the main loop // -// Returns 0 if no event is available -uint32_t button_get_event(void); +// Returns false if no event is available +bool button_get_event(button_event_t* event); // Checks if the specified button is currently pressed // @@ -68,5 +73,3 @@ uint32_t button_get_event(void); // `button_get_event()` was called. In the future, we may fix this limitation. // For now, `button_get_event()` must be called before `button_is_down()`. bool button_is_down(button_t button); - -#endif // TREZORHAL_BUTTON_H diff --git a/core/embed/io/button/stm32/button.c b/core/embed/io/button/stm32/button.c index f22dec2dd8..a426ae1b9c 100644 --- a/core/embed/io/button/stm32/button.c +++ b/core/embed/io/button/stm32/button.c @@ -110,11 +110,13 @@ void button_deinit(void) { #endif } -uint32_t button_get_event(void) { +bool button_get_event(button_event_t *event) { button_driver_t *drv = &g_button_driver; + memset(event, 0, sizeof(*event)); + if (!drv->initialized) { - return 0; + return false; } #ifdef BTN_LEFT_PIN @@ -124,9 +126,13 @@ uint32_t button_get_event(void) { if (drv->left_down != left_down) { drv->left_down = left_down; if (left_down) { - return BTN_EVT_DOWN | BTN_LEFT; + event->button = BTN_LEFT; + event->event_type = BTN_EVENT_DOWN; + return true; } else { - return BTN_EVT_UP | BTN_LEFT; + event->button = BTN_LEFT; + event->event_type = BTN_EVENT_UP; + return true; } } #endif @@ -138,9 +144,13 @@ uint32_t button_get_event(void) { if (drv->right_down != right_down) { drv->right_down = right_down; if (right_down) { - return BTN_EVT_DOWN | BTN_RIGHT; + event->button = BTN_RIGHT; + event->event_type = BTN_EVENT_DOWN; + return true; } else { - return BTN_EVT_UP | BTN_RIGHT; + event->button = BTN_RIGHT; + event->event_type = BTN_EVENT_UP; + return true; } } #endif @@ -152,9 +162,13 @@ uint32_t button_get_event(void) { if (drv->power_down != power_down) { drv->power_down = power_down; if (power_down) { - return BTN_EVT_DOWN | BTN_POWER; + event->button = BTN_POWER; + event->event_type = BTN_EVENT_DOWN; + return true; } else { - return BTN_EVT_UP | BTN_POWER; + event->button = BTN_POWER; + event->event_type = BTN_EVENT_UP; + return true; } } #endif diff --git a/core/embed/projects/prodtest/cmd/prodtest_button.c b/core/embed/projects/prodtest/cmd/prodtest_button.c index 8281df7adf..277c60e1f0 100644 --- a/core/embed/projects/prodtest/cmd/prodtest_button.c +++ b/core/embed/projects/prodtest/cmd/prodtest_button.c @@ -30,7 +30,9 @@ static void test_single_button(cli_t* cli, uint32_t timeout, button_t btn) { cli_trace(cli, "Waiting for the button press..."); - while (button_get_event() != (btn | BTN_EVT_DOWN)) { + button_event_t btn_event = {0}; + while (!button_get_event(&btn_event) || !(btn_event.button == btn) || + !(btn_event.event_type == BTN_EVENT_DOWN)) { if (ticks_expired(expire_time)) { cli_error(cli, CLI_ERROR_TIMEOUT, ""); return; @@ -43,7 +45,8 @@ static void test_single_button(cli_t* cli, uint32_t timeout, button_t btn) { cli_trace(cli, "Waiting for the button release..."); - while (button_get_event() != (btn | BTN_EVT_UP)) { + while (!button_get_event(&btn_event) || !(btn_event.button == btn) || + !(btn_event.event_type == BTN_EVENT_UP)) { if (ticks_expired(expire_time)) { cli_error(cli, CLI_ERROR_TIMEOUT, ""); return; @@ -65,7 +68,8 @@ static void test_button_combination(cli_t* cli, uint32_t timeout, button_t btn1, while (true) { // Event must be read before calling `button_is_down()` - button_get_event(); + button_event_t e = {0}; + button_get_event(&e); if (button_is_down(btn1) && button_is_down(btn2)) { break; @@ -81,7 +85,8 @@ static void test_button_combination(cli_t* cli, uint32_t timeout, button_t btn1, while (true) { // Event must be read before calling `button_is_down()` - button_get_event(); + button_event_t e = {0}; + button_get_event(&e); if (!button_is_down(btn1) && !button_is_down(btn2)) { break; diff --git a/core/embed/rust/src/trezorhal/button.rs b/core/embed/rust/src/trezorhal/button.rs new file mode 100644 index 0000000000..da50587072 --- /dev/null +++ b/core/embed/rust/src/trezorhal/button.rs @@ -0,0 +1,35 @@ +use super::ffi; + +use num_traits::FromPrimitive; + +#[derive(Copy, Clone, PartialEq, Eq, FromPrimitive)] +#[cfg_attr(feature = "debug", derive(ufmt::derive::uDebug))] +pub enum PhysicalButton { + Left = ffi::button_t_BTN_LEFT as _, + Right = ffi::button_t_BTN_RIGHT as _, + Power = ffi::button_t_BTN_POWER as _, +} + +#[derive(Copy, Clone, PartialEq, Eq, FromPrimitive)] +#[cfg_attr(feature = "debug", derive(ufmt::derive::uDebug))] +pub enum PhysicalButtonEvent { + Down = ffi::button_event_type_t_BTN_EVENT_DOWN as _, + Up = ffi::button_event_type_t_BTN_EVENT_UP as _, +} + +pub fn button_get_event() -> Option<(PhysicalButton, PhysicalButtonEvent)> { + unsafe { + let mut e = ffi::button_event_t { + event_type: ffi::button_event_type_t_BTN_EVENT_DOWN, + button: ffi::button_t_BTN_LEFT, + }; + if ffi::button_get_event(&mut e as _) { + Some(( + unwrap!(PhysicalButton::from_u8(e.button)), + unwrap!(PhysicalButtonEvent::from_u8(e.event_type)), + )) + } else { + None + } + } +} diff --git a/core/embed/rust/src/trezorhal/io.rs b/core/embed/rust/src/trezorhal/io.rs deleted file mode 100644 index 4fbaaad161..0000000000 --- a/core/embed/rust/src/trezorhal/io.rs +++ /dev/null @@ -1,20 +0,0 @@ -use super::ffi; - -#[cfg(feature = "touch")] -pub fn io_touch_get_event() -> u32 { - unsafe { ffi::touch_get_event() } -} - -#[cfg(feature = "button")] -pub fn io_button_get_event() -> u32 { - unsafe { ffi::button_get_event() } -} - -#[cfg(feature = "button")] -#[derive(Copy, Clone, PartialEq, Eq, FromPrimitive)] -#[cfg_attr(feature = "debug", derive(ufmt::derive::uDebug))] -pub enum PhysicalButton { - Left = ffi::button_t_BTN_LEFT as _, - Right = ffi::button_t_BTN_RIGHT as _, - Power = ffi::button_t_BTN_POWER as _, -} diff --git a/core/embed/rust/src/trezorhal/mod.rs b/core/embed/rust/src/trezorhal/mod.rs index f3d211ac40..5785d9de36 100644 --- a/core/embed/rust/src/trezorhal/mod.rs +++ b/core/embed/rust/src/trezorhal/mod.rs @@ -12,7 +12,11 @@ mod ffi; #[cfg(feature = "haptic")] pub mod haptic; -pub mod io; +#[cfg(feature = "button")] +pub mod button; + +#[cfg(feature = "touch")] +pub mod touch; #[cfg(feature = "hw_jpeg_decoder")] pub mod jpegdec; diff --git a/core/embed/rust/src/trezorhal/touch.rs b/core/embed/rust/src/trezorhal/touch.rs new file mode 100644 index 0000000000..4d2752836b --- /dev/null +++ b/core/embed/rust/src/trezorhal/touch.rs @@ -0,0 +1,5 @@ +use super::ffi; + +pub fn touch_get_event() -> u32 { + unsafe { ffi::touch_get_event() } +} diff --git a/core/embed/rust/src/ui/event/button.rs b/core/embed/rust/src/ui/event/button.rs index 04d0e32abc..54cf99d114 100644 --- a/core/embed/rust/src/ui/event/button.rs +++ b/core/embed/rust/src/ui/event/button.rs @@ -1,7 +1,7 @@ use crate::error::Error; -use num_traits::FromPrimitive; -pub use crate::trezorhal::io::PhysicalButton; +pub use crate::trezorhal::button::PhysicalButton; +use crate::trezorhal::button::PhysicalButtonEvent; #[derive(Copy, Clone, PartialEq, Eq)] #[cfg_attr(feature = "debug", derive(ufmt::derive::uDebug))] @@ -19,15 +19,10 @@ pub enum ButtonEvent { } impl ButtonEvent { - pub fn new(event: u32, button: u32) -> Result { - let button = PhysicalButton::from_u32(button); - - let button = button.ok_or(Error::OutOfRange)?; - - let result = match event & 0xFF { - 1 => Self::ButtonPressed(button), - 2 => Self::ButtonReleased(button), - _ => return Err(Error::OutOfRange), + pub fn new(event: PhysicalButtonEvent, button: PhysicalButton) -> Result { + let result = match event { + PhysicalButtonEvent::Down => Self::ButtonPressed(button), + PhysicalButtonEvent::Up => Self::ButtonReleased(button), }; Ok(result) } diff --git a/core/embed/rust/src/ui/layout/obj.rs b/core/embed/rust/src/ui/layout/obj.rs index be99651921..d9675356ac 100644 --- a/core/embed/rust/src/ui/layout/obj.rs +++ b/core/embed/rust/src/ui/layout/obj.rs @@ -4,11 +4,12 @@ use core::{ marker::PhantomData, ops::{Deref, DerefMut}, }; +use num_traits::FromPrimitive; #[cfg(feature = "touch")] use crate::ui::{event::TouchEvent, geometry::Direction}; #[cfg(feature = "touch")] -use num_traits::{FromPrimitive, ToPrimitive}; +use num_traits::ToPrimitive; #[cfg(feature = "ble")] use crate::micropython::buffer::get_buffer; @@ -16,8 +17,12 @@ use crate::micropython::buffer::get_buffer; use crate::ui::event::BLEEvent; #[cfg(feature = "button")] -use crate::ui::event::ButtonEvent; +use crate::{ + trezorhal::button::{PhysicalButton, PhysicalButtonEvent}, + ui::event::ButtonEvent, +}; +use super::base::{Layout, LayoutState}; use crate::{ error::Error, maybe_trace::MaybeTrace, @@ -46,8 +51,6 @@ use crate::{ }, }; -use super::base::{Layout, LayoutState}; - impl AttachType { fn to_obj(self) -> Obj { match self { @@ -516,7 +519,13 @@ extern "C" fn ui_layout_button_event(n_args: usize, args: *const Obj) -> Obj { return Err(Error::TypeError); } let this: Gc = args[0].try_into()?; - let event = ButtonEvent::new(args[1].try_into()?, args[2].try_into()?)?; + let event_type_num: u8 = args[1].try_into()?; + let button_num: u8 = args[2].try_into()?; + + let event_type = unwrap!(PhysicalButtonEvent::from_u8(event_type_num)); + let button = unwrap!(PhysicalButton::from_u8(button_num)); + + let event = ButtonEvent::new(event_type, button)?; let msg = this.inner_mut().obj_event(Event::Button(event))?; Ok(msg) }; diff --git a/core/embed/rust/src/ui/layout/simplified.rs b/core/embed/rust/src/ui/layout/simplified.rs index e42a59de33..3ea4b1609a 100644 --- a/core/embed/rust/src/ui/layout/simplified.rs +++ b/core/embed/rust/src/ui/layout/simplified.rs @@ -1,7 +1,7 @@ #[cfg(feature = "button")] -use crate::trezorhal::io::io_button_get_event; +use crate::trezorhal::button::button_get_event; #[cfg(feature = "touch")] -use crate::trezorhal::io::io_touch_get_event; +use crate::trezorhal::touch::touch_get_event; #[cfg(feature = "button")] use crate::ui::event::ButtonEvent; #[cfg(feature = "touch")] @@ -35,13 +35,10 @@ where } #[cfg(feature = "button")] fn button_eval() -> Option { - let event = io_button_get_event(); - if event == 0 { + let event = button_get_event(); + let Some((event_btn, event_type)) = event else { return None; - } - - let event_type = event >> 24; - let event_btn = event & 0xFFFFFF; + }; let event = ButtonEvent::new(event_type, event_btn); @@ -84,7 +81,7 @@ pub fn run(frame: &mut impl Component) -> u32 { #[cfg(all(feature = "button", not(feature = "touch")))] let event = button_eval(); #[cfg(feature = "touch")] - let event = touch_unpack(io_touch_get_event()); + let event = touch_unpack(touch_get_event()); if let Some(e) = event { let mut ctx = EventCtx::new(); #[cfg(all(feature = "button", not(feature = "touch")))] diff --git a/core/embed/sys/syscall/stm32/syscall_dispatch.c b/core/embed/sys/syscall/stm32/syscall_dispatch.c index 0b1fbe7338..1fbed25511 100644 --- a/core/embed/sys/syscall/stm32/syscall_dispatch.c +++ b/core/embed/sys/syscall/stm32/syscall_dispatch.c @@ -420,7 +420,8 @@ __attribute((no_stack_protector)) void syscall_handler(uint32_t *args, #ifdef USE_BUTTON case SYSCALL_BUTTON_GET_EVENT: { - args[0] = button_get_event(); + button_event_t *event = (button_event_t *)args[0]; + args[0] = button_get_event__verified(event); } break; #endif diff --git a/core/embed/sys/syscall/stm32/syscall_stubs.c b/core/embed/sys/syscall/stm32/syscall_stubs.c index 549e17688f..c3c866f035 100644 --- a/core/embed/sys/syscall/stm32/syscall_stubs.c +++ b/core/embed/sys/syscall/stm32/syscall_stubs.c @@ -363,8 +363,8 @@ secbool secret_bootloader_locked(void) { #include -uint32_t button_get_event(void) { - return syscall_invoke0(SYSCALL_BUTTON_GET_EVENT); +bool button_get_event(button_event_t *event) { + return syscall_invoke1(SYSCALL_BUTTON_GET_EVENT, (uint32_t)event); } #endif diff --git a/core/embed/sys/syscall/stm32/syscall_verifiers.c b/core/embed/sys/syscall/stm32/syscall_verifiers.c index 74d0404769..031fc97838 100644 --- a/core/embed/sys/syscall/stm32/syscall_verifiers.c +++ b/core/embed/sys/syscall/stm32/syscall_verifiers.c @@ -861,6 +861,8 @@ access_violation: #endif // USE_HW_JPEG_DECODER +// --------------------------------------------------------------------- + #ifdef USE_DMA2D bool dma2d_rgb565_fill__verified(const gfx_bitblt_t *bb) { @@ -1050,4 +1052,24 @@ access_violation: #endif +// --------------------------------------------------------------------- + +#ifdef USE_BUTTON + +#include + +bool button_get_event__verified(button_event_t *event) { + if (!probe_write_access(event, sizeof(*event))) { + goto access_violation; + } + + return button_get_event(event); + +access_violation: + apptask_access_violation(); + return false; +} + +#endif + #endif // SYSCALL_DISPATCH diff --git a/core/embed/sys/syscall/stm32/syscall_verifiers.h b/core/embed/sys/syscall/stm32/syscall_verifiers.h index 7dba851ebb..c5cf506739 100644 --- a/core/embed/sys/syscall/stm32/syscall_verifiers.h +++ b/core/embed/sys/syscall/stm32/syscall_verifiers.h @@ -255,4 +255,13 @@ bool dma2d_rgba8888_blend_mono8__verified(const gfx_bitblt_t *bb); #endif +// --------------------------------------------------------------------- +#ifdef USE_BUTTON + +#include + +bool button_get_event__verified(button_event_t *event); + +#endif + #endif // SYSCALL_DISPATCH diff --git a/core/embed/upymod/modtrezorio/modtrezorio-poll.h b/core/embed/upymod/modtrezorio/modtrezorio-poll.h index f6e84b7506..6439ee3c11 100644 --- a/core/embed/upymod/modtrezorio/modtrezorio-poll.h +++ b/core/embed/upymod/modtrezorio/modtrezorio-poll.h @@ -155,11 +155,12 @@ STATIC mp_obj_t mod_trezorio_poll(mp_obj_t ifaces, mp_obj_t list_ref, } #if USE_BUTTON else if (iface == BUTTON_IFACE) { - const uint32_t evt = button_get_event(); - if (evt & (BTN_EVT_DOWN | BTN_EVT_UP)) { + button_event_t btn_event = {0}; + const bool btn = button_get_event(&btn_event); + if (btn) { mp_obj_tuple_t *tuple = MP_OBJ_TO_PTR(mp_obj_new_tuple(2, NULL)); - uint32_t etype = (evt >> 24) & 0x3U; // button down/up - uint32_t en = evt & 0xFFFF; // button number + uint32_t etype = btn_event.event_type; + uint32_t en = btn_event.button; if (display_get_orientation() == 180) { en = (en == BTN_LEFT) ? BTN_RIGHT : BTN_LEFT; } diff --git a/core/embed/upymod/modtrezorio/modtrezorio.c b/core/embed/upymod/modtrezorio/modtrezorio.c index a8ddcfbf34..ce765c2057 100644 --- a/core/embed/upymod/modtrezorio/modtrezorio.c +++ b/core/embed/upymod/modtrezorio/modtrezorio.c @@ -111,10 +111,8 @@ STATIC const mp_rom_map_elem_t mp_module_trezorio_globals_table[] = { #endif #ifdef USE_BUTTON {MP_ROM_QSTR(MP_QSTR_BUTTON), MP_ROM_INT(BUTTON_IFACE)}, - {MP_ROM_QSTR(MP_QSTR_BUTTON_PRESSED), - MP_ROM_INT((BTN_EVT_DOWN >> 24) & 0x3U)}, - {MP_ROM_QSTR(MP_QSTR_BUTTON_RELEASED), - MP_ROM_INT((BTN_EVT_UP >> 24) & 0x3U)}, + {MP_ROM_QSTR(MP_QSTR_BUTTON_PRESSED), MP_ROM_INT(BTN_EVENT_DOWN)}, + {MP_ROM_QSTR(MP_QSTR_BUTTON_RELEASED), MP_ROM_INT(BTN_EVENT_UP)}, {MP_ROM_QSTR(MP_QSTR_BUTTON_LEFT), MP_ROM_INT(BTN_LEFT)}, {MP_ROM_QSTR(MP_QSTR_BUTTON_RIGHT), MP_ROM_INT(BTN_RIGHT)}, #endif