Address code review comments/concerns

Author: zfields

FirmataMarshaller.cpp

@@ -124,7 +124,7 @@ void FirmataMarshaller::end(void)
 void FirmataMarshaller::reportAnalogDisable(uint8_t pin)
 const
 {
-    reportAnalog(pin, false);
+  reportAnalog(pin, false);
 }
 
 /**
@@ -133,13 +133,11 @@ const
  * (16384). To increase the pin range or value, see the documentation for the EXTENDED_ANALOG
  * message.
  * @param pin The analog pin for which to request the value (limited to pins 0 - 15).
- * @param stream_enable A zero value will disable the stream, a non-zero will enable the stream
- * @note The maximum resulting value is 14-bits (16384).
  */
 void FirmataMarshaller::reportAnalogEnable(uint8_t pin)
 const
 {
-    reportAnalog(pin, true);
+  reportAnalog(pin, true);
 }
 
 /**
@@ -148,12 +146,11 @@ const
  * @param portNumber The port number for which to request the value. Note that this is not the same as a "port" on the
  * physical microcontroller. Ports are defined in order per every 8 pins in ascending order
  * of the Arduino digital pin numbering scheme. Port 0 = pins D0 - D7, port 1 = pins D8 - D15, etc.
- * @param stream_enable A zero value will disable the stream, a non-zero will enable the stream
  */
 void FirmataMarshaller::reportDigitalPortDisable(uint8_t portNumber)
 const
 {
-    reportDigitalPort(portNumber, false);
+  reportDigitalPort(portNumber, false);
 }
 
 /**
@@ -162,12 +159,11 @@ const
  * @param portNumber The port number for which to request the value. Note that this is not the same as a "port" on the
  * physical microcontroller. Ports are defined in order per every 8 pins in ascending order
  * of the Arduino digital pin numbering scheme. Port 0 = pins D0 - D7, port 1 = pins D8 - D15, etc.
- * @param stream_enable A zero value will disable the stream, a non-zero will enable the stream
  */
 void FirmataMarshaller::reportDigitalPortEnable(uint8_t portNumber)
 const
 {
-    reportDigitalPort(portNumber, true);
+  reportDigitalPort(portNumber, true);
 }
 
 /**
@@ -188,10 +184,21 @@ const
   sendValueAsTwo7bitBytes(value);
 }
 
+/**
+ * Send an analog mapping query to the Firmata host application. The resulting sysex message will
+ * have an ANALOG_MAPPING_RESPONSE command byte, followed by a list of pins [0-n]; where each
+ * pin will specify its corresponding analog pin number or 0x7F (127) if not applicable.
+ */
+void FirmataMarshaller::sendAnalogMappingQuery(void)
+const
+{
+  sendSysex(ANALOG_MAPPING_QUERY, 0, NULL);
+}
+
 /**
  * Send a capability query to the Firmata host application. The resulting sysex message will have
  * a CAPABILITY_RESPONSE command byte, followed by a list of byte tuples (mode and mode resolution)
- * for each pin; where each pin list is terminated by 0x7F (128).
+ * for each pin; where each pin list is terminated by 0x7F (127).
  */
 void FirmataMarshaller::sendCapabilityQuery(void)
 const

FirmataMarshaller.h

@@ -41,6 +41,7 @@ class FirmataMarshaller
     void reportDigitalPortDisable(uint8_t portNumber) const;
     void reportDigitalPortEnable(uint8_t portNumber) const;
     void sendAnalog(uint8_t pin, uint16_t value) const;
+    void sendAnalogMappingQuery(void) const;
     void sendCapabilityQuery(void) const;
     void sendDigital(uint8_t pin, uint8_t value) const;
     void sendDigitalPort(uint8_t portNumber, uint16_t portData) const;

FirmataParser.cpp

@@ -98,35 +98,35 @@ void FirmataParser::parse(uint8_t inputData)
       switch (executeMultiByteCommand) {
         case ANALOG_MESSAGE:
           if (currentAnalogCallback) {
-            (*currentAnalogCallback)(this,
+            (*currentAnalogCallback)(currentAnalogCallbackContext,
                                      multiByteChannel,
                                      (dataBuffer[0] << 7)
                                      + dataBuffer[1]);
           }
           break;
         case DIGITAL_MESSAGE:
           if (currentDigitalCallback) {
-            (*currentDigitalCallback)(this,
+            (*currentDigitalCallback)(currentDigitalCallbackContext,
                                       multiByteChannel,
                                       (dataBuffer[0] << 7)
                                       + dataBuffer[1]);
           }
           break;
         case SET_PIN_MODE:
           if (currentPinModeCallback)
-            (*currentPinModeCallback)(this, dataBuffer[1], dataBuffer[0]);
+            (*currentPinModeCallback)(currentPinModeCallbackContext, dataBuffer[1], dataBuffer[0]);
           break;
         case SET_DIGITAL_PIN_VALUE:
           if (currentPinValueCallback)
-            (*currentPinValueCallback)(this, dataBuffer[1], dataBuffer[0]);
+            (*currentPinValueCallback)(currentPinValueCallbackContext, dataBuffer[1], dataBuffer[0]);
           break;
         case REPORT_ANALOG:
           if (currentReportAnalogCallback)
-            (*currentReportAnalogCallback)(this, multiByteChannel, dataBuffer[0]);
+            (*currentReportAnalogCallback)(currentReportAnalogCallbackContext, multiByteChannel, dataBuffer[0]);
           break;
         case REPORT_DIGITAL:
           if (currentReportDigitalCallback)
-            (*currentReportDigitalCallback)(this, multiByteChannel, dataBuffer[0]);
+            (*currentReportDigitalCallback)(currentReportDigitalCallbackContext, multiByteChannel, dataBuffer[0]);
           break;
       }
       executeMultiByteCommand = 0;
@@ -162,7 +162,7 @@ void FirmataParser::parse(uint8_t inputData)
         break;
       case REPORT_VERSION:
         if (currentReportVersionCallback)
-          (*currentReportVersionCallback)(this);
+          (*currentReportVersionCallback)(currentReportVersionCallbackContext);
         break;
     }
   }
@@ -207,7 +207,9 @@ int FirmataParser::setDataBufferOfSize(uint8_t * dataBuffer, size_t dataBufferSi
  * DIGITAL_MESSAGE, REPORT_ANALOG, REPORT DIGITAL, SET_PIN_MODE and SET_DIGITAL_PIN_VALUE).
  * @param command The ID of the command to attach a callback function to.
  * @param newFunction A reference to the callback function to attach.
- * @param context The context for the callback function.
+ * @param context An optional context to be provided to the callback function (NULL by default).
+ * @note The context parameter is provided so you can pass a parameter, by reference, to
+ *       your callback function.
  */
 void FirmataParser::attach(uint8_t command, callbackFunction newFunction, void * context)
 {
@@ -244,7 +246,9 @@ void FirmataParser::attach(uint8_t command, callbackFunction newFunction, void *
  * and SYSTEM_RESET).
  * @param command The ID of the command to attach a callback function to.
  * @param newFunction A reference to the callback function to attach.
- * @param context The context for the callback function.
+ * @param context An optional context to be provided to the callback function (NULL by default).
+ * @note The context parameter is provided so you can pass a parameter, by reference, to
+ *       your callback function.
  */
 void FirmataParser::attach(uint8_t command, systemCallbackFunction newFunction, void * context)
 {
@@ -268,7 +272,9 @@ void FirmataParser::attach(uint8_t command, systemCallbackFunction newFunction,
  * Attach a callback function for the STRING_DATA command.
  * @param command Must be set to STRING_DATA or it will be ignored.
  * @param newFunction A reference to the string callback function to attach.
- * @param context The context for the callback function.
+ * @param context An optional context to be provided to the callback function (NULL by default).
+ * @note The context parameter is provided so you can pass a parameter, by reference, to
+ *       your callback function.
  */
 void FirmataParser::attach(uint8_t command, stringCallbackFunction newFunction, void * context)
 {
@@ -284,7 +290,9 @@ void FirmataParser::attach(uint8_t command, stringCallbackFunction newFunction,
  * Attach a generic sysex callback function to sysex command.
  * @param command The ID of the command to attach a callback function to.
  * @param newFunction A reference to the sysex callback function to attach.
- * @param context The context for the callback function.
+ * @param context An optional context to be provided to the callback function (NULL by default).
+ * @note The context parameter is provided so you can pass a parameter, by reference, to
+ *       your callback function.
  */
 void FirmataParser::attach(uint8_t command, sysexCallbackFunction newFunction, void * context)
 {
@@ -296,7 +304,9 @@ void FirmataParser::attach(uint8_t command, sysexCallbackFunction newFunction, v
 /**
  * Attach a buffer overflow callback
  * @param newFunction A reference to the buffer overflow callback function to attach.
- * @param context The context supplied by the end-user, and provided during the execution of the callback
+ * @param context An optional context to be provided to the callback function (NULL by default).
+ * @note The context parameter is provided so you can pass a parameter, by reference, to
+ *       your callback function.
  */
 void FirmataParser::attach(dataBufferOverflowCallbackFunction newFunction, void * context)
 {
@@ -383,7 +393,7 @@ void FirmataParser::processSysexMessage(void)
   switch (dataBuffer[0]) { //first byte in buffer is command
     case REPORT_FIRMWARE:
       if (currentReportFirmwareCallback)
-        (*currentReportFirmwareCallback)(this);
+        (*currentReportFirmwareCallback)(currentReportFirmwareCallbackContext);
       break;
     case STRING_DATA:
       if (currentStringCallback) {
@@ -405,12 +415,12 @@ void FirmataParser::processSysexMessage(void)
         if (dataBuffer[j - 1] != '\0') {
           bufferDataAtPosition('\0', j);
         }
-        (*currentStringCallback)(this, (char *)&dataBuffer[0]);
+        (*currentStringCallback)(currentStringCallbackContext, (char *)&dataBuffer[0]);
       }
       break;
     default:
       if (currentSysexCallback)
-        (*currentSysexCallback)(this, dataBuffer[0], sysexBytesRead - 1, dataBuffer + 1);
+        (*currentSysexCallback)(currentSysexCallbackContext, dataBuffer[0], sysexBytesRead - 1, dataBuffer + 1);
   }
 }
 
@@ -434,5 +444,5 @@ void FirmataParser::systemReset(void)
   sysexBytesRead = 0;
 
   if (currentSystemResetCallback)
-    (*currentSystemResetCallback)(this);
+    (*currentSystemResetCallback)(currentSystemResetCallbackContext);
 }

FirmataParser.h

@@ -40,11 +40,11 @@ class FirmataParser
     int setDataBufferOfSize(uint8_t * dataBuffer, size_t dataBufferSize);
 
     /* attach & detach callback functions to messages */
-    void attach(uint8_t command, callbackFunction newFunction, void * context);
-    void attach(dataBufferOverflowCallbackFunction newFunction, void * context);
-    void attach(uint8_t command, stringCallbackFunction newFunction, void * context);
-    void attach(uint8_t command, sysexCallbackFunction newFunction, void * context);
-    void attach(uint8_t command, systemCallbackFunction newFunction, void * context);
+    void attach(uint8_t command, callbackFunction newFunction, void * context = NULL);
+    void attach(dataBufferOverflowCallbackFunction newFunction, void * context = NULL);
+    void attach(uint8_t command, stringCallbackFunction newFunction, void * context = NULL);
+    void attach(uint8_t command, sysexCallbackFunction newFunction, void * context = NULL);
+    void attach(uint8_t command, systemCallbackFunction newFunction, void * context = NULL);
     void detach(uint8_t command);
     void detach(dataBufferOverflowCallbackFunction);