This repository was archived by the owner on Jun 3, 2026. It is now read-only.

Temporary PR to view code (do not merge)#41

Draft

MobileOak wants to merge 27 commits into

mainfrom

MobileOak commented Mar 21, 2026

Just creating this to give feedback to Elliot.

ryanelbert5125gmail and others added 10 commits

March 14, 2026 19:14


          Changes

ac16f30


          feat: make isSimulation final

0381d48


          Changes

3ce3fb1


          feat: make isSimulation final

1e26ea8

tmp

803e164


          fix: everything. Still working on it. Will make

218dfe5

better commit messages when I am changing specific
things instead of just redoing everything again +
again.


          Merge branch 'Vision-V2' of https://github.com/IronRiders/rebuilt int…

beb3c10

…o Vision-V2


          fix: changed < 0 to >= 0 to align with expected behavior and prev…

d703e2e

…ent `ArithmeticException`


          style: removed redundant CameraMode enum

e8ea8a2


          style: add @nonnull to some stuff to make it not yell at me

dbbea25

MobileOak requested review from a team as code owners

March 21, 2026 16:49

Amber-leaf marked this pull request as draft

March 21, 2026 16:50

Amber-leaf commented Mar 21, 2026

Contributor

making this a draft to enforce not merging

Amber-leaf suggested changes

View reviewed changes

Amber-leaf left a comment

Contributor

You do a lot of work in VisionCamera. The point of the Subsystem paradigm is to primarily do work in subsystem files. I would refactor this as such.

MobileOak commented

View reviewed changes

src/main/java/org/ironriders/vision/VisionCamera.java Outdated

MobileOak commented

View reviewed changes

src/main/java/org/ironriders/vision/VisionCamera.java Outdated

Amber-leaf suggested changes

View reviewed changes

Amber-leaf left a comment

Contributor

You don't do any filtering of good vs bad tags. We determined that this was necessary to not get bad results.

MobileOak commented

View reviewed changes

src/main/java/org/ironriders/vision/VisionCamera.java Outdated

MobileOak commented

View reviewed changes

src/main/java/org/ironriders/vision/VisionCamera.java Outdated

MobileOak commented

View reviewed changes

src/main/java/org/ironriders/vision/VisionCamera.java Outdated

Amber-leaf suggested changes

View reviewed changes

Amber-leaf left a comment

Contributor

This code is just very unreadable and overcomplicated in general. Sorry if im being to harsh TwT.

src/main/java/org/ironriders/vision/VisionSubsystem.java Outdated

MobileOak commented

View reviewed changes

src/main/java/org/ironriders/vision/VisionCamera.java Outdated

Amber-leaf suggested changes

View reviewed changes

Amber-leaf left a comment

Contributor

You also do a lot of recursion, which is not optimal. Also, you often assume what you are indexing over is not empty or null, which is not necessarily the case.

Amber-leaf commented Mar 21, 2026

Contributor

see https://en.wikipedia.org/wiki/The_Power_of_10:_Rules_for_Developing_Safety-Critical_Code

MobileOak commented

View reviewed changes

src/main/java/org/ironriders/vision/VisionCamera.java Outdated

+                      this.simulation = (mode == CameraMode.MULTI_SIM || mode == CameraMode.SINGLE_SIM) ? true : false;
+                      if (simulation) {
+                          if (cameraSim.isEmpty()) {
+                              throw new IllegalArgumentException("Simulation cameras must be passed a PhotonCameraSim instance");

MobileOak Mar 21, 2026

Author

Could also be IllegalStateException

MobileOak Mar 21, 2026

Author

Adding more of the variables and their values will help if this exception is thrown

Amber-leaf Mar 21, 2026 •

edited

Loading

Contributor

you should also probably try to go down a failure path that recovers instead of just exiting. e.g. just throw a warning and continue without vision

MobileOak commented

View reviewed changes

src/main/java/org/ironriders/vision/VisionCamera.java Outdated

MobileOak commented

View reviewed changes

src/main/java/org/ironriders/vision/VisionCamera.java Outdated

MobileOak commented

View reviewed changes

src/main/java/org/ironriders/vision/VisionCamera.java Outdated

MobileOak commented

View reviewed changes

src/main/java/org/ironriders/vision/VisionCamera.java Outdated

MobileOak commented

View reviewed changes

src/main/java/org/ironriders/vision/VisionCamera.java Outdated

MobileOak commented

View reviewed changes

src/main/java/org/ironriders/vision/VisionCamera.java

MobileOak commented

View reviewed changes

src/main/java/org/ironriders/vision/VisionCamera.java Outdated

MobileOak commented

View reviewed changes

src/main/java/org/ironriders/vision/VisionCamera.java Outdated

MobileOak commented

View reviewed changes

src/main/java/org/ironriders/vision/VisionConstants.java Outdated

MobileOak commented

View reviewed changes

src/main/java/org/ironriders/vision/VisionConstants.java Outdated

MobileOak commented

View reviewed changes

src/main/java/org/ironriders/vision/VisionConstants.java Outdated

MobileOak commented

View reviewed changes

src/main/java/org/ironriders/vision/VisionConstants.java Outdated

-                  public static final Double WEIGHT_SCALE = 5d;
+                  public static final AprilTagFieldLayout TAG_FIELD_LAYOUT = AprilTagFieldLayout
+                          .loadField(AprilTagFields.kDefaultField);
+                  public static final SimCameraProperties SIM_CAM_PROPS = new SimCameraProperties() // todo: get actual values

MobileOak Mar 21, 2026

Author

If you're not the person getting the actual values, it would be helpful to have more detailed instructions on the source.

MobileOak commented

View reviewed changes

src/main/java/org/ironriders/vision/VisionSubsystem.java Outdated

MobileOak commented

View reviewed changes

src/main/java/org/ironriders/vision/VisionSubsystem.java Outdated

MobileOak commented

View reviewed changes

src/main/java/org/ironriders/vision/VisionSubsystem.java Outdated

MobileOak commented

View reviewed changes

src/main/java/org/ironriders/vision/VisionSubsystem.java Outdated

MobileOak commented

View reviewed changes

src/main/java/org/ironriders/vision/VisionSubsystem.java Outdated

Iprobablydontknowwhatimdoing and others added 17 commits

March 31, 2026 13:25


          Change things to make them clearer

283051e


          fix: imported VisonCamera to VisionSubsystem

1158bd9


          fix: removed import to nonexistent file

6d031b0


          feat: changed camera data from map to enum to improve readability

9cc81d6


          fix: removed unused import

674122a


          chore: Merge branch 'main' into Vision-V2

169a7c4


          fix: added import to allow build

c117419


          feat: removed unused imports

38bd8ff


          feat: clarified "magic number"

8ca7ac2


          feat: moved 'magic #s' to constants file, added comments

4d9b76a


          chore: removed unused imports

6e3c27f


          chore: formatting

bf5bc81


          feet: added ROBOT_CENTER camera (for new camera mount)

0fe9a3c


          Merge branch 'Vision-V2' of https://github.com/IronRiders/rebuilt int…

52ff507

…o Vision-V2


          some logging

50c5c6c


          Fixed some syntax errors; disabled a non-existant camera mount.

e6d01af


          Merge branch 'Vision-V2' of https://github.com/IronRiders/rebuilt int…

4fd1d0d

…o Vision-V2

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet