Fix for Classic Controller clones without data mode support (#69)

* Classic: Remove initialization for high res buffer val

This doesn't need a value because it's set in the function. If it's not set in the function we can't know what it is *anyways*, and a default value serves only to confuse.

* Classic: Re-attempt connect if HR write fails

Some Classic clone controllers apparently NACK the register write for "high res" mode which wasn't known when they were first reverse engineered. The result is that the specificInit returns 'false' indicating a communication error and the controller fails to indicate that it's connected (#68).

This commit should hopefully fix that issue by attempting to distinguish between controllers that NACK the register write for "high res" mode because they don't support it, and controllers that NACK the register write because they're no longer connected to the bus. This is done with a simple read of the control data. If we receive data the controller is still present. If not, the mode is unsupported.

Note that this can't distinguish between unsupported data and momentary (sub ms) loss of connection. That's just something we're going to have to live with for the time being...

* Classic: Change HR requests to use controls func.

Better than setting the pointer "manually" here. There should be no change in functionality.

Previously I must have used the generic requestData function because I was trying to read from bytes 7/8 instead of reading the entire control group but some of the clones didn't like that. The proper control data request function is more idiomatic and clearer about what's going on.

Author: dmadison

src/controllers/ClassicController.cpp

@@ -159,14 +159,13 @@ boolean ClassicController_Shared::checkDataMode(boolean *hr) const {
 	 * register-based I2C device and just return junk. So instead we're starting
 	 * at the beginning of the data block.
 	 */
-	static const uint8_t CheckPtr   = 0x00;  // start of the control data block
 	static const uint8_t CheckSize  = 8;     // 8 bytes to cover both std and high res
 	static const uint8_t DataOffset = 0x06;  // start of the data we're interested in (7 / 8)
 	uint8_t checkData[CheckSize] = { 0x00 }, verifyData[CheckSize] = { 0x00 };
 	do {
-		if (!requestData(CheckPtr, CheckSize, checkData)) return false;
+		if (!requestControlData(CheckSize, checkData)) return false;
 		delayMicroseconds(I2C_ConversionDelay);  // need a brief delay between reads
-		if (!requestData(CheckPtr, CheckSize, verifyData)) return false;
+		if (!requestControlData(CheckSize, verifyData)) return false;
 
 		boolean equal = true;
 		for (uint8_t i = 0; i < CheckSize - DataOffset; i++) {
@@ -186,15 +185,38 @@ boolean ClassicController_Shared::checkDataMode(boolean *hr) const {
 
 boolean ClassicController_Shared::setDataMode(boolean hr, boolean verify) {
 	const uint8_t regVal = hr ? 0x03 : 0x01;  // 0x03 for high res, 0x01 for standard
-	if (!writeRegister(0xFE, regVal)) return false;  // write to controller
+
+	// Attempt to write 'high res' mode to controller register.
+	const bool writeSuccess = writeRegister(0xFE, regVal);
+
+	/* If there's no success on the register write there are two possibilities:
+	 *
+	 *   #1: The controller is disconnected and no I2C data can get through
+	 *   #2: The controller is silly and not processing the command properly, 
+	 *       as some clone controllers may
+	 *
+	 * To determine which state we're in we perform a bog-standard data read,
+	 * which is guaranteed to be supported by all controllers. If the controller
+	 * returns data (*any* data) then we're still connected and the controller
+	 * NACK'd the register write. If the controller does *not* return data the I2C
+	 * bus is presumably disconnected and we can return 'false' for a communication
+	 * error.
+	 */
+	if (!writeSuccess) {
+		uint8_t buffer[MinRequestSize];  // we don't care about this data, we just need someplace to dump it
+		if (!requestControlData(MinRequestSize, buffer)) return false;  // bad read, we must be disconnected
+
+		// if we're going to perfom more reads below, the controller needs a short delay to catch its breath
+		if(verify == true) delayMicroseconds(I2C_ConversionDelay);
+	}
 
 	if (verify == true) {
-		boolean currentMode = false;  // check controller's HR setting 
+		boolean currentMode;  // buffer for controller's deduced HR setting, set in the 'check' function
 		if (!checkDataMode(&currentMode)) return false;  // error: could not read mode
 		highRes = currentMode;  // save current mode to class
 	}
 	else {
-		highRes = hr;  // save mode (no verification)
+		highRes = hr;  // save mode we're attempting to set (no verification)
 	}
 
 	if (getHighRes() == true && getRequestSize() < 8) {