Migrate constants to JSON files

[jkleiber]

Purpose The purpose of this issue is to make configuration management better by refactoring our current Constants.java constants into JSON files that can be read at init. This would make it such that we don't need to re-compile if changing just one variable.

Some constants are sim or robot specific. Some may be specific to shop vs competition. Some constants are common to all configurations. A file structures that makes sense should be used to manage these differences.

A new class (or an update to Constants.java) should be made that loads in the appropriate configuration files and makes the parameters available to the software stack.

Project Scope

• [ ] Migrate current constants into JSON files
• [ ] Create a reasonable file structure for managing parameters
• [ ] Create a class or modify Constants.java to manage the loaded parameters

[aidnem]

@jkleiber would you prefer to have this still be type-safe (keep the existing constant type definitions but read values on init from JSON files)? I'm planning out how I could implement this. Also, I've looked into it and it looks like WPILib depends on Jackson for JSON parsing. Can you think of any reason this wouldn't be suitable for our needs?

[jkleiber]

@aidnem keeping type safety is probably best since we're using Java.

I don't really know what Jackson is tbh. I'll give you the design freedom to do whatever you think is best and scalable, and we can iterate on it as needed

[aidnem]

Potential file structure: One JSON file per object from Constants.java. I think we should have a standard folder for the constants as previously defined. Additionally, we should add a sim folder containing one file per object, just like in standard. However, only constants that need to be overwritten in sim need to be included, the rest will use the default values defined in their standard counterparts. The tree can be seen below:

src/main/deploy/constants
├── sim
│   ├── CANDevices.json
│   ├── ConversionConstants.json
│   ├── DriveConstants.json
│   ├── EndgameConstants.json
│   ├── FeatureFlags.json
│   ├── FieldConstants.json
│   ├── IOConstants.json
│   ├── IntakeConstants.json
│   ├── LEDConstants.json
│   ├── ScoringConstants.json
│   ├── SensorConstants.json
│   ├── TunerConstants.json
│   └── VisionConstants.json
└── standard
    ├── CANDevices.json
    ├── ConversionConstants.json
    ├── DriveConstants.json
    ├── EndgameConstants.json
    ├── FeatureFlags.json
    ├── FieldConstants.json
    ├── IOConstants.json
    ├── IntakeConstants.json
    ├── LEDConstants.json
    ├── ScoringConstants.json
    ├── SensorConstants.json
    ├── TunerConstants.json
    └── VisionConstants.json

[jkleiber]

@aidnem I like this concept, and it's given me some ideas:

1. We should make a common folder for constants that don't change based on sim/robot/HITL (i.e. conversion constants)
2. We should migrate feature flags to their respective subsystem constants JSON. Since your framework has constants separated out based on deployment platform (sim/robot). This would have the added benefit of solving #188 by design rather than using some complicated enum set-up. The only potential problem here is that we could have repetitive constants files for sim/robot depending on the subsystem, but I think we can refactor that as needed (resisting premature optimization lol)
3. CANDevices isn't currently used, and we have the seemingly similar IOConstants and SensorConstants. I recommend making all of these into one JSON: DeviceConstants.json
4. In support of #157, it might be best to have a folder for field environments. I also added a file to the structure called "ConfigurationConstants.json" that could theoretically be used to differentiate sim from HITL, and select which field layout we want, etc.

src/main/deploy/constants
├── common
│   ├── ConfigurationConstants.json
│   └── ConversionConstants.json  
├── env
│   ├── shop
│   │   └── FieldConstants.json
│   └── official
│        └── FieldConstants.json
├── sim
│   ├── DeviceConstants.json
│   ├── DriveConstants.json
│   ├── EndgameConstants.json
│   ├── IntakeConstants.json
│   ├── LEDConstants.json
│   ├── ScoringConstants.json
│   ├── TunerConstants.json
│   └── VisionConstants.json
└── robot
    ├── DeviceConstants.json
    ├── DriveConstants.json
    ├── EndgameConstants.json
    ├── IntakeConstants.json
    ├── LEDConstants.json
    ├── ScoringConstants.json
    ├── TunerConstants.json
    └── VisionConstants.json

[aidnem]

@jkleiber Awesome. One question, in your system will constants default to be from robot and then only be overwritten if they exist in sim, do you plan to define every constant in both places?

[jkleiber]

@aidnem maybe we could define default constants in common and then modify in robot or sim as needed. i.e. use the platform specific constant if it is defined, otherwise use common

[aidnem]

I'm super swamped with AP review at the moment, so I'm not sure if I'll have time to implement this before school is out. If somebody else wants to take it, I can help get started, as I'm already in the process of planning how it could be done.

[jkleiber]

@aidnem school comes first of course - with your file structure and some reading of the docs I'm sure someone else can do the implementation. If not, the work will always be here waiting

[jkleiber]

Discussion in person: We should use a non-wpilib library that loads in files based on a schema (i.e. something like protobuf, flatbuffers) in order to maintain static typing while also being able to change constant values without needing to recompile

[avidraccoon]

@jkleiber I have a question about how the structure of the class would the class variables work because they can't be final anymore because they need to be loaded in so would you want to access them thru functions or maybe just make them public static?

[jkleiber]

@avidraccoon making everything public static for now is acceptable.

This is a trade-off we will make in order to avoid recompiling over and over again when tuning constants. Ultimately it may be good to allow the code to change the value itself and save it to a JSON when running test mode

[avidraccoon]

@jkleiber Would making the Constants class have an initiate method that loads the Constants from JSON work?

[jkleiber]

Yes that would be best @avidraccoon

[avidraccoon]

@jkleiber I believe I have almost finished the code to read the constants should it include exceptions if it can't find the constants or are the wrong type?

[jkleiber]

@avidraccoon we are using the schema based approach right?

If yes, we should be able to fall back to defaults encoded in the schema if the file cannot be found / is corrupted / the wrong type is provided.

Let's catch any exception and fall back to defaults as a result. Then we can set a validity flag in the constants class, which can be logged in order to see that we made a mistake (without having the robot code crash)

[avidraccoon]

@jkleiber Should the schema be a second file that is used to generate a Java class or just a Java class?

[jkleiber]

@avidraccoon The schema is normally a second file that gets turned into a file recognized by the target programming language in a step before compiling the main program.

Part of this ticket will be to evaluate the Interface Description Language (IDL) we want to use. The two I am most familiar with are here:

• flatbuffers
• protobuf

For example, if we were to choose flatbuffers (and we are using Java): we would use the flatbuffers schema compiler to turn a fbs schema file into a java file

If you find an option that you're more comfortable with than one of those choices, feel free to post it here. I think our only current requirements are:

1. The IDL library shall have Java support (we don't want to write our own)
2. The IDL library shall read in a JSON file and serialize that to a java object/class that it autogenerates
3. The IDL library shall support deserializing a java class to a JSON file (in case we want to auto-save changes)

[avidraccoon]

@jkleiber Does that work on static variables?

[jkleiber]

I don't see why it wouldn't (but I'm not a java expert). I believe we could do the following (pseudo-java-code):

public class Constants {
    public static ConfigurationConstants configurationConstants; // where ConfigurationConstants is auto-generated by the IDL library
    public static void init(String path) {
        // given a path to constants directory, load all the constants
       ...

        initConfigurationConstants(path + "ConfigurationConstants.json");
    }

    private static void initConfigurationConstants(String path) {
        // read the JSON file with the library provided JSON reader
        IDLJSONReader reader = new IDLJSONReader(path);

        configurationConstants =  ConfigurationConstants.getJSONAsConfigurationConstants(reader);
    }
}

[avidraccoon]

@jkleiber I should be able to implement that. Is jackson or gson fine I have a bit more experience with those?

[jkleiber]

@avidraccoon the main reason I'd advocate for the protobuf/flatbuffers options is due to their faster speed and lower memory usage: https://medium.com/@anirudh.ramanan/flatbuffers-fast-serialization-library-345bb57f35c6

Perhaps give one of those things a try first, and if it doesn't feel like a good idea try something you're more familiar with. We'll need the solution to be quite adaptable and scalable to various constants files