rename OICondition inner class to Condition. update example OI to use OIFragment and Conditions.

[dejabot]

Description

• simple renaming of OICondition to Condition, since it's within OIFragment.
• set the loggerCategory for OIFragment
• add usage of OIFragment and Condition to example OI.

How Has This Been Tested?

renaming, loggerCategory, and examples only.

[rcahoon]

should there not be a rightJoystick?

[dejabot]

for these examples (most frequently used with BurroBots - a competition bot would typically use Swerve drive with the com.team766.robot.common.DriverOI), I was thinking arcade drive. but the example sets the wrong expectation with leftJoystick - renamed to joystick.

I can also include leftJoystick and rightJoystick or joystick and gamepad if you think that's better.

[rcahoon]

joystick SGTM

[dejabot]

Thx! PTAL?

[rcahoon]

non-blocking, because i'm not sure if i have a better way to do this, but

it seems like the code around conditions is getting pretty spread out, which hurts readability. it requires code in the constructor, in handlePre, and in handleOI

[dejabot]

sure, let's discuss.

this is less about code for conditions and more about code used in the OI that's also used in conditions.
problem to solve - allow for some code to run before conditions are evaluated (since we moved that to an explicit step run by something like the OIFragment). we could simplify by having everything in handleOI, at the cost of "missing a frame".

I understand the concern - open to a different approach that allows code to run before the evaluation step.

wdyt?

[dejabot]

thinking about alternatives - one could be to make this part of the Conditions - tho I don't think that would be more readable.

eg, that woud make updating variables a side effect.. and given that lambdas are supposed to be short, devs might end up moving some of the logic into helper methods called via method references.

[rcahoon]

A couple of ideas. I'm not sure any of them is the best, though I like (2) because it doesn't require class member variables.

1. Condition has an invalidate() method that OIFragment calls before handleOI. Then conditions are lazily evaluated on the first call to e.g. isTriggering
2. Instead of passing the lambda to the constructor, Condition has an evaluate method that the user is required to call during handleOI.

public class DriverOI extends OIFragment {
    private final JoystickReader joystick;

    private Condition moveJoystick = new Condition();

    public DriverOI(JoystickReader joystick) {
        super("DriverOI");
        this.joystick = joystick;
    }

    protected void handleOI(Context context) {
        final double joystickX = joystick.getAxis(InputConstants.AXIS_X);
        final double joystickY = joystick.getAxis(InputConstants.AXIS_Y);
        moveJoystick.evaluate(Math.abs(joystickX) > 0 || Math.abs(joystickY) > 0);

        if (moveJoystick.isTriggering()) {
            // handle joystick movement
        }
    }
}

[dejabot]

hmm, I'd be a bit uncomfortable with 2 since that feels a bit error-prone, eg if devs miss calling that, they'll wonder why their conditions aren't triggering as expected.

if we wanted to avoid having a preHandle, we could recommend conditions be more self-contained vs depend on these intermediate member variables that store their own state. (nothing would prevent someone from doing that eg in a helper method the developer set up and called from their conditions but those calls would be more obviously visible).

[rcahoon]

i thought about requesting that conditions be more self-contained, but then I thought about what if the condition is based off of some complex code, for example processing vision output or something. I guess in that case, probably that complex processing should be moved into a Mechanism or ProcedureWithValue or something. but it still seems like something that someone might want to do, even if it was just temporary.

what if we combined (1) and (2)? the OIFragment invalidates the Condition prior to calling handleOI, and if you try to query the Condition before evaluating it, it raises an exception. we could also raise an exception if someone tries to evaluate a condition multiple times in a single frame (i.e. evaluate is called when the Condition is not invalidated)

[dejabot]

thanks! riffing off of this, I'm trying something simple that allows a fragment sublass to evaluate all of the conditions and checks that that call was made, throwing an exception if not.

wdyt?

[dejabot]

thx, PTAL?

[dejabot]

removed pre and post methods. make users evaluate conditions (OIFragment will throw an IllegalStateException if this is omitted).

PTAL? thank you!

[rcahoon]

getting rid of pre and post methods is a nice improvement. i'm still kinda hung up on the need to use class member variables, which don't make data dependencies between variables/conditions obvious (e.g. the user could call evaluateConditions() before all variables have been updated), so I made this branch for comparison https://github.com/Team766/2024/compare/rcahoon/oi-conditions