fix remaining bugs and improve testing coverage

test/encoder_test/encoder_test.ino

@@ -1,18 +1,16 @@
 /*
- * To run this test suite, you must first install the ArduinoUnit library
- * to your Arduino/libraries/ directory.
- * You can get ArduinoUnit here: https://github.com/mmurdoch/arduinounit
- * Download version 2.0 or greater.
+ * To run this test suite, you must first install to your Arduino/libraries/ directory : 
+ * the ArduinoUnit library to your Arduino/libraries/ directory. 
+ * available  here: https://github.com/mmurdoch/arduinounit (2.0 or greater.)
  */
-
 #include <Wire.h>
 #include <Servo.h>
 #include <ArduinoUnit.h>
 #include "Firmata.h"
 #include "utility/EncoderFirmata.h" 
 
 // overide interrupt macro (for unit tests only)
-#define IS_PIN_INTERRUPT(p)   ((p) == 2 || (p) == 3 ) 
+#define IS_PIN_INTERRUPT(p)   ((p) == 2 || (p) == 3) 
 
 
 FakeStream stream;
@@ -29,15 +27,63 @@ void loop()
   Test::run();
 }
 
-test(handleInterruptPins)
+test(fullTest)
 {
+  Serial.print(F("BT mem.: "));
+  Serial.println(freeMemory());
+
   EncoderFirmata encoder;
-  assertTrue(encoder.handlePinMode(2, INPUT));
-}
 
+  assertTrue(encoder.handlePinMode(2, INPUT));  // 2 is interrupt
+  assertFalse(encoder.handlePinMode(1, INPUT)); // 1 is NOT intterrupt
 
-test(handleNonInterruptPinS)
-{
-  EncoderFirmata encoder;
-  assertFalse(encoder.handlePinMode(4, INPUT));
-}
+  byte encoderNum = 0, pin1 = 2, pin2 = 3;
+
+  byte attachEncoderMessage[] = { ENCODER_ATTACH, encoderNum, pin1, pin2 };
+  byte detachEncoderMessage[] = { ENCODER_DETACH, encoderNum };
+
+  assertFalse(encoder.isEncoderAttached(encoderNum));
+
+  assertTrue(encoder.handleSysex(ENCODER_DATA, 0, attachEncoderMessage));
+
+  assertTrue(encoder.isEncoderAttached(encoderNum));
+
+  assertTrue(encoder.handleSysex(ENCODER_DATA, 0, detachEncoderMessage));
+  assertFalse(encoder.isEncoderAttached(encoderNum));
+
+  encoder.handleSysex(ENCODER_DATA, 0, attachEncoderMessage);
+  assertTrue(encoder.isEncoderAttached(encoderNum));
+  //report encoder position
+  byte reportPosition[] = {ENCODER_REPORT_POSITION, encoderNum};
+  stream.flush();
+  assertTrue(encoder.handleSysex(ENCODER_DATA, 0, reportPosition));
+  assertEqual(stream.bytesWritten().length(), 6);
+
+  byte reportPositions[] = {ENCODER_REPORT_POSITIONS};
+  stream.flush();
+  assertTrue(encoder.handleSysex(ENCODER_DATA, 0, reportPositions));
+  assertEqual(stream.bytesWritten().length(), 9);
+
+
+  // check that auto-report is disable by default
+  assertFalse(encoder.autoReport);
+  stream.flush();
+  encoder.report(); 
+  assertTrue(stream.bytesWritten().length()==0);
+
+
+  //  simulate logical reset
+  encoder.reset();
+  assertFalse(encoder.isEncoderAttached(encoderNum));
+
+  // Enable auto reports
+  byte enableReportsMessage[] = {ENCODER_REPORT_AUTO, 0x01};
+  assertTrue(encoder.handleSysex(ENCODER_DATA, 0, enableReportsMessage));
+  assertTrue(encoder.autoReport);
+  stream.flush();
+  encoder.report();
+  assertEqual(stream.bytesWritten().length(), 3); // i.e : no encoder attached, message is empty (expt sysex start/end and ENCODER_DATA message)
+
+  Serial.print(F("AT mem.: "));
+  Serial.println(freeMemory());
+}

utility/EncoderFirmata.cpp

@@ -28,6 +28,11 @@
 /* Constructor */
 EncoderFirmata::EncoderFirmata()
 {
+  byte encoder;
+  for(encoder=0; encoder<MAX_ENCODERS; encoder++) 
+  {
+    encoders[encoder]=NULL;
+  }
   autoReport = false;
 }
 EncoderFirmata::~EncoderFirmata()
@@ -55,6 +60,7 @@ void EncoderFirmata::detachEncoder(byte encoderNum)
   if (isEncoderAttached(encoderNum)) 
   {
     delete encoders[encoderNum];
+    encoders[encoderNum] = NULL;
   }
 }
 
@@ -124,39 +130,42 @@ boolean EncoderFirmata::handleSysex(byte command, byte argc, byte *argv)
       attachEncoder(encoderNum, pinA, pinB);
       return true;
     }
-    else if (encoderCommand == ENCODER_REPORT_POSITION)
+
+
+    if (encoderCommand == ENCODER_REPORT_POSITION)
     {
       encoderNum = argv[1];
       reportEncoder(encoderNum);
       return true;
     }
-    else if (encoderCommand == ENCODER_REPORT_POSITIONS)
+
+    if (encoderCommand == ENCODER_REPORT_POSITIONS)
     {
       reportEncodersPositions();
       return true;
     }
-    else if (encoderCommand == ENCODER_RESET_POSITION)
+
+    if (encoderCommand == ENCODER_RESET_POSITION)
     {
       encoderNum = argv[1];
       resetEncoderPosition(encoderNum);
       return true;
     }
-    else if (encoderCommand == ENCODER_REPORT_AUTO)
+    if (encoderCommand == ENCODER_REPORT_AUTO)
     {
       enableReports = argv[1];
       toggleAutoReport(enableReports == 0x00 ? false : true);
       return true;
     }
-    else if (encoderCommand == ENCODER_DETACH)
+
+    if (encoderCommand == ENCODER_DETACH)
     {
       encoderNum = argv[1];
       detachEncoder(encoderNum);
       return true;
     }
-    else
-    {
-      Firmata.sendString("Encoder Error: Invalid command");
-    }
+
+    Firmata.sendString("Encoder Error: Invalid command");
   }
   return false;
 }

utility/EncoderFirmata.h

@@ -85,14 +85,14 @@
 #include "Encoder.h"
 
 #define MAX_ENCODERS                5 // arbitrary value, may need to adjust
-#define ENCODER_ATTACH              0x00
-#define ENCODER_REPORT_POSITION     0x01
-#define ENCODER_REPORT_POSITIONS    0x02
-#define ENCODER_RESET_POSITION      0x03
-#define ENCODER_REPORT_AUTO         0x04
-#define ENCODER_DETACH              0x05
-#define ENCODER_DATA                0x61 // TODO : Move to Firmata.h
-#define ENCODER_MESSAGE             0x80 // TODO : Move to Firmata.h
+#define ENCODER_ATTACH              (0x00)
+#define ENCODER_REPORT_POSITION     (0x01)
+#define ENCODER_REPORT_POSITIONS    (0x02)
+#define ENCODER_RESET_POSITION      (0x03)
+#define ENCODER_REPORT_AUTO         (0x04)
+#define ENCODER_DETACH              (0x05)
+#define ENCODER_DATA                (0x61) // TODO : Move to Firmata.h
+#define ENCODER_MESSAGE             (0x80) // TODO : Move to Firmata.h
 
 class EncoderFirmata:public FirmataFeature
 {
@@ -104,7 +104,8 @@ class EncoderFirmata:public FirmataFeature
   boolean handleSysex(byte command, byte argc, byte *argv);
   void reset();
   void report();
-  boolean isEncoderAttached(byte encoderNum);
+  boolean isEncoderAttached(byte encoderNum); 
+  volatile bool autoReport;
 
 private:
   void attachEncoder(byte encoderNum, byte pinANum, byte pinBNum);
@@ -113,8 +114,7 @@ class EncoderFirmata:public FirmataFeature
   void reportEncodersPositions();
   void resetEncoderPosition(byte encoderNum);
   void toggleAutoReport(bool report);
-  Encoder *encoders[MAX_ENCODERS]; 
-  volatile bool autoReport;
+  Encoder *encoders[MAX_ENCODERS];
 };
 
 #endif