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

Unify touch and button handling, enable usage of both in one model #3083

Closed
wants to merge 5 commits into from

Conversation

TychoVrahe
Copy link
Contributor

We will need this in a future model. Only layout for TT is modifed to be able to serve both, with hope that we will unify the layout API soon.

@TychoVrahe TychoVrahe requested a review from matejcik as a code owner June 15, 2023 20:35
@TychoVrahe TychoVrahe self-assigned this Jun 15, 2023
@TychoVrahe TychoVrahe requested a review from prusnak as a code owner June 15, 2023 20:35
@TychoVrahe TychoVrahe marked this pull request as draft June 15, 2023 22:07
@TychoVrahe TychoVrahe force-pushed the tychovrahe/input/unification branch 2 times, most recently from 753b4e8 to 6e6473f Compare June 16, 2023 11:34
@TychoVrahe TychoVrahe force-pushed the tychovrahe/input/unification branch from 6e6473f to 1035c24 Compare June 16, 2023 13:51
@TychoVrahe TychoVrahe marked this pull request as ready for review June 16, 2023 15:02
@TychoVrahe TychoVrahe force-pushed the tychovrahe/input/unification branch from 9a0c1a3 to b0b7817 Compare June 20, 2023 21:37
@matejcik
Copy link
Contributor

why not keep the button and touch interfaces separate and just allow both to be available at the same time?


in general we don't really want to keep the button code active in TT and vice-versa for TR. to facilitate that, we should do to USE_BUTTON and USE_TOUCH the same thing we do to BITCOIN_ONLY, namely:
(a) always use util.USE_XYZ instead of from util import USE_XYZ
(b) in site_tools/micropython/__init__.py, statically replace util.USE_XYZ with True / False as appropriate

@TychoVrahe
Copy link
Contributor Author

why not keep the button and touch interfaces separate and just allow both to be available at the same time?

not sure how to do that actually, could that be done with loop.race? as loop.wait() only accepts a single interface. and if using loop.race is possible for this, i kinda don't like doing the added complexity of polling in parallel

@TychoVrahe
Copy link
Contributor Author

added the replacement: 5925829

@matejcik
Copy link
Contributor

not sure how to do that actually, could that be done with loop.race? as loop.wait() only accepts a single interface. and if using loop.race is possible for this, i kinda don't like doing the added complexity of polling in parallel

loop.race would work, yes, but the way you'd do it is define handle_touch and handle_button and specify both in create_tasks() (conditional on whether you're on touch- or button-enabled model of course)

@TychoVrahe
Copy link
Contributor Author

ok trying it that way here: 72b816a

a bit annoying is needing to add the variants in several layout subclasses (for debug)

But mainly i am realizing here that this PR does not yet handle debug both interfaces properly, will need some more work. I don't like how button debug signal is implemented too: the middle button synthesis in TR layout seems wrong there, shouldn't it be synthesized rather in concrete tests? although it would probably require of sending PRESS and RELEASE separately, or maybe allow sending arbitrary button sequence. I would say that in this layout class we shouldn't care much about concrete buttons and their names (LEFT, RIGHT, etc), just receive a button code and send it to rust for interpretation. Thinking how to make the layout compatible with future models, where we won't have a LEFT and RIGHT button, but for example POWER button. Also not sure what 'swipe' is for in TR/button based model, seems odd.

@Hannsek Hannsek assigned matejcik and unassigned TychoVrahe Jul 20, 2023
@Hannsek
Copy link
Contributor

Hannsek commented Aug 30, 2023

What is the state of this one?

@matejcik
Copy link
Contributor

it's in progress as part of #2299

@matejcik
Copy link
Contributor

closing in favor of #2335

@matejcik matejcik closed this Jan 25, 2024
@matejcik matejcik deleted the tychovrahe/input/unification branch April 8, 2024 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants