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

BCP: Use constants for all IDs instead of raw numbers #112

Open
INeedCaffeine opened this issue Sep 10, 2024 · 0 comments
Open

BCP: Use constants for all IDs instead of raw numbers #112

INeedCaffeine opened this issue Sep 10, 2024 · 0 comments
Labels
chore Cleans Code low priority Something that doesn't have to be completed

Comments

@INeedCaffeine
Copy link

Using a comment to describe the CAN ID assignments while using explicit numbers in the code is a little error prone because it is easy to overlook an ID conflict. For example:

  /**
   * CAN IDs for motors and encoders:
   * 1: PDH,
   * 2-5: Absolute Encoders,
   * 6-9: Krakens (Drive),
   * 10-13: Neos (Turn)
   * 14: UTB
   * 15: OTB
   * 16: Actuator
   * 17 & 18: AVAILABLE (formerly used by  the removed Climber)
   * 19: Indexer
   */
  public static enum ABSOLUTE_ENCODER {
    FRONT_RIGHT(3), // Module 0
    FRONT_LEFT(2), // Module 1
    BACK_LEFT(5), // Module 2
    BACK_RIGHT(4); // Module 3

    public final int ENCODER_ID;

    ABSOLUTE_ENCODER(int ID) {
      ENCODER_ID = ID;
    }
  }

Given that the enum CAN ID values are assigned in separate "constants" files (ArmConstants.java, DriveConstants.java, etc) it is easy to mistakenly mess up the ID assignment to the enum values like FRONT_RIGHT. It is better to define constants that you use can can easily search the code for any conflicts. It is easier to find a conflict when searching for "CAN_ID05" than simply 5 which appears in 31 files in our repo.

It is better practice to define a set of constants like CAN_ID05 (or CAN_ID_05 or CANID_05) in Constants.java and then use that in place of a hardcoded value. It also makes changing an ID easier if we have to for some reason; you simply edit the Constants.java file and re-deploy. If you have to figure out which of the 10 "constants" files we have you need to edit a value in you will take longer and could not find the one to edit as easily.

Another benefit is that you not have to maintain multiple variations of the same ID comment above in each of the constants files where you declare your types / enums. Keeping them all up to date is a needless chore that can easily become outdated which makes the comment unhelpful at best.

@INeedCaffeine INeedCaffeine added low priority Something that doesn't have to be completed chore Cleans Code labels Sep 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Cleans Code low priority Something that doesn't have to be completed
Projects
None yet
Development

No branches or pull requests

1 participant