search

wpilibsuite / allwpilib

Official Repository of WPILibJ and WPILibC
https://wpilib.org/
Other
1.03k stars 607 forks source link

Consider adding VirtualSubsystems? #6754

Closed Braykoff closed 2 weeks ago

Braykoff commented 2 weeks ago

I have seen teams create their own 'VirtualSubsystem' class, (example, by 6328) which functions similarly to a Subsystem with periodic() methods but has no hardware mutex. It doesn't need to handle the logic of being required by commands or having a default command, it is simply a more readable way of having a part of their robot (such as cameras) have periodic code without needing to handle commands for it. The addPeriodic() method may be insufficient in some cases because it must be called from a TimedRobot class (as most teams define subsystems in a RobotContainer, they would need wrapper functions to use this). Also, having the periodic method handled by the CommandScheduler, which has a watchdog, makes tracking down loop overrun issues in these virtual subsystems easier because it would add an epoch specifically for them.

I think VirtualSubsystems would be a good addition and I can implement it soon, but I'm looking to get some other feedback on this first.

rzblue commented 2 weeks ago

You can use Subsystem/SubsystemBase to accomplish this and simply not add it to commands as a requirement. I don't think this needs to be added as a separate class at the moment.

jwbonner commented 2 weeks ago

simply not add it to commands as a requirement

The original purpose of this class was to prevent commands from accidentally requiring subsystems that shouldn't interfere with scheduling. One example was an LED class, which benefits from periodic callbacks and is otherwise structured as a subsystem but handles state conflicts internally. Vision is also subsystem-like in structure (especially with AdvantageKit-style hardware abstraction) but doesn't use commands, so we didn't want to risk any misuse.

This feature is associated with a fairly specific structure for command-based. In particular, subsystems under AdvantageKit are often defined more by their use of hardware abstraction rather than the mutexing behavior with commands; hence why we called it VirtualSubsystem despite not being a subsystem in the traditional command-based sense. Given that context, I'm doubtful whether this makes sense as a standard library feature.