Skip to content
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

add brightness setting #3791

Open
wants to merge 3 commits into
base: ui-t3t1
Choose a base branch
from
Open

Conversation

TychoVrahe
Copy link
Contributor

building on #3483, but adding support for mercury

Copy link

github-actions bot commented May 7, 2024

core UI changes device test click test persistence test
T2T1 Model T test(screens) main(screens) test(screens) main(screens) test(screens) main(screens)
T2B1 Safe 3 3280 test(screens) main(screens) 2724
T3T1 test(screens) main(screens) test(screens) main(screens) test(screens) main(screens)
All main(screens)

Copy link

github-actions bot commented May 17, 2024

legacy UI changes device test(screens) main(screens)

@TychoVrahe TychoVrahe requested review from mmilata and removed request for matejcik May 19, 2024 13:02
@Hannsek Hannsek requested a review from obrusvit May 20, 2024 12:03
@TychoVrahe TychoVrahe requested a review from matejcik May 20, 2024 12:10
Copy link
Contributor

@matejcik matejcik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

two high level comments (also mentioned in the review):

(a) perhaps there should be a SetBrightness call as opposed to shoving it into settings? depends on whether we ever want the user to set brightness from host
(if we do, the handling for that is completely missing)

(b) I would avoid exposing to Python. I can take over that part if you like

common/protob/messages-management.proto Outdated Show resolved Hide resolved
core/embed/rust/src/ui/model_mercury/layout.rs Outdated Show resolved Hide resolved
core/embed/rust/src/ui/model_mercury/layout.rs Outdated Show resolved Hide resolved
core/embed/rust/src/ui/model_mercury/theme/backlight.rs Outdated Show resolved Hide resolved
core/embed/rust/src/ui/model_mercury/theme/backlight.rs Outdated Show resolved Hide resolved
core/embed/trezorhal/stm32f4/backlight_pwm.c Show resolved Hide resolved
core/src/storage/device.py Outdated Show resolved Hide resolved
core/src/trezor/ui/style.py Outdated Show resolved Hide resolved
@matejcik
Copy link
Contributor

@TychoVrahe please check my changes
you can ignore 86bc1ad, it is a cherry-pick from main, i needed it for vscode

@matejcik matejcik force-pushed the tychovrahe/ui-t3t1/brightness branch 2 times, most recently from 2acd186 to 9d0deeb Compare May 24, 2024 08:27
let default_macros = DEFAULT_BINDGEN_MACROS_COMMON
.iter()
.chain(DEFAULT_BINDGEN_MACROS_T2T1)
.chain(DEFAULT_BINDGEN_MACROS_T2B1);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we miss also T3T1 specifics?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, this is a cherrypick of cf58fdd from main. we probably should, but this commit should be removed from the PR before merging, and someone should add T3T1 features when merging the UI branch into main (or later, whenever we hit a problem caused by it, so probably when you try to run T3T1 Rust tests)

Cancelled,
}

pub struct NumberInputSliderDialog {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: copy of model_t version. Should be reworked for mercury when design done.
Linking it here for now: #3748

@TychoVrahe TychoVrahe force-pushed the tychovrahe/ui-t3t1/brightness branch from fdeaf19 to d57d281 Compare May 28, 2024 10:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: 🏃‍♀️ In progress
Development

Successfully merging this pull request may close these issues.

None yet

6 participants