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

Add Monitor and MonitoredSubystem to WPILIb Interface #24

Closed
aidnem opened this issue Oct 7, 2024 · 10 comments · Fixed by #31
Closed

Add Monitor and MonitoredSubystem to WPILIb Interface #24

aidnem opened this issue Oct 7, 2024 · 10 comments · Fixed by #31
Assignees

Comments

@aidnem
Copy link
Contributor

aidnem commented Oct 7, 2024

Add a Monitor class to monitor whether a certain state is unacceptable for an unacceptable period of time and take action if it is. Also add a MonitoredSubsystem class to check each monitor every time periodic is called.

This has already been described and implemented in 2024-Robot-Code:199.

Scope

  • Add Monitor class
  • Add MonitoredSubsystem class
  • Test it out in sim to see if it works.
@jkleiber
Copy link
Member

jkleiber commented Oct 7, 2024

I think the callback approach is the way to go (see this comment).

The monitored subsystem would live in wpi_interface, but I think the monitor class itself should live in its own package (maybe called "monitors")

@aidnem
Copy link
Contributor Author

aidnem commented Nov 6, 2024

I'm going to add the option for a building architecture for the Monitor class because I think it will be more readable and clear than having a giant constructor with a bunch of unnamed suppliers.

/* BEFORE */
MonitorManager.addMonitor(
    new CustomMonitor(
        "armEncoderUnplugged", // Name to log callback under
        true, // Sticky fault (remain faulted even after conditions become acceptable again
        () -> !(DriverStation.isEnabled()
                && (aimerInputs.aimAppliedVolts > ScoringConstants.aimerkS * 2
                        && aimerInputs.aimVelocityRadPerSec
                                < ScoringConstants.aimerMovementThresholdRadPerSec)), // isStateValid function to check whether the state is currently valid or not
        ScoringConstants.maxAimUnresponsiveTimeSeconds, // timeToFault (how long can state be invalid before a fault occurs)
        () -> { // faultCallback, the function run every tick when the monitor has detected a fault.
            ScoringSubsystem.setOverrideMode(true);
            ScoringSubsystem.setOverrideVolts(0.0);
        }));
/* AFTER */
MonitorManager.addMonitor(
    new CustomMonitor("armEncoderUnplugged") // Name of Monitor. This will be the name it's logged under.
        .withStickyness(true) // Sticky fault (remain faulted even after conditions become acceptable again
        .withIsStateValid(() -> !(DriverStation.isEnabled()
                && (aimerInputs.aimAppliedVolts > ScoringConstants.aimerkS * 2
                        && aimerInputs.aimVelocityRadPerSec
                                < ScoringConstants.aimerMovementThresholdRadPerSec))) // isStateValid function to check whether the state is currently valid or not
        .withTimeToFault(ScoringConstants.maxAimUnresponsiveTimeSeconds) // timeToFault (how long can state be invalid before a fault occurs)
        .withFaultCallback(() -> { // faultCallback, the function run every tick when the monitor has detected a fault.
            ScoringSubsystem.setOverrideMode(true);
            ScoringSubsystem.setOverrideVolts(0.0);
        }));

@avidraccoon
Copy link
Contributor

avidraccoon commented Nov 6, 2024

Where will the monitors be added to the MonitorManager?

@aidnem
Copy link
Contributor Author

aidnem commented Nov 6, 2024

Monitor is under its own package and MonitoredSubsystem is under wpi interface.

@avidraccoon
Copy link
Contributor

Where in the robot code would we want them to be used we might want to limit them to prevent hacky solutions and make sure the are added in a consistent manor.

@aidnem
Copy link
Contributor Author

aidnem commented Nov 6, 2024

Subsystems should use them by extending MonitoredSubsystem. They will register their monitors in their constructor and then the monitors will automatically be run every periodic by the MonitoredSubsystem; no other action needs to be taken. I think this solution is fairly consistent.

@avidraccoon
Copy link
Contributor

So should only MonitoredSubsytem's child classes be able to add the monitors?

@aidnem
Copy link
Contributor Author

aidnem commented Nov 7, 2024

No I believe they should be able to be created anywhere, because if we create a monitor for battery voltage it might live in robot container.

@avidraccoon
Copy link
Contributor

Where would that monitor even be added? It isn't a MonitoredSubsystem where they are meant to be stored.

@aidnem
Copy link
Contributor Author

aidnem commented Nov 7, 2024

Just manually store the monitor as a variable and run its periodic in the periodic of RobotContainer.

@aidnem aidnem linked a pull request Nov 7, 2024 that will close this issue
@jkleiber jkleiber added this to the 2025 MVP Features milestone Nov 19, 2024
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 a pull request may close this issue.

3 participants