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

Create LimelightMath.java #16

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

EliTheCompSciGuy
Copy link

I didn't know the exact number for some variables or the exact format of the Limelight code, but I got the math for most (if not all) of the calculations to determine distances and their rate of change. I parsed whatever I didn't know or if the value would be imported from another class.

I didn't know the exact number for some variables or the exact format of the Limelight code, but I got the math for most (if not all) of the calculations to determine distances and their rate of change. I parsed whatever I didn't know or if the value would be imported from another class.
Copy link
Member

@DanWaxman DanWaxman left a comment

Choose a reason for hiding this comment

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

The math itself looks good. However, a few things to change before merging:

  • There's a lack of documentation for parameters and returns. Documentation is key to the usage of a lot of these methods.
  • This class isn't in the package with the rest of the robot code.
  • It utilizes an entrypoint and GUI components, neither of which are available on the robot (okay entrypoint maybe, but still not good practice).

After these changes are made, it looks good and will be good to push. Good job!

I removed all of the parsing at the beginning of the code to prevent dialog boxes from popping up. I didn't put in any computations/values for these variables since x and v would be imported from another source and y hasn't been determined yet. I also put in documentation stating what each variable means and what each method is calculating .
@EliTheCompSciGuy EliTheCompSciGuy changed the title Create LimelightMath Create LimelightMath.java Feb 4, 2018
@EliTheCompSciGuy
Copy link
Author

I updated the code:
04ed5b2

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