Merge pull request firmata#120 from ntruchsess/encoder_report_changedonly

implement reporting for changed positions only

Author: ntruchsess

Firmata.h

@@ -20,7 +20,7 @@
  * software can test whether it will be compatible with the currently
  * installed firmware. */
 #define FIRMATA_MAJOR_VERSION   2 // for non-compatible changes
-#define FIRMATA_MINOR_VERSION   5 // for backwards compatible changes
+#define FIRMATA_MINOR_VERSION   6 // for backwards compatible changes
 #define FIRMATA_BUGFIX_VERSION  0 // for bugfix releases
 
 #define MAX_DATA_BYTES 64 // max number of data bytes in incoming messages

test/encoder_test/encoder_test.ino

@@ -150,16 +150,19 @@ test(handleReportEncodersPositionsMessage)
 test(enableAutomaticReports)
 {
   EncoderFirmata encoder;
-  assertFalse(encoder.isReportingEnabled());
-  encoder.toggleAutoReport(true);
   assertTrue(encoder.isReportingEnabled());
-  encoder.toggleAutoReport(false);
+  encoder.toggleAutoReport(1);
+  assertTrue(encoder.isReportingEnabled());
+  encoder.toggleAutoReport(2);
+  assertTrue(encoder.isReportingEnabled());
+  encoder.toggleAutoReport(0);
   assertFalse(encoder.isReportingEnabled());
 }
 
 test(handleEnableAutomaticReportsMessage)
 {
   EncoderFirmata encoder;
+  encoder.toggleAutoReport(0);
   assertFalse(encoder.isReportingEnabled());
 
   byte enableMessage[]={ENCODER_REPORT_AUTO, 0x01};
@@ -183,7 +186,7 @@ test(fullReport)
   encoder.report();
   assertEqual(stream.bytesWritten().length(), 0); // reports disable
 
-  encoder.toggleAutoReport(true);
+  encoder.toggleAutoReport(1);
 
   stream.reset();
   encoder.report();

utility/EncoderFirmata.cpp

@@ -21,22 +21,23 @@
 #include <Firmata.h>
 #include "EncoderFirmata.h"
 #include "Encoder.h"
+#include <String.h>
+
+#define isAttached(encoderNum) (encoderNum < MAX_ENCODERS && encoders[encoderNum])
+
+static Encoder *encoders[MAX_ENCODERS];
+static int32_t positions[MAX_ENCODERS];
+static byte autoReport = 0x02;
 
 /* Constructor */
 EncoderFirmata::EncoderFirmata()
 {
-  byte encoder;
-  for(encoder=0; encoder<MAX_ENCODERS; encoder++) 
-  {
-    encoders[encoder]=NULL;
-  }
-  autoReport = false;
-  numEncoders = 0;
+  memset(encoders,0,sizeof(Encoder*)*MAX_ENCODERS);
 }
 
 void EncoderFirmata::attachEncoder(byte encoderNum, byte pinANum, byte pinBNum)
 {
-  if (isEncoderAttached(encoderNum)) 
+  if (isAttached(encoderNum))
   {
     //Firmata.sendString("Encoder Warning: encoder is already attached. Operation cancelled.");
     return;
@@ -49,17 +50,15 @@ void EncoderFirmata::attachEncoder(byte encoderNum, byte pinANum, byte pinBNum)
   Firmata.setPinMode(pinANum, ENCODER);
   Firmata.setPinMode(pinBNum, ENCODER);
   encoders[encoderNum] = new Encoder(pinANum, pinBNum);
-  numEncoders++;
   reportPosition(encoderNum);
 }
 
 void EncoderFirmata::detachEncoder(byte encoderNum)
 {
-  if (isEncoderAttached(encoderNum)) 
+  if (isAttached(encoderNum))
   {
     free(encoders[encoderNum]);
     encoders[encoderNum] = NULL;
-    numEncoders--;
   }
 }
 
@@ -131,8 +130,7 @@ boolean EncoderFirmata::handleSysex(byte command, byte argc, byte *argv)
     }
     if (encoderCommand == ENCODER_REPORT_AUTO)
     {
-      enableReports = argv[1];
-      toggleAutoReport(enableReports == 0x00 ? false : true);
+      autoReport = argv[1];
       return true;
     }
 
@@ -155,38 +153,47 @@ void EncoderFirmata::reset()
   {
     detachEncoder(encoder);
   }
-  autoReport = false;
-  numEncoders= 0;
+  autoReport = 0x02;
 }
 
 void EncoderFirmata::report()
 {
-  if (autoReport && numEncoders>0)
+  if (autoReport > 0)
   {
-    reportPositions();
+    bool report = false;
+    for (uint8_t encoderNum=0; encoderNum < MAX_ENCODERS; encoderNum++)
+    {
+      if (isAttached(encoderNum))
+      {
+        int32_t position = encoders[encoderNum]->read();
+        if ( autoReport == 1 || positions[encoderNum] != position )
+        {
+          if (!report)
+          {
+            Firmata.write(START_SYSEX);
+            Firmata.write(ENCODER_DATA);
+            report = true;
+          }
+          positions[encoderNum] = position;
+          _reportEncoderPosition(encoderNum,position);
+        }
+      }
+    }
+    if (report)
+    {
+      Firmata.write(END_SYSEX);
+    }
   }
 }
 
 boolean EncoderFirmata::isEncoderAttached(byte encoderNum)
 {
-  if (encoderNum>=MAX_ENCODERS)
-  {
-    //Firmata.sendString("Encoder Error: encoder number should be less than 5. Operation cancelled.");
-    return false;
-  }
-  if (encoders[encoderNum]) 
-  {
-    return true;
-  }
-  else
-  {
-    return false;
-  }
+  return isAttached(encoderNum);
 }
 
 void EncoderFirmata::resetPosition(byte encoderNum)
 {
-  if (isEncoderAttached(encoderNum)) 
+  if (isAttached(encoderNum))
   {
     encoders[encoderNum]->write(0);
   }
@@ -195,51 +202,44 @@ void EncoderFirmata::resetPosition(byte encoderNum)
 // Report specify encoder postion using midi protocol
 void EncoderFirmata::reportPosition(byte encoder)
 {
-  if (isEncoderAttached(encoder)) 
+  if (isAttached(encoder))
   {
     Firmata.write(START_SYSEX);
     Firmata.write(ENCODER_DATA);
 
-    _reportEncoderPosition(encoder);
+    _reportEncoderPosition(encoder,encoders[encoder]->read());
 
     Firmata.write(END_SYSEX);
   }
 }
 // Report all attached encoders positions (one message for all encoders) 
 void EncoderFirmata::reportPositions()
 {
-  Firmata.write(START_SYSEX);
-  Firmata.write(ENCODER_DATA);
-  byte encoder;
-  for(encoder=0; encoder<MAX_ENCODERS; encoder++) 
-  {
-    _reportEncoderPosition(encoder);
-  }
-  Firmata.write(END_SYSEX);
+  byte tmpReport = autoReport;
+  autoReport = 1;
+  report();
+  autoReport = tmpReport;
 }
-void EncoderFirmata::_reportEncoderPosition(byte encoder)
+
+void EncoderFirmata::_reportEncoderPosition(byte encoder, int32_t position)
 {
-  if (isEncoderAttached(encoder)) 
-  {
-    signed long position = encoders[encoder]->read();
-    long absValue = abs(position);
-    byte direction = position >= 0 ? 0x00 : 0x01;
-    Firmata.write((direction << 6) | (encoder));
-    Firmata.write((byte)absValue & 0x7F);
-    Firmata.write((byte)(absValue >> 7) & 0x7F);
-    Firmata.write((byte)(absValue >> 14) & 0x7F);
-    Firmata.write((byte)(absValue >> 21) & 0x7F);
-  }
+  long absValue = abs(position);
+  byte direction = position >= 0 ? 0x00 : 0x01;
+  Firmata.write((direction << 6) | (encoder));
+  Firmata.write((byte)absValue & 0x7F);
+  Firmata.write((byte)(absValue >> 7) & 0x7F);
+  Firmata.write((byte)(absValue >> 14) & 0x7F);
+  Firmata.write((byte)(absValue >> 21) & 0x7F);
 }
 
-
-void EncoderFirmata::toggleAutoReport(bool report)
+void EncoderFirmata::toggleAutoReport(byte report)
 {
   autoReport = report;
 }
 
 bool EncoderFirmata::isReportingEnabled()
 {
-  return autoReport;
+  return autoReport > 0;
 }
 
+

utility/EncoderFirmata.h

@@ -29,7 +29,7 @@
 #include "FirmataFeature.h"
 
 // This optional setting causes Encoder to use more optimized code
-// safe iif 'attachInterrupt' is never used in the same time
+// safe if 'attachInterrupt' is never used in the same time
 //#define ENCODER_OPTIMIZE_INTERRUPTS // => not compiling
 #include "Encoder.h"
 
@@ -61,14 +61,11 @@ class EncoderFirmata:public FirmataFeature
   void reportPosition(byte encoderNum);
   void reportPositions();
   void resetPosition(byte encoderNum);
-  void toggleAutoReport(bool report);
+  void toggleAutoReport(byte report);
   bool isReportingEnabled();
 
 private:
-  Encoder *encoders[MAX_ENCODERS];
-  void _reportEncoderPosition(byte encoder);
-  volatile bool autoReport;
-  byte numEncoders; 
+  static void _reportEncoderPosition(byte encoder, int32_t position);
 };
 
-#endif
+#endif