Skip to content

Conversation

@TheSecurityDev
Copy link
Contributor

@TheSecurityDev TheSecurityDev commented Oct 18, 2025

We should also include it in the list of images in the PR, but only exclude from the diff check (since the QR code isn't repeatable)

Here's the artifacts branch for this PR for confirmation:
https://github.com/commaai/ci-artifacts/blob/openpilot/pr-36393-raylib-ui/pair_device.png

And from a recent PR:
https://github.com/commaai/ci-artifacts/blob/openpilot/pr-36439-raylib-ui/pair_device.png

...ok now it's working? 🤔

Copilot AI review requested due to automatic review settings October 18, 2025 00:00
@github-actions github-actions bot added the ui label Oct 18, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Fix navigation for the "pair device" raylib screenshot flow so the screenshot can be captured deterministically.

  • Open Settings before attempting to navigate to the Pair Device screen.
  • Update the click target to new coordinates presumably corresponding to the Pair Device entry.

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 18, 2025

raylib UI Preview

homescreen : $${\color{red}\text{DIFFERENT}}$$
master proposed
diff composite diff
homescreen_paired : $${\color{red}\text{DIFFERENT}}$$
master proposed
diff composite diff
homescreen_prime : $${\color{red}\text{DIFFERENT}}$$
master proposed
diff composite diff
homescreen_update_available : $${\color{red}\text{DIFFERENT}}$$
master proposed
diff composite diff
offroad_alert : $${\color{red}\text{DIFFERENT}}$$
master proposed
diff composite diff
settings_developer : $${\color{red}\text{DIFFERENT}}$$
master proposed
diff composite diff
settings_software : $${\color{red}\text{DIFFERENT}}$$
master proposed
diff composite diff
settings_software_download : $${\color{red}\text{DIFFERENT}}$$
master proposed
diff composite diff
settings_software_release_notes : $${\color{red}\text{DIFFERENT}}$$
master proposed
diff composite diff
All Screenshots

@sshane
Copy link
Contributor

sshane commented Oct 23, 2025

why doesn't anything change?

@TheSecurityDev
Copy link
Contributor Author

TheSecurityDev commented Oct 23, 2025

why doesn't anything change?

Because the pair device screenshot is purposely excluded from the diff check in the workflow. But you can confirm by checking the artifacts that are uploaded, and it should be included now where previously it wasn't. Actually it was included before but it was just the home screen or settings and not the pair device page, since the coordinates were wrong.

I tried to make it included in the list still but only excluded from the diff check, but I couldn't make it work.

@TheSecurityDev
Copy link
Contributor Author

I've now included a link to the artifacts file for confirmation

@TheSecurityDev
Copy link
Contributor Author

Hold up, did this get fixed somehow without this?

@sshane
Copy link
Contributor

sshane commented Oct 23, 2025

There's two buttons that can open the pairing dialog.

@sshane sshane closed this Oct 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants