Skip to content

[WIP] Test THP ACK piggybacking#6557

Closed
romanz wants to merge 8 commits into
mainfrom
romanz/enable-piggy
Closed

[WIP] Test THP ACK piggybacking#6557
romanz wants to merge 8 commits into
mainfrom
romanz/enable-piggy

Conversation

@romanz
Copy link
Copy Markdown
Contributor

@romanz romanz commented Mar 5, 2026

This reverts commit 586df8a and enabled host send-side THP ACK piggybacking for some interactive workflows.

See #6135 and #6506.

@romanz romanz self-assigned this Mar 5, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 5, 2026

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 555100e5-c34b-4ba5-a0b9-11606aa22546

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch romanz/enable-piggy
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Tip

You can generate walkthrough in a markdown collapsible section to save space.

Enable the reviews.collapse_walkthrough setting to generate walkthrough in a markdown collapsible section.

@trezor-bot trezor-bot Bot added this to Firmware Mar 5, 2026
@github-project-automation github-project-automation Bot moved this to 🔎 Needs review in Firmware Mar 5, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 5, 2026

en main(all)

model device_test click_test persistence_test
T2T1 test(all) main(all) test(all) main(all) test(all) main(all)
T3B1 test(all) main(all) test(all) main(all) test(all) main(all)
T3T1 test(all) main(all) test(all) main(all) test(all) main(all)
T3W1 test(all) main(all) test(all) main(all) test(all) main(all)
Translations

cs main(all)

model device_test click_test
T2T1 test(all) main(all) test(all) main(all)
T3B1 test(all) main(all) test(all) main(all)
T3T1 test(all) main(all) test(all) main(all)
T3W1 test(all) main(all) test(all) main(all)

de main(all)

model device_test click_test
T2T1 test(all) main(all) test(all) main(all)
T3B1 test(all) main(all) test(all) main(all)
T3T1 test(all) main(all) test(all) main(all)
T3W1 test(all) main(all) test(all) main(all)

es main(all)

model device_test click_test
T2T1 test(all) main(all) test(all) main(all)
T3B1 test(all) main(all) test(all) main(all)
T3T1 test(all) main(all) test(all) main(all)
T3W1 test(all) main(all) test(all) main(all)

fr main(all)

model device_test click_test
T2T1 test(all) main(all) test(all) main(all)
T3B1 test(all) main(all) test(all) main(all)
T3T1 test(all) main(all) test(all) main(all)
T3W1 test(all) main(all) test(all) main(all)

pt main(all)

model device_test click_test
T2T1 test(all) main(all) test(all) main(all)
T3B1 test(all) main(all) test(all) main(all)
T3T1 test(all) main(all) test(all) main(all)
T3W1 test(all) main(all) test(all) main(all)

Latest CI run: 23165734553

@romanz romanz moved this from 🔎 Needs review to 🏃‍♀️ In progress in Firmware Mar 5, 2026
@romanz romanz force-pushed the romanz/thp-retrans-test branch from f09d7d1 to 9da9dca Compare March 5, 2026 14:58
@romanz romanz force-pushed the romanz/enable-piggy branch from 3373bf4 to 6023d44 Compare March 5, 2026 15:28
@romanz romanz force-pushed the romanz/thp-retrans-test branch from 9da9dca to ea0800e Compare March 5, 2026 15:31
@romanz romanz changed the title Revert "fix(core/thp): disable THP ACK piggybacking as a workaround for #6506" Re-enable THP ACK piggybacking Mar 5, 2026
@romanz romanz force-pushed the romanz/enable-piggy branch from 6023d44 to b6490f6 Compare March 5, 2026 15:43
@romanz romanz added the translations Put this label on a PR to run tests in all languages label Mar 5, 2026
@romanz romanz force-pushed the romanz/enable-piggy branch 3 times, most recently from ec77347 to 514fe62 Compare March 6, 2026 11:24
@romanz romanz force-pushed the romanz/enable-piggy branch from 514fe62 to b51bfd8 Compare March 6, 2026 14:20
@romanz romanz force-pushed the romanz/thp-retrans-test branch from ea0800e to 6075da0 Compare March 6, 2026 14:21
@romanz romanz force-pushed the romanz/thp-retrans-test branch from 6075da0 to 20a2a87 Compare March 6, 2026 18:26
@romanz romanz force-pushed the romanz/enable-piggy branch from b51bfd8 to 1c17670 Compare March 6, 2026 18:27
@romanz romanz moved this from 🏃‍♀️ In progress to 🔎 Needs review in Firmware Mar 12, 2026
@romanz romanz moved this from 🔎 Needs review to 🏃‍♀️ In progress in Firmware Mar 12, 2026
@romanz romanz force-pushed the romanz/thp-retrans-test branch 2 times, most recently from 48efeb1 to 32d9001 Compare March 15, 2026 14:59
Base automatically changed from romanz/thp-retrans-test to main March 16, 2026 08:55
romanz added 3 commits March 16, 2026 21:07
It allows scoping "interactive" context, where THP ACK piggybacking
will be enabled.

[no changelog]
`session.call_raw()` doesn't send a THP ACK when it is done,
so the device will re-transmit the error message over and over.

[no changelog]
romanz added 5 commits March 16, 2026 21:58
Also rename ACK-sending methods, since "flushing" ACKs should not be skipped
(otherwise the device may get stuck waiting for the ACK, while the host is waiting on DebugLink).

[no changelog]
Used by translations & homescreen upload.

[no changelog]
@romanz romanz changed the title Re-enable THP ACK piggybacking [WIP] Test THP ACK piggybacking Mar 16, 2026
@romanz romanz force-pushed the romanz/enable-piggy branch from 1c17670 to b6bfb8e Compare March 16, 2026 21:01
@romanz
Copy link
Copy Markdown
Contributor Author

romanz commented Mar 17, 2026

Closing in favor of #6594.

@romanz romanz closed this Mar 17, 2026
@obrusvit obrusvit moved this from 🏃‍♀️ In progress to ✅ Done (no QA) in Firmware Mar 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

translations Put this label on a PR to run tests in all languages

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

2 participants