move csActiveState to spiBeginTransaction and misc refactoring

Author: soundanalogous

examples/StandardFirmataPlus/StandardFirmataPlus.ino

@@ -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
@@ -20,7 +20,7 @@
 
   See file LICENSE.txt for further informations on licensing terms.
 
-  Last updated January 7th, 2017
+  Last updated January 8th, 2017
 */
 
 /*
@@ -384,11 +384,6 @@ void setPinModeCallback(byte pin, int mode)
     case PIN_MODE_SERIAL:
 #ifdef FIRMATA_SERIAL_FEATURE
       serialFeature.handlePinMode(pin, PIN_MODE_SERIAL);
-#endif
-      break;
-    case PIN_MODE_SPI:
-#ifdef FIRMATA_SPI_FEATURE
-      spiFeature.handlePinMode(pin, PIN_MODE_SPI);
 #endif
       break;
     default:

utility/SPIFirmata.cpp

@@ -9,28 +9,30 @@
 
   See file LICENSE.txt for further informations on licensing terms.
 
-  Last updated January 7th, 2017
+  Last updated January 8th, 2017
 */
 
 #include "SPIFirmata.h"
 
 SPIFirmata::SPIFirmata()
+{
+  init();
+}
+
+void SPIFirmata::init()
 {
   mDeviceId = 0;
   mCsPin = -1;
+  mCsActiveState = SPI_CS_ACTIVE_LOW; // default
 }
 
-boolean SPIFirmata::handlePinMode(byte pin, int mode)
+bool SPIFirmata::handlePinMode(uint8_t pin, int mode)
 {
-  // ignore SS pin for now
-  if (mode == PIN_MODE_SPI && pin != SS) {
-    Firmata.setPinMode(pin, PIN_MODE_SPI);
-    return true;
-  }
+  // There is no reason for a user to manually set the SPI pin modes
   return false;
 }
 
-void SPIFirmata::handleCapability(byte pin)
+void SPIFirmata::handleCapability(uint8_t pin)
 {
   // ignore SS pin for now
   if (IS_PIN_SPI(pin) && pin != SS) {
@@ -41,30 +43,43 @@ void SPIFirmata::handleCapability(byte pin)
   }
 }
 
-boolean SPIFirmata::handleSysex(byte command, byte argc, byte *argv)
+bool SPIFirmata::handleSysex(uint8_t command, uint8_t argc, uint8_t *argv)
 {
   if (command == SPI_DATA) {
-    byte mode = argv[0];
+    uint8_t mode = argv[0];
     // not using channel yet
-    byte channel = argv[1] & SPI_CHANNEL_MASK;
+    uint8_t channel = argv[1] & SPI_CHANNEL_MASK;
 
     switch (mode) {
       case SPI_CONFIG:
         SPI.begin();
+        // SPI pin states are configured by SPI.begin, but we still register them with Firmata.
+        Firmata.setPinMode(MOSI, PIN_MODE_SPI);
+        Firmata.setPinMode(MISO, PIN_MODE_SPI);
+        Firmata.setPinMode(SCK, PIN_MODE_SPI);
+        // ignore SS for now
+        //Firmata.setPinMode(SS, PIN_MODE_SPI);
         break;
       case SPI_BEGIN_TRANSACTION:
       {
         mDeviceId = argv[1] >> 2;
         uint8_t bitOrder = argv[2] & SPI_BIT_ORDER_MASK;
         uint8_t dataMode = argv[2] >> 1;
-        uint32_t clockSpeed = (uint32_t)argv[3] | ((uint32_t)argv[4] << 7) | ((uint32_t)argv[5] << 14) |
-                          ((uint32_t)argv[6] << 21) | ((uint32_t)argv[7] << 28);
+        uint32_t clockSpeed = (uint32_t)argv[3] | ((uint32_t)argv[4] << 7) |
+            ((uint32_t)argv[5] << 14) | ((uint32_t)argv[6] << 21) | ((uint32_t)argv[7] << 28);
 
         if (argc > 8) {
           mCsPin = argv[8];
           pinMode(mCsPin, OUTPUT);
-          // TODO - need to know if the device is active LOW or active HIGH at this time.
-          digitalWrite(mCsPin, HIGH);
+
+          if (argv[9] != END_SYSEX) {
+            mCsActiveState = argv[9];
+          } else {
+            // set default
+            mCsActiveState = SPI_CS_ACTIVE_LOW;
+          }
+          // set CS pin to opposite of active state
+          digitalWrite(mCsPin, !mCsActiveState);
 
           // protect the CS pin
           // TODO - decide if this is the best approach. If PIN_MODE_SPI is set, the user cannot
@@ -81,34 +96,28 @@ boolean SPIFirmata::handleSysex(byte command, byte argc, byte *argv)
         break;
       case SPI_TRANSFER:
       {
-        byte transferOptions = argv[2] & SPI_TRANSFER_OPTS_MASK;
-        byte numBytes = argv[3];
+        uint8_t transferOptions = argv[2] & SPI_TRANSFER_MODES_MASK;
+        uint8_t numBytes = argv[3];
 
-        boolean csIsActive = true;
-        byte csStartVal = LOW;
-        byte csEndVal = HIGH;
-        boolean csStartOnly = false;
-        boolean csEndOnly = false;
+        bool csIsActive = true;
+        bool csStartOnly = false;
+        bool csEndOnly = false;
 
-        //boolean csToggle = false;
+        //bool csToggle = false;
 
         if (mCsPin >= 0) {
           if (argv[2] & SPI_CS_DISABLE_MASK) {
             csIsActive = false;
           } else {
             if (argv[2] & SPI_CS_START_ONLY_MASK) csStartOnly = true;
             if (argv[2] & SPI_CS_END_ONLY_MASK) csEndOnly = true;
-            if (argv[2] & SPI_CS_ACTIVE_EDGE_MASK) {
-              csStartVal = HIGH;
-              csStartVal = LOW;
-            }
             // TODO - handle csToggle
             // if (argv[2] & SPI_CS_TOGGLE_MASK) csToggle = true;
           }
         }
 
-        if ((csIsActive || csStartOnly) && !csEndOnly) {
-          digitalWrite(mCsPin, csStartVal);
+        if (mCsPin >= 0 && csIsActive && !csEndOnly) {
+          digitalWrite(mCsPin, mCsActiveState);
         }
 
         if (transferOptions == SPI_READ_WRITE) {
@@ -122,8 +131,9 @@ boolean SPIFirmata::handleSysex(byte command, byte argc, byte *argv)
           Firmata.sendString("No transferOptions set");
         }
 
-        if ((csIsActive || csEndOnly) && !csStartOnly) {
-          digitalWrite(mCsPin, csEndVal);
+        // TODO - ideally this should be called before the SPI_REPLY message is sent.
+        if (mCsPin >= 0 && csIsActive && !csStartOnly) {
+          digitalWrite(mCsPin, !mCsActiveState);
         }
         break; // SPI_TRANSFER
       }
@@ -138,20 +148,19 @@ boolean SPIFirmata::handleSysex(byte command, byte argc, byte *argv)
 
 void SPIFirmata::reset()
 {
-  mCsPin = -1;
-  mDeviceId = 0;
+  init();
 }
 
-void SPIFirmata::readWrite(byte channel, byte numBytes, byte argc, byte *argv)
+void SPIFirmata::readWrite(uint8_t channel, uint8_t numBytes, uint8_t argc, uint8_t *argv)
 {
-  byte offset = 4; // mode + channel + opts + numBytes
-  byte buffer[numBytes];
-  byte bufferIndex = 0;
+  uint8_t offset = 4; // mode + channel + opts + numBytes
+  uint8_t buffer[numBytes];
+  uint8_t bufferIndex = 0;
   if (numBytes * 2 != argc - offset) {
     // TODO - handle error
     Firmata.sendString("fails numBytes test");
   }
-  for (byte i = 0; i < numBytes * 2; i += 2) {
+  for (uint8_t i = 0; i < numBytes * 2; i += 2) {
     bufferIndex = (i + 1) / 2;
     buffer[bufferIndex] = argv[i + offset + 1] << 7 | argv[i + offset];
   }
@@ -160,38 +169,38 @@ void SPIFirmata::readWrite(byte channel, byte numBytes, byte argc, byte *argv)
   reply(channel, numBytes, buffer);
 }
 
-void SPIFirmata::writeOnly(byte channel, byte numBytes, byte argc, byte *argv)
+void SPIFirmata::writeOnly(uint8_t channel, uint8_t numBytes, uint8_t argc, uint8_t *argv)
 {
-  byte offset = 4;
-  byte txValue;
+  uint8_t offset = 4;
+  uint8_t txValue;
   if (numBytes * 2 != argc - offset) {
     // TODO - handle error
     Firmata.sendString("fails numBytes test");
   }
-  for (byte i = 0; i < numBytes * 2; i += 2) {
+  for (uint8_t i = 0; i < numBytes * 2; i += 2) {
     txValue = argv[i + offset + 1] << 7 | argv[i + offset];
     SPI.transfer(txValue);
   }
 }
 
-void SPIFirmata::readOnly(byte channel, byte numBytes)
+void SPIFirmata::readOnly(uint8_t channel, uint8_t numBytes)
 {
-  byte replyData[numBytes];
-  for (byte i = 0; i < numBytes; i++) {
+  uint8_t replyData[numBytes];
+  for (uint8_t i = 0; i < numBytes; i++) {
     replyData[i] = SPI.transfer(0x00);
   }
   reply(channel, numBytes, replyData);
 }
 
-void SPIFirmata::reply(byte channel, byte numBytes, byte *buffer)
+void SPIFirmata::reply(uint8_t channel, uint8_t numBytes, uint8_t *buffer)
 {
   Firmata.write(START_SYSEX);
   Firmata.write(SPI_DATA);
   Firmata.write(SPI_REPLY);
   Firmata.write(mDeviceId << 2 | channel);
   Firmata.write(numBytes);
 
-  for (byte i = 0; i < numBytes; i++) {
+  for (uint8_t i = 0; i < numBytes; i++) {
     Firmata.write(buffer[i] & 0x7F);
     Firmata.write(buffer[i] >> 7 & 0x7F);
   }

utility/SPIFirmata.h

@@ -9,7 +9,7 @@
 
   See file LICENSE.txt for further informations on licensing terms.
 
-  Last updated January 7th, 2017
+  Last updated January 8th, 2017
 */
 
 #ifndef SPIFirmata_h
@@ -35,7 +35,7 @@
 #define SPI_END                     0x05
 
 // transfer options
-#define SPI_TRANSFER_OPTS_MASK      0x03
+#define SPI_TRANSFER_MODES_MASK     0x03
 #define SPI_READ_WRITE              0x00
 #define SPI_READ_ONLY               0x01
 #define SPI_WRITE_ONLY              0x02
@@ -44,13 +44,15 @@
 #define SPI_CS_DISABLE_MASK         0x04
 #define SPI_CS_START_ONLY_MASK      0x08
 #define SPI_CS_END_ONLY_MASK        0x10
-#define SPI_CS_ACTIVE_EDGE_MASK     0x20
-#define SPI_CS_TOGGLE_MASK          0x40
+#define SPI_CS_TOGGLE_MASK          0x20
 
 #define SPI_CHANNEL_MASK            0x03
 #define SPI_DEVICE_ID_MASK          0x7C
 #define SPI_BIT_ORDER_MASK          0x01
 
+#define SPI_CS_ACTIVE_LOW           0x00
+#define SPI_CS_ACTIVE_HIGH          0x01
+
 namespace {
 // TODO - check Teensy and other non SAM, SAMD or AVR variants
 #if defined(ARDUINO_ARCH_SAMD) || defined(ARDUINO_ARCH_SAM)
@@ -82,19 +84,23 @@ class SPIFirmata: public FirmataFeature
 {
   public:
     SPIFirmata();
-    boolean handlePinMode(byte pin, int mode);
-    void handleCapability(byte pin);
-    boolean handleSysex(byte command, byte argc, byte *argv);
+    bool handlePinMode(uint8_t pin, int mode);
+    void handleCapability(uint8_t pin);
+    bool handleSysex(uint8_t command, uint8_t argc, uint8_t *argv);
     void reset();
 
   private:
-    signed char mCsPin;
-    byte mDeviceId;
+    // TODO - each of these properties should be in an array of channels
+    int8_t mCsPin;
+    bool mCsActiveState;
+    uint8_t mDeviceId;
+
+    void init();
 
-    void readWrite(byte channel, byte numBytes, byte argc, byte *argv);
-    void readOnly(byte channel, byte numBytes);
-    void writeOnly(byte channel, byte numBytes, byte argc, byte *argv);
-    void reply(byte channel, byte numBytes, byte *buffer);
+    void readWrite(uint8_t channel, uint8_t numBytes, uint8_t argc, uint8_t *argv);
+    void readOnly(uint8_t channel, uint8_t numBytes);
+    void writeOnly(uint8_t channel, uint8_t numBytes, uint8_t argc, uint8_t *argv);
+    void reply(uint8_t channel, uint8_t numBytes, uint8_t *buffer);
 };
 
 #endif /* SPIFirmata_h */