BroncBotz3481 / YAGSL-Example

Yet Another General Swerve Library Example Project
Apache License 2.0
58 stars 143 forks source link

Document TimedRobot base class dependency and support AdvantageKit LoggedRobot #3

Closed dave-billin closed 1 year ago

dave-billin commented 1 year ago

Presently, swervelib has several modules that call Robot.getInstance().addPeriodic() to set up periodic operations:

The use of addPeriodic() implicitly creates a dependency on Robot being a subclass of edu.wpi.first.wpilibj.TimedRobot. However, this is not the case when using the AdvantageKit library from FRC 6328 for logging and log playback. AdvantageKit requires the use of a LoggedRobot base class, which does not supply the addPeriodic functionality.

Minimally, the dependency on TimedRobot should be documented.

For cases where LoggedRobot is being used as the robot base class, it is suggested that a Timer (edu.wpi.first.wpilibj.Timer) or similar class be used in a swerve subsystem to provide periodic callbacks to swerve components.

thenetworkgrinch commented 1 year ago

I haven't added the addPeriodic as a requirement in the rewrite yet, but this is very good to know. Thank you!

thenetworkgrinch commented 1 year ago

Also if you are testing/using this I would reccommend the rewrite branch. I will be merging it soon. I just need to add support for integrated absolute encoders (spark max first) then write the documentation. Ensure every class, attribute, function is documented, test it on my teams robot, and generate the javadoc htmls.

thenetworkgrinch commented 1 year ago

I finished adding support for integrated encoders and got around to adding support for Talon's too.

thenetworkgrinch commented 1 year ago

Closing because issue of addPeriodic function being required from Robot has been removed, and will not be added back in any future release.