Simplify all StandardFirmata examples

[d-a-v]

A new file is added utility/ExampleStandardFirmataCommon.h which is 99% of StandardFirmata.ino (diff -u is given below because github won't show it).

All other StandardFirmata examples were/are based on this file. The goal is to simplify them all as well and obtain a common source code for all "Standard Firmata" examples.
This file can also can be included from user projects without having to copy the full code, which in turn may be improved upstream sometimes (which is always better).

Links to simplified examples (easier to read than looking at "files changed"):
StandardFirmata
StandardFirmataBLE
StandardFirmataChipKIT
StandardFirmataEthernet
StandardFirmataPlus
StandardFirmataWiFi

--- StandardFirmata.ino.orig	2020-05-17 18:21:37.772320715 +0200
+++ ../../utility/ExampleStandardFirmataCommon.h	2020-05-17 18:21:42.180322855 +0200
@@ -11,7 +11,7 @@
   Copyright (C) 2006-2008 Hans-Christoph Steiner.  All rights reserved.
   Copyright (C) 2010-2011 Paul Stoffregen.  All rights reserved.
   Copyright (C) 2009 Shigeru Kobayashi.  All rights reserved.
-  Copyright (C) 2009-2016 Jeff Hoefs.  All rights reserved.
+  Copyright (C) 2009-2017 Jeff Hoefs.  All rights reserved.
 
   This library is free software; you can redistribute it and/or
   modify it under the terms of the GNU Lesser General Public
@@ -23,17 +23,23 @@
   Last updated August 17th, 2017
 */
 
-#include <Servo.h>
-#include <Wire.h>
-#include <Firmata.h>
-
-#define I2C_WRITE                   B00000000
-#define I2C_READ                    B00001000
-#define I2C_READ_CONTINUOUSLY       B00010000
-#define I2C_STOP_READING            B00011000
-#define I2C_READ_WRITE_MODE_MASK    B00011000
-#define I2C_10BIT_ADDRESS_MODE_MASK B00100000
-#define I2C_END_TX_MASK             B01000000
+//#include <Firmata.h> <-- sketch must include this file
+
+#ifndef SERVO
+#define SERVO Servo
+#endif
+
+#ifndef ANALOGWRITE
+#define ANALOGWRITE_PIN_MODE_PWM analogWrite
+#endif
+
+#define I2C_WRITE                   0x00 //B00000000
+#define I2C_READ                    0x08 //B00001000
+#define I2C_READ_CONTINUOUSLY       0x10 //B00010000
+#define I2C_STOP_READING            0x18 //B00011000
+#define I2C_READ_WRITE_MODE_MASK    0x18 //B00011000
+#define I2C_10BIT_ADDRESS_MODE_MASK 0x20 //B00100000
+#define I2C_END_TX_MASK             0x40 //B01000000
 #define I2C_STOP_TX                 1
 #define I2C_RESTART_TX              0
 #define I2C_MAX_QUERIES             8
@@ -42,7 +48,6 @@
 // the minimum interval for sampling analog input
 #define MINIMUM_SAMPLING_INTERVAL   1
 
-
 /*==============================================================================
  * GLOBAL VARIABLES
  *============================================================================*/
@@ -74,7 +79,7 @@
   byte stopTX;
 };
 
-/* for i2c read continuous more */
+/* for i2c read continuous mode */
 i2c_device_info query[I2C_MAX_QUERIES];
 
 byte i2cRxData[64];
@@ -233,7 +238,7 @@
 
 /* -----------------------------------------------------------------------------
  * check all the active digital inputs for change of state, then add any events
- * to the Serial output queue using Serial.print() */
+ * to the output queue using Stream.write() (Serial is a Stream) */
 void checkDigitalInputs(void)
 {
   /* Using non-looping code allows constants to be given to readPort().
@@ -277,7 +282,8 @@
     }
   }
   if (IS_PIN_ANALOG(pin)) {
-    reportAnalogCallback(PIN_TO_ANALOG(pin), mode == PIN_MODE_ANALOG ? 1 : 0); // turn on/off reporting
+    // turn on/off reporting
+    reportAnalogCallback(PIN_TO_ANALOG(pin), mode == PIN_MODE_ANALOG ? 1 : 0);
   }
   if (IS_PIN_DIGITAL(pin)) {
     if (mode == INPUT || mode == PIN_MODE_PULLUP) {
@@ -330,7 +336,7 @@
     case PIN_MODE_PWM:
       if (IS_PIN_PWM(pin)) {
         pinMode(PIN_TO_PWM(pin), OUTPUT);
-        analogWrite(PIN_TO_PWM(pin), 0);
+        ANALOGWRITE_PIN_MODE_PWM(PIN_TO_PWM(pin), 0);
         Firmata.setPinMode(pin, PIN_MODE_PWM);
       }
       break;
@@ -389,7 +395,7 @@
         break;
       case PIN_MODE_PWM:
         if (IS_PIN_PWM(pin))
-          analogWrite(PIN_TO_PWM(pin), value);
+          ANALOGWRITE_PIN_MODE_PWM(PIN_TO_PWM(pin), value);
         Firmata.setPinState(pin, value);
         break;
     }
@@ -753,7 +759,7 @@
   isResetting = false;
 }
 
-void setup()
+void initFirmataCommonBegin()
 {
   Firmata.setFirmwareVersion(FIRMATA_FIRMWARE_MAJOR_VERSION, FIRMATA_FIRMWARE_MINOR_VERSION);
 
@@ -765,42 +771,39 @@
   Firmata.attach(SET_DIGITAL_PIN_VALUE, setPinValueCallback);
   Firmata.attach(START_SYSEX, sysexCallback);
   Firmata.attach(SYSTEM_RESET, systemResetCallback);
+}
 
-  // to use a port other than Serial, such as Serial1 on an Arduino Leonardo or Mega,
-  // Call begin(baud) on the alternate serial port and pass it to Firmata to begin like this:
-  // Serial1.begin(57600);
-  // Firmata.begin(Serial1);
-  // However do not do this if you are using SERIAL_MESSAGE
-
-  Firmata.begin(57600);
-  while (!Serial) {
-    ; // wait for serial port to connect. Needed for ATmega32u4-based boards and Arduino 101
-  }
-
+void initFirmataCommonEnd()
+{
   systemResetCallback();  // reset to default config
 }
 
 /*==============================================================================
  * LOOP()
  *============================================================================*/
-void loop()
+void loopFirmataCommon(bool bleSpecific = false)
 {
   byte pin, analogPin;
 
   /* DIGITALREAD - as fast as possible, check for changes and output them to the
-   * FTDI buffer using Serial.print()  */
+   * Stream buffer using Stream.print()  */
   checkDigitalInputs();
 
   /* STREAMREAD - processing incoming messagse as soon as possible, while still
    * checking digital inputs.  */
-  while (Firmata.available())
+  while (Firmata.available()) {
     Firmata.processInput();
+  }
 
   // TODO - ensure that Stream buffer doesn't go over 60 bytes
 
   currentMillis = millis();
   if (currentMillis - previousMillis > samplingInterval) {
-    previousMillis += samplingInterval;
+    if (bleSpecific) {
+      previousMillis = currentMillis;
+    } else {
+      previousMillis += samplingInterval; // no drift
+    }
     /* ANALOGREAD - do all analogReads() at the configured sampling interval */
     for (pin = 0; pin < TOTAL_PINS; pin++) {
       if (IS_PIN_ANALOG(pin) && Firmata.getPinMode(pin) == PIN_MODE_ANALOG) {

[rwaldron]

Is there a test plan for making sure nothing that relies on these implementations is broken?

[d-a-v]

Is there a test plan for making sure nothing that relies on these implementations is broken?

I can't say, I agree testers are needed. Do you have @rwaldron something in mind ?
@soundanalogous What do you think ?

For the record, I used the meld tool which is of a great help for visually comparing two or three files.
I don't mind at all if this is not merged but I find it would be a pity if this kind of change is not done on this repository by some maintainer.

The other day I hated myself to copy and include the wifi example in one of my projetcs. I would have loved to be able to only add an

#include <utility/ExampleStandardFirmataCommon.h>

and rely on upstream improvement when they happen (in one place).

[soundanalogous]

I agree that duplication of code across all the examples needs to be reduced. However my preferred solution here is to converge ConfigurableFirmata and Firmata so that Firmata uses the same modular approach.