Remove detection from Peripheral constructor

Although I think the detector being a part of the Peripheral base class is wise, I'm not a fan of passing the detector pointer through the constructor. If a derived class passes its own detector and a later derived class wants to change that detector for whatever reason, it's blocked. Changing to a protected function makes more sense.

This also in effect removes arbitrary detection support for the generic base classes (Shifter, Pedals, Handbrake, etc.). I have no evidence that that feature was ever used by an end-user, and with the way the library is structured that is better implemented as a custom derived class for their specific device. It can always be added back in later if necessary (via a second constructor taking a pin and declaring an object on the heap, most likely).

Author: dmadison

src/SimRacing.cpp

@@ -430,10 +430,6 @@ void AnalogInput::setCalibration(AnalogInput::Calibration newCal) {
 //                     Peripheral                         #
 //#########################################################
 
-Peripheral::Peripheral(DeviceConnection* detector)
-	: detector(detector)
-{}
-
 bool Peripheral::update() {
 	// if the detector exists, poll for state
 	if (this->detector) {
@@ -457,6 +453,10 @@ bool Peripheral::isConnected() const {
 	return true;
 }
 
+void Peripheral::setDetectPtr(DeviceConnection* d) {
+	this->detector = d;
+}
+
 void Peripheral::setStablePeriod(unsigned long t) {
 	// if detector exists, set the stable period
 	if (this->detector) {
@@ -468,9 +468,8 @@ void Peripheral::setStablePeriod(unsigned long t) {
 //                       Pedals                           #
 //#########################################################
 
-Pedals::Pedals(AnalogInput* dataPtr, uint8_t nPedals, DeviceConnection* detector)
+Pedals::Pedals(AnalogInput* dataPtr, uint8_t nPedals)
 	: 
-	Peripheral(detector),
 	pedalData(dataPtr),
 	NumPedals(nPedals),
 	changed(false)
@@ -676,8 +675,8 @@ void Pedals::serialCalibration(Stream& iface) {
 }
 
 
-TwoPedals::TwoPedals(PinNum gasPin, PinNum brakePin, DeviceConnection* detector)
-	: Pedals(pedalData, NumPedals, detector),
+TwoPedals::TwoPedals(PinNum gasPin, PinNum brakePin)
+	: Pedals(pedalData, NumPedals),
 	pedalData{ AnalogInput(gasPin), AnalogInput(brakePin) }
 {}
 
@@ -687,8 +686,8 @@ void TwoPedals::setCalibration(AnalogInput::Calibration gasCal, AnalogInput::Cal
 }
 
 
-ThreePedals::ThreePedals(PinNum gasPin, PinNum brakePin, PinNum clutchPin, DeviceConnection* detector)
-	: Pedals(pedalData, NumPedals, detector),
+ThreePedals::ThreePedals(PinNum gasPin, PinNum brakePin, PinNum clutchPin)
+	: Pedals(pedalData, NumPedals),
 	pedalData{ AnalogInput(gasPin), AnalogInput(brakePin), AnalogInput(clutchPin) }
 {}
 
@@ -701,9 +700,11 @@ void ThreePedals::setCalibration(AnalogInput::Calibration gasCal, AnalogInput::C
 
 LogitechPedals::LogitechPedals(PinNum gasPin, PinNum brakePin, PinNum clutchPin, PinNum detectPin)
 	: 
-	ThreePedals(gasPin, brakePin, clutchPin, &this->detectObj),
+	ThreePedals(gasPin, brakePin, clutchPin),
 	detectObj(detectPin, false)  // active high
 {
+	this->setDetectPtr(&this->detectObj);
+
 	// taken from calibrating my own pedals. the springs are pretty stiff so while
 	// this covers the whole travel range, users may want to back it down for casual
 	// use (esp. for the brake travel)
@@ -712,9 +713,10 @@ LogitechPedals::LogitechPedals(PinNum gasPin, PinNum brakePin, PinNum clutchPin,
 
 LogitechDrivingForceGT_Pedals::LogitechDrivingForceGT_Pedals(PinNum gasPin, PinNum brakePin, PinNum detectPin)
 	: 
-	TwoPedals(gasPin, brakePin, &this->detectObj),
+	TwoPedals(gasPin, brakePin),
 	detectObj(detectPin, false)  // active high
 {
+	this->setDetectPtr(&this->detectObj);
 	this->setCalibration({ 646, 0 }, { 473, 1023 });  // taken from calibrating my own pedals
 }
 
@@ -723,9 +725,8 @@ LogitechDrivingForceGT_Pedals::LogitechDrivingForceGT_Pedals(PinNum gasPin, PinN
 //                       Shifter                          #
 //#########################################################
 
-Shifter::Shifter(Gear min, Gear max, DeviceConnection* detector)
+Shifter::Shifter(Gear min, Gear max)
 	:
-	Peripheral(detector),
 	MinGear(min), MaxGear(max)
 {
 	this->currentGear = this->previousGear = 0;  // neutral
@@ -814,10 +815,9 @@ const float AnalogShifter::CalEdgeOffset = 0.60;
 
 AnalogShifter::AnalogShifter(
 	Gear gearMin, Gear gearMax,
-	PinNum pinX, PinNum pinY, PinNum pinRev,
-	DeviceConnection* detector
+	PinNum pinX, PinNum pinY, PinNum pinRev
 ) : 
-	Shifter(gearMin, gearMax, detector),
+	Shifter(gearMin, gearMax),
 
 	/* Two axes, X and Y */
 	analogAxis{ AnalogInput(pinX), AnalogInput(pinY) },
@@ -1132,12 +1132,15 @@ void AnalogShifter::serialCalibration(Stream& iface) {
 }
 
 LogitechShifter::LogitechShifter(PinNum pinX, PinNum pinY, PinNum pinRev, PinNum detectPin)
-	: AnalogShifter(
+	: 
+	AnalogShifter(
 		-1, 6,  // includes reverse and gears 1-6
-		pinX, pinY, pinRev, &this->detectObj),
+		pinX, pinY, pinRev
+	),
 
-		detectObj(detectPin, false)  // active high
+	detectObj(detectPin, false)  // active high
 {
+	this->setDetectPtr(&this->detectObj);
 	this->setCalibration({ 490, 440 }, { 253, 799 }, { 262, 86 }, { 460, 826 }, { 470, 76 }, { 664, 841 }, { 677, 77 });
 }
 
@@ -1654,11 +1657,9 @@ void LogitechShifterG25::serialCalibrationSequential(Stream& iface) {
 //                      Handbrake                         #
 //#########################################################
 
-Handbrake::Handbrake(PinNum pinAx, PinNum detectPin, boolean detectActiveLow)
+Handbrake::Handbrake(PinNum pinAx)
 	: 
-	Peripheral(&this->detectObj),
 	analogAxis(pinAx),
-	detectObj(detectPin, detectActiveLow),
 	changed(false)
 {}
 

src/SimRacing.h

@@ -250,14 +250,6 @@ namespace SimRacing {
 	*/
 	class Peripheral {
 	public:
-		/**
-		* Class Constructor
-		* 
-		* @param detector pointer to a device connection instance, to use for
-		*                 determining if the peripheral is connected
-		*/
-		Peripheral(DeviceConnection* detector = nullptr);
-
 		/**
 		* Class destructor
 		*/
@@ -301,6 +293,20 @@ namespace SimRacing {
 		*/
 		virtual bool updateState(bool connected) = 0;
 
+		/**
+		* Sets the pointer to the detector object
+		*
+		* The detector object is used to check if the peripheral is connected
+		* to the microcontroller. The object is polled on every update.
+		*
+		* Although the detector instance is accessed via the Peripheral class,
+		* it is the responsibility of the dervied class to store the
+		* DeviceConnection object and manage its lifetime.
+		*
+		* @param d pointer to the detector object
+		*/
+		void setDetectPtr(DeviceConnection* d);
+
 	private:
 		DeviceConnection* detector;  ///< Pointer to a device connection instance
 	};
@@ -337,12 +343,9 @@ namespace SimRacing {
 		* @param dataPtr  pointer to the analog input data managed by the class,
 		*                 stored elsewhere
 		* @param nPedals  the number of pedals stored in said data pointer
-		* @param detector pointer to a device connection instance, to use for
-		*                 determining if the peripheral is connected
 		*/
 		Pedals(
-			AnalogInput* dataPtr, uint8_t nPedals,
-			DeviceConnection* detector = nullptr
+			AnalogInput* dataPtr, uint8_t nPedals
 		);
 
 		/** @copydoc Peripheral::begin() */
@@ -438,12 +441,9 @@ namespace SimRacing {
 		*
 		* @param pinGas   the analog pin for the gas pedal potentiometer
 		* @param pinBrake the analog pin for the brake pedal potentiometer
-		* @param detector pointer to a device connection instance, to use for
-		*                 determining if the peripheral is connected
 		*/
 		TwoPedals(
-			PinNum pinGas, PinNum pinBrake,
-			DeviceConnection* detector = nullptr
+			PinNum pinGas, PinNum pinBrake
 		);
 
 		/**
@@ -471,12 +471,9 @@ namespace SimRacing {
 		* @param pinGas    the analog pin for the gas pedal potentiometer
 		* @param pinBrake  the analog pin for the brake pedal potentiometer
 		* @param pinClutch the analog pin for the clutch pedal potentiometer
-		* @param detector  pointer to a device connection instance, to use for
-		*                  determining if the peripheral is connected
 		*/
 		ThreePedals(
-			PinNum pinGas, PinNum pinBrake, PinNum pinClutch,
-			DeviceConnection* detector = nullptr
+			PinNum pinGas, PinNum pinBrake, PinNum pinClutch
 		);
 
 		/**
@@ -515,12 +512,10 @@ namespace SimRacing {
 		/**
 		* Class constructor
 		* 
-		* @param min       the lowest gear possible
-		* @param max       the highest gear possible
-		* @param detector  pointer to a device connection instance, to use for
-		*                  determining if the peripheral is connected
+		* @param min the lowest gear possible
+		* @param max the highest gear possible
 		*/
-		Shifter(Gear min, Gear max, DeviceConnection* detector = nullptr);
+		Shifter(Gear min, Gear max);
 
 		/**
 		* Returns the currently selected gear.
@@ -621,13 +616,11 @@ namespace SimRacing {
 		/**
 		* Class constructor
 		* 
-		* @param gearMin  the lowest gear possible
-		* @param gearMax  the highest gear possible
-		* @param pinX     the analog input pin for the X axis
-		* @param pinY     the analog input pin for the Y axis
-		* @param pinRev   the digital input pin for the 'reverse' button
-		* @param detector pointer to a device connection instance, to use for
-		*                 determining if the peripheral is connected
+		* @param gearMin the lowest gear possible
+		* @param gearMax the highest gear possible
+		* @param pinX    the analog input pin for the X axis
+		* @param pinY    the analog input pin for the Y axis
+		* @param pinRev  the digital input pin for the 'reverse' button
 		* 
 		* @note With the way the class is designed, the lowest possible gear is
 		*       -1 (reverse), and the highest possible gear is 6. Setting the
@@ -638,8 +631,7 @@ namespace SimRacing {
 		AnalogShifter(
 			Gear gearMin, Gear gearMax,
 			PinNum pinX, PinNum pinY,
-			PinNum pinRev = UnusedPin,
-			DeviceConnection* detector = nullptr
+			PinNum pinRev = UnusedPin
 		);
 
 		/**
@@ -781,15 +773,9 @@ namespace SimRacing {
 		/**
 		* Class constructor
 		*
-		* @param pinAx           analog pin number for the handbrake axis
-		* @param pinDetect       the digital pin for device detection
-		* @param detectActiveLow whether the device is detected on a high signal (false,
-		*                        default) or a low signal (true)
-		*/
-		Handbrake(
-			PinNum pinAx,
-			PinNum pinDetect = UnusedPin, boolean detectActiveLow = false
-		);
+		* @param pinAx analog pin number for the handbrake axis
+		*/
+		Handbrake(PinNum pinAx);
 
 		/**
 		* Initializes the pin for reading from the handbrake.
@@ -836,7 +822,6 @@ namespace SimRacing {
 
 	private:
 		AnalogInput analogAxis;      ///< axis data for the handbrake's position
-		DeviceConnection detectObj;  ///< detector instance for checking if the handbrake is connected
 		bool changed;                ///< whether the handbrake position has changed since the previous update
 	};