From d66c61a36e6d2c9264cd73d288ee98b8909fd5af Mon Sep 17 00:00:00 2001 From: Tyler Veness Date: Sun, 19 Jun 2016 00:13:18 -0700 Subject: [PATCH] Cleaned up robot startup and cleanup/shutdown code (#77) Cleaned up RobotBase, removed singleton list from SensorBase, and removed unused typedefs and NULL_TASK macro from HAL's Task.hpp. Making the robot class instance static fixed non-POD statics used by the instance during destruction from being destroyed first. --- hal/include/HAL/Task.h | 11 -------- wpilibc/athena/include/RobotBase.h | 25 +++++------------ wpilibc/athena/src/DriverStation.cpp | 6 ++-- wpilibc/athena/src/RobotBase.cpp | 42 ---------------------------- wpilibc/athena/src/SensorBase.cpp | 32 --------------------- wpilibc/shared/include/SensorBase.h | 8 ------ wpilibc/sim/include/RobotBase.h | 19 +++++-------- wpilibc/sim/src/DriverStation.cpp | 2 -- wpilibc/sim/src/RobotBase.cpp | 23 --------------- wpilibc/sim/src/SensorBase.cpp | 32 --------------------- 10 files changed, 16 insertions(+), 184 deletions(-) diff --git a/hal/include/HAL/Task.h b/hal/include/HAL/Task.h index 2420d76eb0..e69b64fae7 100644 --- a/hal/include/HAL/Task.h +++ b/hal/include/HAL/Task.h @@ -10,16 +10,6 @@ #include #include -#ifndef _FUNCPTR_DEFINED -#define _FUNCPTR_DEFINED -#ifdef __cplusplus -typedef int (*FUNCPTR)(...); -/* ptr to function returning int */ -#else -typedef int (*FUNCPTR)(); /* ptr to function returning int */ -#endif /* __cplusplus */ -#endif /* _FUNCPTR_DEFINED */ - #ifndef _STATUS_DEFINED #define _STATUS_DEFINED typedef int STATUS; @@ -32,7 +22,6 @@ typedef int STATUS; #define ERROR (-1) #endif /* ERROR */ -#define NULL_TASK NULL typedef pthread_t* TASK; extern "C" { diff --git a/wpilibc/athena/include/RobotBase.h b/wpilibc/athena/include/RobotBase.h index 724dae135d..29d3ac1c39 100644 --- a/wpilibc/athena/include/RobotBase.h +++ b/wpilibc/athena/include/RobotBase.h @@ -7,8 +7,10 @@ #pragma once +#include +#include + #include "Base.h" -#include "Task.h" class DriverStation; @@ -20,9 +22,9 @@ class DriverStation; } \ HALReport(HALUsageReporting::kResourceType_Language, \ HALUsageReporting::kLanguage_CPlusPlus); \ - _ClassName_* robot = new _ClassName_(); \ - RobotBase::robotSetup(robot); \ - return 0; \ + static _ClassName_ robot; \ + std::printf("\n********** Robot program starting **********\n"); \ + robot.StartCompetition(); \ } /** @@ -35,34 +37,21 @@ class DriverStation; * then killed at the end of the Autonomous period. */ class RobotBase { - friend class RobotDeleter; - public: - static RobotBase& getInstance(); - static void setInstance(RobotBase* robot); - bool IsEnabled() const; bool IsDisabled() const; bool IsAutonomous() const; bool IsOperatorControl() const; bool IsTest() const; bool IsNewDataAvailable() const; - static void startRobotTask(FUNCPTR factory); - static void robotTask(FUNCPTR factory, Task* task); virtual void StartCompetition() = 0; - static void robotSetup(RobotBase* robot); - protected: RobotBase(); - virtual ~RobotBase(); + virtual ~RobotBase() = default; RobotBase(const RobotBase&) = delete; RobotBase& operator=(const RobotBase&) = delete; - Task* m_task = nullptr; DriverStation& m_ds; - - private: - static RobotBase* m_instance; }; diff --git a/wpilibc/athena/src/DriverStation.cpp b/wpilibc/athena/src/DriverStation.cpp index c80ddd5747..7fd36e2b10 100644 --- a/wpilibc/athena/src/DriverStation.cpp +++ b/wpilibc/athena/src/DriverStation.cpp @@ -58,8 +58,6 @@ DriverStation::DriverStation() { // It will signal when new packet data is available. HALSetNewDataSem(&m_packetDataAvailableCond); - AddToSingletonList(); - m_task = Task("DriverStation", &DriverStation::Run, this); } @@ -104,8 +102,8 @@ void DriverStation::Run() { * @return Pointer to the DS instance */ DriverStation& DriverStation::GetInstance() { - static DriverStation* instance = new DriverStation(); - return *instance; + static DriverStation instance; + return instance; } /** diff --git a/wpilibc/athena/src/RobotBase.cpp b/wpilibc/athena/src/RobotBase.cpp index fc74151627..75bef37041 100644 --- a/wpilibc/athena/src/RobotBase.cpp +++ b/wpilibc/athena/src/RobotBase.cpp @@ -17,20 +17,6 @@ #include "Utility.h" #include "networktables/NetworkTable.h" -RobotBase* RobotBase::m_instance = nullptr; - -void RobotBase::setInstance(RobotBase* robot) { - wpi_assert(m_instance == nullptr); - m_instance = robot; -} - -RobotBase& RobotBase::getInstance() { return *m_instance; } - -void RobotBase::robotSetup(RobotBase* robot) { - std::printf("\n********** Robot program starting **********\n"); - robot->StartCompetition(); -} - /** * Constructor for a generic robot program. * @@ -46,8 +32,6 @@ RobotBase::RobotBase() : m_ds(DriverStation::GetInstance()) { RobotState::SetImplementation(DriverStation::GetInstance()); HLUsageReporting::SetImplementation(new HardwareHLReporting()); - RobotBase::setInstance(this); - NetworkTable::SetNetworkIdentity("Robot"); NetworkTable::SetPersistentFilename("/home/lvuser/networktables.ini"); @@ -60,18 +44,6 @@ RobotBase::RobotBase() : m_ds(DriverStation::GetInstance()) { } } -/** - * Free the resources for a RobotBase class. - * This includes deleting all classes that might have been allocated as - * Singletons to they would never be deleted except here. - */ -RobotBase::~RobotBase() { - SensorBase::DeleteSingletons(); - delete m_task; - m_task = nullptr; - m_instance = nullptr; -} - /** * Determine if the Robot is currently enabled. * @return True if the Robot is currently enabled by the field controls. @@ -111,17 +83,3 @@ bool RobotBase::IsTest() const { return m_ds.IsTest(); } * function was called? */ bool RobotBase::IsNewDataAvailable() const { return m_ds.IsNewControlData(); } - -/** - * This class exists for the sole purpose of getting its destructor called when - * the module unloads. - * Before the module is done unloading, we need to delete the RobotBase derived - * singleton. This should delete the other remaining singletons that were - * registered. This should also stop all tasks that are using the Task class. - */ -class RobotDeleter { - public: - RobotDeleter() {} - ~RobotDeleter() { delete &RobotBase::getInstance(); } -}; -static RobotDeleter g_robotDeleter; diff --git a/wpilibc/athena/src/SensorBase.cpp b/wpilibc/athena/src/SensorBase.cpp index 8c578e3114..a8df7f1937 100644 --- a/wpilibc/athena/src/SensorBase.cpp +++ b/wpilibc/athena/src/SensorBase.cpp @@ -19,7 +19,6 @@ const uint32_t SensorBase::kPwmChannels; const uint32_t SensorBase::kRelayChannels; const uint32_t SensorBase::kPDPChannels; const uint32_t SensorBase::kChassisSlots; -SensorBase* SensorBase::m_singletonList = nullptr; static bool portsInitialized = false; void* SensorBase::m_digital_ports[kDigitalChannels]; @@ -57,37 +56,6 @@ SensorBase::SensorBase() { } } -/** - * Add sensor to the singleton list. - * - * Add this sensor to the list of singletons that need to be deleted when - * the robot program exits. Each of the sensors on this list are singletons, - * that is they aren't allocated directly with new, but instead are allocated - * by the static GetInstance method. As a result, they are never deleted when - * the program exits. Consequently these sensors may still be holding onto - * resources and need to have their destructors called at the end of the - * program. - */ -void SensorBase::AddToSingletonList() { - m_nextSingleton = m_singletonList; - m_singletonList = this; -} - -/** - * Delete all the singleton classes on the list. - * - * All the classes that were allocated as singletons need to be deleted so - * their resources can be freed. - */ -void SensorBase::DeleteSingletons() { - for (SensorBase* next = m_singletonList; next != nullptr;) { - SensorBase* tmp = next; - next = next->m_nextSingleton; - delete tmp; - } - m_singletonList = nullptr; -} - /** * Check that the solenoid module number is valid. * diff --git a/wpilibc/shared/include/SensorBase.h b/wpilibc/shared/include/SensorBase.h index 87c7abd856..07fc6bf67f 100644 --- a/wpilibc/shared/include/SensorBase.h +++ b/wpilibc/shared/include/SensorBase.h @@ -25,8 +25,6 @@ class SensorBase : public ErrorBase { SensorBase(const SensorBase&) = delete; SensorBase& operator=(const SensorBase&) = delete; - static void DeleteSingletons(); - static uint32_t GetDefaultSolenoidModule() { return 0; } static bool CheckSolenoidModule(uint8_t moduleNumber); @@ -49,13 +47,7 @@ class SensorBase : public ErrorBase { static const uint32_t kChassisSlots = 8; protected: - void AddToSingletonList(); - static void* m_digital_ports[kDigitalChannels]; static void* m_relay_ports[kRelayChannels]; static void* m_pwm_ports[kPwmChannels]; - - private: - static SensorBase* m_singletonList; - SensorBase* m_nextSingleton = nullptr; }; diff --git a/wpilibc/sim/include/RobotBase.h b/wpilibc/sim/include/RobotBase.h index 588a3bcc22..ee503438db 100644 --- a/wpilibc/sim/include/RobotBase.h +++ b/wpilibc/sim/include/RobotBase.h @@ -7,15 +7,18 @@ #pragma once +#include + #include "Base.h" #include "DriverStation.h" #include "simulation/MainNode.h" #include "simulation/simTime.h" -#define START_ROBOT_CLASS(_ClassName_) \ - int main() { \ - (new _ClassName_())->StartCompetition(); \ - return 0; \ +#define START_ROBOT_CLASS(_ClassName_) \ + int main() { \ + static _ClassName_ robot; \ + std::printf("\n********** Robot program starting **********\n"); \ + robot.StartCompetition(); \ } /** @@ -29,12 +32,7 @@ * then killed at the end of the Autonomous period. */ class RobotBase { - friend class RobotDeleter; - public: - static RobotBase& getInstance(); - static void setInstance(RobotBase* robot); - bool IsEnabled() const; bool IsDisabled() const; bool IsAutonomous() const; @@ -51,7 +49,4 @@ class RobotBase { DriverStation& m_ds; transport::SubscriberPtr time_sub; - - private: - static RobotBase* m_instance; }; diff --git a/wpilibc/sim/src/DriverStation.cpp b/wpilibc/sim/src/DriverStation.cpp index 2e83b941e4..cd0a57d4ef 100644 --- a/wpilibc/sim/src/DriverStation.cpp +++ b/wpilibc/sim/src/DriverStation.cpp @@ -51,8 +51,6 @@ DriverStation::DriverStation() { joysticks[5] = msgs::FRCJoystickPtr(new msgs::FRCJoystick()); joysticksSub[5] = MainNode::Subscribe( "~/ds/joysticks/5", &DriverStation::joystickCallback5, this); - - AddToSingletonList(); } /** diff --git a/wpilibc/sim/src/RobotBase.cpp b/wpilibc/sim/src/RobotBase.cpp index 16677f65dd..3812b5a380 100644 --- a/wpilibc/sim/src/RobotBase.cpp +++ b/wpilibc/sim/src/RobotBase.cpp @@ -11,15 +11,6 @@ #include -RobotBase* RobotBase::m_instance = nullptr; - -void RobotBase::setInstance(RobotBase* robot) { - wpi_assert(m_instance == nullptr); - m_instance = robot; -} - -RobotBase& RobotBase::getInstance() { return *m_instance; } - /** * Constructor for a generic robot program. * @@ -73,17 +64,3 @@ bool RobotBase::IsOperatorControl() const { return m_ds.IsOperatorControl(); } * field controls. */ bool RobotBase::IsTest() const { return m_ds.IsTest(); } - -/** - * This class exists for the sole purpose of getting its destructor called when - * the module unloads. - * - * Before the module is done unloading, we need to delete the RobotBase derived - * singleton. This should delete the other remaining singletons that were - * registered. This should also stop all tasks that are using the Task class. - */ -class RobotDeleter { - public: - ~RobotDeleter() { delete &RobotBase::getInstance(); } -}; -static RobotDeleter g_robotDeleter; diff --git a/wpilibc/sim/src/SensorBase.cpp b/wpilibc/sim/src/SensorBase.cpp index b97691a3d4..d2ee427d06 100644 --- a/wpilibc/sim/src/SensorBase.cpp +++ b/wpilibc/sim/src/SensorBase.cpp @@ -17,44 +17,12 @@ const uint32_t SensorBase::kPwmChannels; const uint32_t SensorBase::kRelayChannels; const uint32_t SensorBase::kPDPChannels; const uint32_t SensorBase::kChassisSlots; -SensorBase* SensorBase::m_singletonList = nullptr; /** * Creates an instance of the sensor base and gets an FPGA handle */ SensorBase::SensorBase() {} -/** - * Add sensor to the singleton list. - * - * Add this sensor to the list of singletons that need to be deleted when - * the robot program exits. Each of the sensors on this list are singletons, - * that is they aren't allocated directly with new, but instead are allocated - * by the static GetInstance method. As a result, they are never deleted when - * the program exits. Consequently these sensors may still be holding onto - * resources and need to have their destructors called at the end of the - * program. - */ -void SensorBase::AddToSingletonList() { - m_nextSingleton = m_singletonList; - m_singletonList = this; -} - -/** - * Delete all the singleton classes on the list. - * - * All the classes that were allocated as singletons need to be deleted so - * their resources can be freed. - */ -void SensorBase::DeleteSingletons() { - for (SensorBase* next = m_singletonList; next != nullptr;) { - SensorBase* tmp = next; - next = next->m_nextSingleton; - delete tmp; - } - m_singletonList = nullptr; -} - /** * Check that the solenoid module number is valid. *