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

Change camera exposure step to integer #1306

Closed

Conversation

DevonRD
Copy link
Contributor

@DevonRD DevonRD commented Apr 1, 2024

Potentially mitigates 1182, see comment there for more details. Exposure is rounded in the backend anyway, so there is no reason to send several update events for ultimately the same setting.

@DevonRD DevonRD requested a review from a team as a code owner April 1, 2024 16:35
@srimanachanta
Copy link
Member

Is this the place we are making this false assumption? Do we not do this elsewhere in the frontend that needs to be fixed

@DevonRD
Copy link
Contributor Author

DevonRD commented Apr 1, 2024

Is this the place we are making this false assumption? Do we not do this elsewhere in the frontend that needs to be fixed

It seems Gaussian blur, filter tightness, and contour ratio all use 0.1 step as well. I imagine the same queue buildup issue will occur with those, depending on the processing speed of the events. However, I believe decimal points are used in the backend so we can't simply restrict the setting without side effects like we can with exposure

@DevonRD
Copy link
Contributor Author

DevonRD commented Apr 1, 2024

Is this the place we are making this false assumption? Do we not do this elsewhere in the frontend that needs to be fixed

However, I did just notice that object detection has 0.01 step for confidence, contour area, and contour ratio, which could send an absurd number of data change events to the queue if sliding manually (depending on the frequency that vue triggers the update listener on the slider). This could also nuke the camera stream and the backend processing until the queue catches up or PhotonVision is restarted

@mcm001
Copy link
Contributor

mcm001 commented Apr 1, 2024

The big difference here is that when you set camera properties like exposure, the backend sets the camera exposure through cscore in that main HTTP server thread. Other property changes are just accomplished through reflection and happen quickly.

This is a bandaid over a proper implantation of asynchronously changing pipeline parameters (including software only things like confidence, and hardware things like camera brightness)

@DevonRD
Copy link
Contributor Author

DevonRD commented Apr 1, 2024

The big difference here is that when you set camera properties like exposure, the backend sets the camera exposure through cscore in that main HTTP server thread. Other property changes are just accomplished through reflection and happen quickly.

This is a bandaid over a proper implantation of asynchronously changing pipeline parameters (including software only things like confidence, and hardware things like camera brightness)

Right, I am aware, but it at least mitigates the existing problem by doing earlier what would be done anyway, right? Maybe it would be better to step to 0.5 to avoid any exposure value misses in ov2311 while keeping backend updates minimal.

@DevonRD DevonRD closed this Apr 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants