8000 Fix snapshot methods not working by Gold856 · Pull Request #1815 · PhotonVision/photonvision · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix snapshot methods not working #1815

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Mar 19, 2025
Merged

Fix snapshot methods not working #1815

merged 4 commits into from
Mar 19, 2025

Conversation

Gold856
Copy link
Collaborator
@Gold856 Gold856 commented Mar 18, 2025 •
edited by mcm001
Loading

Description

It seems like the slash in N/A caused the image to not get saved. When not connected to FMS, the name is now None and not N/A. I also changed some of the other info added to make it consistent with the internal docs (and because I think 0 looks nicer than -1. Also, the DS seems to use 0.) Amusingly enough, I bet if you tried to take a snapshot while connected to FMS, it would work, but it would never work anywhere else. PR that introduced this bug seems to be #1460, which matches with my own manual testing of old JARs.

In addition, it seems like #1556 also broke the snapshot button in the UI. Not quite sure what happened here, but it seems like an object from UiCameraConfiguration was being used when the CameraSettingsStore should've been used instead.

Here's the new format:
image

Closes #1793.

Meta

Merge checklist:

  • Pull Request title is short, imperative summary of proposed changes
  • The description documents the what and why
  • If this PR changes behavior or adds a feature, user documentation is updated
  • If this PR touches photon-serde, all messages have been regenerated and hashes have not changed unexpectedly
  • If this PR touches configuration, this is backwards compatible with settings back to v2024.3.1
  • If this PR addresses a bug, a regression test for it is added

@Gold856 Gold856 requested a review from a team as a code owner March 18, 2025 06:58
@Juniormunk
Copy link
Contributor

Im not so sure useCameraSettingsStore().saveInputSnapshot(); is correct. We should be passing the cameras unique name since each camera stream can be unique.

@mcm001
Copy link
Contributor
mcm001 commented Mar 18, 2025

This sets the default name:
saveInputSnapshot(cameraUniqueName: string = useStateStore().currentCameraUniqueName)
from CameraSettingsStore.ts

@mcm001 mcm001 enabled auto-merge (squash) March 19, 2025 05:47
@mcm001 mcm001 merged commit de98f5f into PhotonVision:main Mar 19, 2025
36 checks passed
@mcm001 mcm001 mentioned this pull request Mar 25, 2025
Open
@Gold856 Gold856 deleted the fix-snapshots branch March 26, 2025 06:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mcm001 mcm001 mcm001 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

takeInputSnapshot and takeOutputSnapshot dont save the image
3 participants
@Gold856 @Juniormunk @mcm001
0