-
Notifications
You must be signed in to change notification settings - Fork 268
Add Camera Focus Mode #2180
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
Add Camera Focus Mode #2180
Conversation
Just a reminder 😄 |
photon-core/src/main/java/org/photonvision/vision/pipe/impl/GrayscalePipe.java
Outdated
Show resolved
Hide resolved
just a heads up, this won't actually close the issue. You need to use the keyword |
samfreund
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should warn teams that refocusing can lead to their calibration being invalidated.
It seems like we're changing the resolution when we change into focus mode, expected behavior for teams would probably be keeping the same resolution or picking the resolution for focus mode. I'm also unsure of whether this affects focus, if it does it's important to note.
We should consider the case where the focus pipeline is left on by accident, perhaps add a warning/popup modal on the dashboard. If this is implemented here, it should also be done for the calibration mode.
It doesn't seem like the highest focus value is saved and displayed, I believe the intent was to do so. Feel free to correct me if this has changed.
photon-core/src/main/java/org/photonvision/vision/processes/PipelineManager.java
Show resolved
Hide resolved
Co-authored-by: Sam Freund <[email protected]>
b4dff9f to
132be8e
Compare
samfreund
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It still doesn't seem that we're saving and displaying the highest focus value, that's something that I think would be helpful and we can save. Otherwise, it's looking pretty good. I left some comments on things that need to get cleaned up, but most of them are nits, so it should be pretty quick to clear up. Also, make sure you remember to lint 😄
… not place target too close to the camera.
Gold856
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working through this!
Gold856
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some consistency things.
Co-authored-by: Gold856 <[email protected]>
Co-authored-by: Gold856 <[email protected]>
This reverts commit 618072c.
## Description #2180 added an additional value to PipelineType.java. Enums are serialized with `ordinal()`, and the frontend wasn't updated to account for this, causing an off-by-one error where the UI thought the pipeline was actually the next pipeline over. This resyncs the enum on the frontend to the backend and adjusts the mapping from PipelineType to WebsocketPipelineType to make everything match. Fixes #2201. closes #2202 ## Meta Merge checklist: - [x] Pull Request title is [short, imperative summary](https://cbea.ms/git-commit/) of proposed changes - [x] The description documents the _what_ and _why_ - [x] This PR has been [linted](https://docs.photonvision.org/en/latest/docs/contributing/linting.html). - [ ] 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 v2025.3.2 - [x] If this PR touches pipeline settings or anything related to data exchange, the frontend typing is updated - [ ] If this PR addresses a bug, a regression test for it is added
## Description PhotonVision#2180 added an additional value to PipelineType.java. Enums are serialized with `ordinal()`, and the frontend wasn't updated to account for this, causing an off-by-one error where the UI thought the pipeline was actually the next pipeline over. This resyncs the enum on the frontend to the backend and adjusts the mapping from PipelineType to WebsocketPipelineType to make everything match. Fixes PhotonVision#2201. closes PhotonVision#2202 ## Meta Merge checklist: - [x] Pull Request title is [short, imperative summary](https://cbea.ms/git-commit/) of proposed changes - [x] The description documents the _what_ and _why_ - [x] This PR has been [linted](https://docs.photonvision.org/en/latest/docs/contributing/linting.html). - [ ] 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 v2025.3.2 - [x] If this PR touches pipeline settings or anything related to data exchange, the frontend typing is updated - [ ] If this PR addresses a bug, a regression test for it is added
## Description Camera focus tool pipeline using a Laplacian and finding the variance. Similar to Limelight. closes PhotonVision#1597 ## Meta Merge checklist: - [x] Pull Request title is [short, imperative summary](https://cbea.ms/git-commit/) of proposed changes - [x] The description documents the _what_ and _why_ - [x] This PR has been [linted](https://docs.photonvision.org/en/latest/docs/contributing/linting.html). - [x] 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 v2025.3.2 - [x] If this PR touches pipeline settings or anything related to data exchange, the frontend typing is updated - [ ] If this PR addresses a bug, a regression test for it is added
## Description PhotonVision#2180 added an additional value to PipelineType.java. Enums are serialized with `ordinal()`, and the frontend wasn't updated to account for this, causing an off-by-one error where the UI thought the pipeline was actually the next pipeline over. This resyncs the enum on the frontend to the backend and adjusts the mapping from PipelineType to WebsocketPipelineType to make everything match. Fixes PhotonVision#2201. closes PhotonVision#2202 ## Meta Merge checklist: - [x] Pull Request title is [short, imperative summary](https://cbea.ms/git-commit/) of proposed changes - [x] The description documents the _what_ and _why_ - [x] This PR has been [linted](https://docs.photonvision.org/en/latest/docs/contributing/linting.html). - [ ] 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 v2025.3.2 - [x] If this PR touches pipeline settings or anything related to data exchange, the frontend typing is updated - [ ] If this PR addresses a bug, a regression test for it is added
Description
Camera focus tool pipeline using a Laplacian and finding the variance. Similar to Limelight.
closes #1597
Meta
Merge checklist: