Shortened names, minor review comments

src/devices/Arduino/ArduinoAnalogControllerDriver.cs → src/devices/Arduino/ArduinoAnalogDriver.cs

@@ -8,15 +8,15 @@
 
 namespace Iot.Device.Arduino
 {
-    internal class ArduinoAnalogControllerDriver : AnalogControllerDriver
+    internal class ArduinoAnalogDriver : AnalogControllerDriver
     {
         private readonly ArduinoBoard _board;
         private readonly List<SupportedPinConfiguration> _supportedPinConfigurations;
         private readonly Dictionary<int, ValueChangeEventHandler> _callbacks;
         private int _autoReportingReferenceCount;
         private int _firstAnalogPin;
 
-        public ArduinoAnalogControllerDriver(ArduinoBoard board,
+        public ArduinoAnalogDriver(ArduinoBoard board,
             List<SupportedPinConfiguration> supportedPinConfigurations)
         {
             _board = board ?? throw new ArgumentNullException(nameof(board));
@@ -34,7 +34,7 @@ public ArduinoAnalogControllerDriver(ArduinoBoard board,
             }
             else
             {
-                _firstAnalogPin = 0;
+                throw new NotSupportedException("No analog pins found or Firmata firmware has no analog pin support");
             }
         }
 

src/devices/Arduino/ArduinoBoard.cs

@@ -32,6 +32,7 @@ public class ArduinoBoard : IDisposable
         private SerialPort _serialPort;
         private string _serialPortName;
         private int _baudRate;
+        private bool _initialized = false;
 
         /// <summary>
         /// Create an instance of an <see cref="ArduinoBoard"/> using the given stream (typically a serial port connection)
@@ -80,6 +81,11 @@ public ArduinoBoard(string serialPort, int baudRate)
 
         public virtual void Initialize()
         {
+            if (_initialized)
+            {
+                return;
+            }
+
             if (_serialPortStream == null)
             {
                 try
@@ -101,9 +107,9 @@ public virtual void Initialize()
             _firmata.Open(_serialPortStream);
             _firmata.OnError += FirmataOnError;
             var protocolVersion = _firmata.QueryFirmataVersion();
-            if (protocolVersion != _firmata.QuerySupportedFirmataVersion())
+            if (protocolVersion < _firmata.QuerySupportedFirmataVersion())
             {
-                throw new NotSupportedException($"Firmata version on board is {protocolVersion}. Expected {_firmata.QuerySupportedFirmataVersion()}. They must be equal.");
+                throw new NotSupportedException($"Firmata version on board is {protocolVersion}. Expected at least {_firmata.QuerySupportedFirmataVersion()}. They must be equal.");
             }
 
             Log($"Firmata version on board is {protocolVersion}.");
@@ -124,12 +130,15 @@ public virtual void Initialize()
 
             // _firmata.SetSamplingInterval(TimeSpan.FromMilliseconds(100));
             _firmata.EnableDigitalReporting();
+
+            _initialized = true;
         }
 
         public Version FirmwareVersion
         {
             get
             {
+                CheckInitialized();
                 return _firmwareVersion;
             }
         }
@@ -138,6 +147,7 @@ public string FirmwareName
         {
             get
             {
+                CheckInitialized();
                 return _firmwareName;
             }
         }
@@ -146,6 +156,7 @@ internal FirmataDevice Firmata
         {
             get
             {
+                CheckInitialized();
                 return _firmata;
             }
         }
@@ -154,27 +165,35 @@ internal List<SupportedPinConfiguration> SupportedPinConfigurations
         {
             get
             {
+                CheckInitialized();
                 return _supportedPinConfigurations;
             }
         }
 
-        internal void Log(string message)
+        internal void Log(string message, Exception innerException = null)
         {
-            LogMessages?.Invoke(message, null);
+            LogMessages?.Invoke(message, innerException);
         }
 
         private void FirmataOnError(string message, Exception innerException)
         {
-            LogMessages?.Invoke(message, innerException);
+            Log(message, innerException);
+        }
+
+        public GpioController CreateGpioController()
+        {
+            return CreateGpioController(PinNumberingScheme.Logical);
         }
 
         public GpioController CreateGpioController(PinNumberingScheme pinNumberingScheme)
         {
-            return new GpioController(pinNumberingScheme, new ArduinoGpioControllerDriver(this, _supportedPinConfigurations));
+            Initialize();
+            return new GpioController(pinNumberingScheme, new ArduinoGpioDriver(this, _supportedPinConfigurations));
         }
 
         public I2cDevice CreateI2cDevice(I2cConnectionSettings connectionSettings)
         {
+            Initialize();
             return new ArduinoI2cDevice(this, connectionSettings);
         }
 
@@ -186,11 +205,8 @@ public I2cDevice CreateI2cDevice(I2cConnectionSettings connectionSettings)
         /// <returns></returns>
         public SpiDevice CreateSpiDevice(SpiConnectionSettings settings)
         {
-            int mosi = 11;
-            int miso = 12;
-            int sck = 13;
-            return new SoftwareSpi(sck, miso, mosi, settings.ChipSelectLine, settings,
-                CreateGpioController(PinNumberingScheme.Board));
+            Initialize();
+            throw new NotSupportedException("Firmata currently has no SPI support");
         }
 
         public PwmChannel CreatePwmChannel(
@@ -199,12 +215,22 @@ public PwmChannel CreatePwmChannel(
             int frequency = 400,
             double dutyCyclePercentage = 0.5)
         {
+            Initialize();
             return new ArduinoPwmChannel(this, chip, channel, frequency, dutyCyclePercentage);
         }
 
         public AnalogController CreateAnalogController(int chip)
         {
-            return new AnalogController(PinNumberingScheme.Logical, new ArduinoAnalogControllerDriver(this, _supportedPinConfigurations));
+            Initialize();
+            return new AnalogController(PinNumberingScheme.Logical, new ArduinoAnalogDriver(this, _supportedPinConfigurations));
+        }
+
+        private void CheckInitialized()
+        {
+            if (!_initialized)
+            {
+                throw new InvalidOperationException("Device not initialized");
+            }
         }
 
         protected virtual void Dispose(bool disposing)
@@ -232,6 +258,7 @@ protected virtual void Dispose(bool disposing)
             }
 
             _firmata = null;
+            _initialized = false;
         }
 
         public void Dispose()

src/devices/Arduino/ArduinoGpioControllerDriver.cs → src/devices/Arduino/ArduinoGpioDriver.cs

@@ -8,7 +8,7 @@
 #pragma warning disable CS1591
 namespace Iot.Device.Arduino
 {
-    internal class ArduinoGpioControllerDriver : GpioDriver
+    internal class ArduinoGpioDriver : GpioDriver
     {
         private readonly ArduinoBoard _arduinoBoard;
         private readonly List<SupportedPinConfiguration> _supportedPinConfigurations;
@@ -17,7 +17,7 @@ internal class ArduinoGpioControllerDriver : GpioDriver
         private readonly object _callbackContainersLock;
         private readonly AutoResetEvent _waitForEventResetEvent;
 
-        internal ArduinoGpioControllerDriver(ArduinoBoard arduinoBoard, List<SupportedPinConfiguration> supportedPinConfigurations)
+        internal ArduinoGpioDriver(ArduinoBoard arduinoBoard, List<SupportedPinConfiguration> supportedPinConfigurations)
         {
             _arduinoBoard = arduinoBoard ?? throw new ArgumentNullException(nameof(arduinoBoard));
             _supportedPinConfigurations = supportedPinConfigurations ?? throw new ArgumentNullException(nameof(supportedPinConfigurations));

src/devices/Arduino/ArduinoI2cDevice.cs

@@ -36,10 +36,17 @@ public ArduinoI2cDevice(ArduinoBoard board, I2cConnectionSettings connectionSett
 
             ConnectionSettings = connectionSettings;
 
+            int pinsFound = 0;
             // Ensure the corresponding pins are set to I2C (not strictly necessary, but consistent)
             foreach (SupportedPinConfiguration supportedPinConfiguration in _board.SupportedPinConfigurations.Where(x => x.PinModes.Contains(SupportedMode.I2C)))
             {
                 _board.Firmata.SetPinMode(supportedPinConfiguration.Pin, SupportedMode.I2C);
+                pinsFound++;
+            }
+
+            if (pinsFound < 2)
+            {
+                throw new NotSupportedException("Need at least two I2C capable pins. Is I2C support disabled in Firmata?");
             }
 
             // Sometimes, the very first I2C command fails (nothing happens), so try reading a byte
@@ -53,6 +60,7 @@ public ArduinoI2cDevice(ArduinoBoard board, I2cConnectionSettings connectionSett
                 }
                 catch (Exception x) when (x is TimeoutException || x is I2cCommunicationException)
                 {
+                    // Ignore exception, just retry
                 }
             }
 

src/devices/Arduino/ArduinoPwmChannel.cs

@@ -41,7 +41,7 @@ public ArduinoPwmChannel(ArduinoBoard board,
             var caps = board.SupportedPinConfigurations.FirstOrDefault(x => x.Pin == _pin);
             if (caps == null || !caps.PinModes.Contains(SupportedMode.PWM))
             {
-                throw new NotSupportedException($"Pin {_pin} does not support PWM");
+                throw new NotSupportedException($"Pin {_pin} does not support PWM or PWM support disabled in Firmware");
             }
 
             _frequency = frequency;

src/devices/Arduino/FirmataDevice.cs

@@ -19,8 +19,9 @@ namespace Iot.Device.Arduino
 
     internal sealed class FirmataDevice : IDisposable
     {
+        // Firmata version 2.5 or later should work
         private const byte FIRMATA_PROTOCOL_MAJOR_VERSION = 2;
-        private const byte FIRMATA_PROTOCOL_MINOR_VERSION = 6;
+        private const byte FIRMATA_PROTOCOL_MINOR_VERSION = 5;
         private const int FIRMATA_INIT_TIMEOUT_SECONDS = 4;
         private const int MESSAGE_TIMEOUT_MILLIS = 500;
         private byte _firmwareVersionMajor;
@@ -107,22 +108,6 @@ public void Close()
             }
         }
 
-        /// <summary>
-        /// Used where?
-        /// </summary>
-        private void SendString(byte command, string message)
-        {
-            byte[] bytes = Encoding.Unicode.GetBytes(message);
-            lock (_synchronisationLock)
-            {
-                _firmataStream.WriteByte(240);
-                _firmataStream.WriteByte((byte)(command & (uint)sbyte.MaxValue));
-                SendValuesAsTwo7bitBytes(bytes);
-                _firmataStream.WriteByte(247);
-                _firmataStream.Flush();
-            }
-        }
-
         private void StartListening()
         {
             if (_inputThread != null && _inputThread.IsAlive)

src/devices/Arduino/SupportedMode.cs

@@ -4,7 +4,7 @@
     /// Mode bits for the Firmata protocol.
     /// These are used both for capability reporting as well as to set a mode
     /// </summary>
-    public enum SupportedMode
+    internal enum SupportedMode
     {
         /// <summary>
         /// The pin supports digital input