Skip to content

error handling #29

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 22 commits into from
Aug 21, 2024
Merged

error handling #29

merged 22 commits into from
Aug 21, 2024

Conversation

Ishan1522
Copy link
Member

No description provided.

JacksonElia
JacksonElia reviewed Jul 27, 2024
View reviewed changes
JacksonElia
JacksonElia reviewed Jul 27, 2024
View reviewed changes
JacksonElia
JacksonElia reviewed Jul 27, 2024
View reviewed changes
JacksonElia
JacksonElia reviewed Jul 27, 2024
View reviewed changes
JacksonElia
JacksonElia reviewed Jul 27, 2024
View reviewed changes
JacksonElia
JacksonElia requested changes Jul 28, 2024
View reviewed changes
Copy link
Member

@JacksonElia JacksonElia left a comment

Choose a reason for hiding this comment

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

I still don't know much about the threading, so once you get it working I'd love to hear about it. I made a few comments earlier of small things to change, but the main thing would probably be the try catch.

JacksonElia
JacksonElia requested changes Jul 28, 2024
View reviewed changes
Copy link
Member

@JacksonElia JacksonElia left a comment

Choose a reason for hiding this comment

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

I still don't know much about the threading, so once you get it working I'd love to hear about it. I made a few comments earlier of small things to change, but the main thing would probably be the try catch.

@Ishan1522
Copy link
Member Author

The threading is working...? That's why we merged vision-overhaul. Or are u referring to something else.

@JacksonElia
Copy link
Member

The threading is working...? That's why we merged vision-overhaul. Or are u referring to something else.

I think somewhere in this branch you changed a part of the threading and commented that you didn't know if it would work

JacksonElia
JacksonElia reviewed Jul 29, 2024
View reviewed changes
JacksonElia
JacksonElia approved these changes Jul 29, 2024
View reviewed changes
Copy link
Member

@JacksonElia JacksonElia left a comment

Choose a reason for hiding this comment

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

Overall LGTM, I had like one nit but it looks good!

Ishan1522 and others added 2 commits July 29, 2024 14:59
@Ishan1522 @JacksonElia
Update src/main/java/frc/robot/subsystems/vision/VisionSubsystem.java
d969077 
Jacks change

Co-authored-by: Jack <85963782+JacksonElia@users.noreply.github.com>
@Ishan1522
decent changes
51f121e
@Ishan1522 Ishan1522 merged commit 9790e2d into main Aug 21, 2024
6 checks passed
@Ishan1522 Ishan1522 deleted the errorsharer branch August 21, 2024 22:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JacksonElia JacksonElia JacksonElia approved these changes

@zuntue zuntue Awaiting requested review from zuntue

@Ryan-Robot12 Ryan-Robot12 Awaiting requested review from Ryan-Robot12

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

Successfully merging this pull request may close these issues.
2 participants
@Ishan1522 @JacksonElia