From 41a056cbffb617d2e09fe7d7ed6e3c055a25021f Mon Sep 17 00:00:00 2001 From: Tyler Veness Date: Tue, 21 May 2024 12:45:36 -0700 Subject: [PATCH] [wpilib] Make robot base class functions protected Users should be inheriting from these classes instead of directly instantiating them. --- wpilibc/src/main/native/cpp/TimedRobot.cpp | 18 ++++---- .../src/main/native/include/frc/TimedRobot.h | 11 ++--- .../main/native/include/frc/TimesliceRobot.h | 2 +- .../src/test/native/cpp/TimedRobotTest.cpp | 5 +++ .../test/native/cpp/TimesliceRobotTest.cpp | 4 ++ .../edu/wpi/first/wpilibj/TimedRobot.java | 42 +++++++++---------- .../edu/wpi/first/wpilibj/TimesliceRobot.java | 4 +- 7 files changed, 48 insertions(+), 38 deletions(-) diff --git a/wpilibc/src/main/native/cpp/TimedRobot.cpp b/wpilibc/src/main/native/cpp/TimedRobot.cpp index 538d1b2cf47..d6ca7fc4f07 100644 --- a/wpilibc/src/main/native/cpp/TimedRobot.cpp +++ b/wpilibc/src/main/native/cpp/TimedRobot.cpp @@ -17,6 +17,15 @@ using namespace frc; +TimedRobot::~TimedRobot() { + int32_t status = 0; + + HAL_StopNotifier(m_notifier, &status); + FRC_ReportError(status, "StopNotifier"); + + HAL_CleanNotifier(m_notifier, &status); +} + void TimedRobot::StartCompetition() { RobotInit(); @@ -89,15 +98,6 @@ TimedRobot::TimedRobot(units::second_t period) : IterativeRobotBase(period) { HALUsageReporting::kFramework_Timed); } -TimedRobot::~TimedRobot() { - int32_t status = 0; - - HAL_StopNotifier(m_notifier, &status); - FRC_ReportError(status, "StopNotifier"); - - HAL_CleanNotifier(m_notifier, &status); -} - void TimedRobot::AddPeriodic(std::function callback, units::second_t period, units::second_t offset) { m_callbacks.emplace( diff --git a/wpilibc/src/main/native/include/frc/TimedRobot.h b/wpilibc/src/main/native/include/frc/TimedRobot.h index 647285474ad..eecc0742096 100644 --- a/wpilibc/src/main/native/include/frc/TimedRobot.h +++ b/wpilibc/src/main/native/include/frc/TimedRobot.h @@ -33,6 +33,11 @@ class TimedRobot : public IterativeRobotBase { /// Default loop period. static constexpr auto kDefaultPeriod = 20_ms; + ~TimedRobot() override; + + TimedRobot(TimedRobot&&) = default; + TimedRobot& operator=(TimedRobot&&) = default; + /** * Provide an alternate "main loop" via StartCompetition(). */ @@ -43,6 +48,7 @@ class TimedRobot : public IterativeRobotBase { */ void EndCompetition() override; + protected: /** * Constructor for TimedRobot. * @@ -50,11 +56,6 @@ class TimedRobot : public IterativeRobotBase { */ explicit TimedRobot(units::second_t period = kDefaultPeriod); - ~TimedRobot() override; - - TimedRobot(TimedRobot&&) = default; - TimedRobot& operator=(TimedRobot&&) = default; - /** * Add a callback to run at a specific period with a starting time offset. * diff --git a/wpilibc/src/main/native/include/frc/TimesliceRobot.h b/wpilibc/src/main/native/include/frc/TimesliceRobot.h index bc045d58b60..7766abb4cec 100644 --- a/wpilibc/src/main/native/include/frc/TimesliceRobot.h +++ b/wpilibc/src/main/native/include/frc/TimesliceRobot.h @@ -83,7 +83,7 @@ namespace frc { * boot the roboRIO into safe mode and delete the robot program to recover. */ class TimesliceRobot : public TimedRobot { - public: + protected: /** * Constructor for TimesliceRobot. * diff --git a/wpilibc/src/test/native/cpp/TimedRobotTest.cpp b/wpilibc/src/test/native/cpp/TimedRobotTest.cpp index d639ecce078..5e98f7972e1 100644 --- a/wpilibc/src/test/native/cpp/TimedRobotTest.cpp +++ b/wpilibc/src/test/native/cpp/TimedRobotTest.cpp @@ -50,6 +50,11 @@ class MockRobot : public TimedRobot { MockRobot() : TimedRobot{kPeriod} { m_robotInitCount++; } + void AddPeriodic(std::function callback, units::second_t period, + units::second_t offset = 0_s) { + TimedRobot::AddPeriodic(std::move(callback), period, offset); + } + void SimulationInit() override { m_simulationInitCount++; } void DisabledInit() override { m_disabledInitCount++; } diff --git a/wpilibc/src/test/native/cpp/TimesliceRobotTest.cpp b/wpilibc/src/test/native/cpp/TimesliceRobotTest.cpp index 5ff9e1a6cc9..b91dc8f8de2 100644 --- a/wpilibc/src/test/native/cpp/TimesliceRobotTest.cpp +++ b/wpilibc/src/test/native/cpp/TimesliceRobotTest.cpp @@ -30,6 +30,10 @@ class MockRobot : public TimesliceRobot { MockRobot() : TimesliceRobot{2_ms, 5_ms} {} + void Schedule(std::function func, units::second_t allocation) { + TimesliceRobot::Schedule(std::move(func), allocation); + } + void RobotPeriodic() override { m_robotPeriodicCount++; } }; } // namespace diff --git a/wpilibj/src/main/java/edu/wpi/first/wpilibj/TimedRobot.java b/wpilibj/src/main/java/edu/wpi/first/wpilibj/TimedRobot.java index 2251d712d9f..2b2c0648a36 100644 --- a/wpilibj/src/main/java/edu/wpi/first/wpilibj/TimedRobot.java +++ b/wpilibj/src/main/java/edu/wpi/first/wpilibj/TimedRobot.java @@ -77,25 +77,6 @@ public int compareTo(Callback rhs) { private final PriorityQueue m_callbacks = new PriorityQueue<>(); - /** Constructor for TimedRobot. */ - protected TimedRobot() { - this(kDefaultPeriod); - } - - /** - * Constructor for TimedRobot. - * - * @param period Period in seconds. - */ - protected TimedRobot(double period) { - super(period); - m_startTimeUs = RobotController.getFPGATime(); - addPeriodic(this::loopFunc, period); - NotifierJNI.setNotifierName(m_notifier, "TimedRobot"); - - HAL.report(tResourceType.kResourceType_Framework, tInstances.kFramework_Timed); - } - @Override public void close() { NotifierJNI.stopNotifier(m_notifier); @@ -160,6 +141,25 @@ public void endCompetition() { NotifierJNI.stopNotifier(m_notifier); } + /** Constructor for TimedRobot. */ + protected TimedRobot() { + this(kDefaultPeriod); + } + + /** + * Constructor for TimedRobot. + * + * @param period Period in seconds. + */ + protected TimedRobot(double period) { + super(period); + m_startTimeUs = RobotController.getFPGATime(); + addPeriodic(this::loopFunc, period); + NotifierJNI.setNotifierName(m_notifier, "TimedRobot"); + + HAL.report(tResourceType.kResourceType_Framework, tInstances.kFramework_Timed); + } + /** * Add a callback to run at a specific period. * @@ -169,7 +169,7 @@ public void endCompetition() { * @param callback The callback to run. * @param periodSeconds The period at which to run the callback in seconds. */ - public final void addPeriodic(Runnable callback, double periodSeconds) { + protected final void addPeriodic(Runnable callback, double periodSeconds) { m_callbacks.add(new Callback(callback, m_startTimeUs, (long) (periodSeconds * 1e6), 0)); } @@ -184,7 +184,7 @@ public final void addPeriodic(Runnable callback, double periodSeconds) { * @param offsetSeconds The offset from the common starting time in seconds. This is useful for * scheduling a callback in a different timeslot relative to TimedRobot. */ - public final void addPeriodic(Runnable callback, double periodSeconds, double offsetSeconds) { + protected final void addPeriodic(Runnable callback, double periodSeconds, double offsetSeconds) { m_callbacks.add( new Callback( callback, m_startTimeUs, (long) (periodSeconds * 1e6), (long) (offsetSeconds * 1e6))); diff --git a/wpilibj/src/main/java/edu/wpi/first/wpilibj/TimesliceRobot.java b/wpilibj/src/main/java/edu/wpi/first/wpilibj/TimesliceRobot.java index 5d10bb57eb3..aa31b03855e 100644 --- a/wpilibj/src/main/java/edu/wpi/first/wpilibj/TimesliceRobot.java +++ b/wpilibj/src/main/java/edu/wpi/first/wpilibj/TimesliceRobot.java @@ -79,7 +79,7 @@ public class TimesliceRobot extends TimedRobot { * @param controllerPeriod The controller period in seconds. The sum of all scheduler allocations * should be less than or equal to this value. */ - public TimesliceRobot(double robotPeriodicAllocation, double controllerPeriod) { + protected TimesliceRobot(double robotPeriodicAllocation, double controllerPeriod) { m_nextOffset = robotPeriodicAllocation; m_controllerPeriod = controllerPeriod; } @@ -95,7 +95,7 @@ public TimesliceRobot(double robotPeriodicAllocation, double controllerPeriod) { * @param func Function to schedule. * @param allocation The function's runtime allocation in seconds out of the controller period. */ - public void schedule(Runnable func, double allocation) { + protected void schedule(Runnable func, double allocation) { if (m_nextOffset + allocation > m_controllerPeriod) { throw new IllegalStateException( "Function scheduled at offset "