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

Tenv link default model #16971

Draft
wants to merge 9 commits into
base: develop
Choose a base branch
from
Draft

Tenv link default model #16971

wants to merge 9 commits into from

Conversation

mroz22
Copy link
Contributor

@mroz22 mroz22 commented Feb 13, 2025

based on #16969

till now, all models had the same firmware version which is not true anymore since this change trezor/trezor-user-env@6339d48#diff-4e726d3141a1307472f5c4ee56a6adde4dbd0664257e4898847074c18faa25dfR60. which results into

websocket_error_message: RuntimeError - Unknown firmware T3T1 2.8.8-arm

so we need to extend our client-side logic that selects a proper default. It would feel better to do this server side - could trezor-user-env start emulator command always pick as default the highest available version which is not main version? cc @grdddj

@mroz22 mroz22 force-pushed the tenv-link-default-model branch from 2808503 to ff5119b Compare February 13, 2025 08:06
@mroz22 mroz22 added the no-project This label is used to specify that PR doesn't need to be added to a project label Feb 13, 2025
@grdddj
Copy link
Contributor

grdddj commented Feb 13, 2025

could trezor-user-env start emulator command always pick as default the highest available version which is not main version? cc @grdddj

I think this is already working with -latest suffix - https://github.com/trezor/trezor-user-env/blob/6339d48f1df4f007fe5caee659e4bca3997d480a/src/controller.py#L178-L179

@mroz22 mroz22 force-pushed the tenv-link-default-model branch from ff5119b to 993c1ae Compare February 13, 2025 13:59
@HajekOndrej HajekOndrej force-pushed the tenv-link-default-model branch 2 times, most recently from de7256f to 10370a3 Compare February 13, 2025 20:37
@mroz22 mroz22 force-pushed the tenv-link-default-model branch 3 times, most recently from 174b7ee to 09b7314 Compare February 17, 2025 07:46
Copy link
Contributor

@Vere-Grey Vere-Grey left a comment

Choose a reason for hiding this comment

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

good catch
my bad, I have accidentally approved the PR, i thought it is just PR for test removal

@Vere-Grey Vere-Grey self-requested a review February 17, 2025 08:06
@mroz22 mroz22 force-pushed the tenv-link-default-model branch from 09b7314 to e4432ec Compare February 17, 2025 08:10
Copy link

github-actions bot commented Feb 17, 2025

🚀 Expo preview is ready!

  • Project → trezor-suite-preview
  • Platforms → android, ios
  • Scheme → trezorsuitelite
  • Runtime Version → 26
  • More info

Learn more about 𝝠 Expo Github Action

@mroz22
Copy link
Contributor Author

mroz22 commented Feb 17, 2025

i thought it is just PR for test removal

unfortunately no, it might take some time, feel free to cherrypick e4432ec if you want

@grdddj grdddj mentioned this pull request Feb 18, 2025
@mroz22 mroz22 force-pushed the tenv-link-default-model branch from b3347b3 to 541640a Compare February 18, 2025 09:32
@mroz22 mroz22 force-pushed the tenv-link-default-model branch from 541640a to 22f08b1 Compare February 18, 2025 13:32
Also cleans up setup of e2e tests that dont need to start/set emulator
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-project This label is used to specify that PR doesn't need to be added to a project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants