HAL driver enhancements

Performance enhancements in IODevice::loop() function.
Improved error handling, device is placed off line if not responding.
Improved error reporting, device shown as offline if not operational (faulty or not present).

IODevice.cpp

@@ -53,9 +53,7 @@ void IODevice::begin() {
   MCP23017::create(180, 16, 0x21);
 
   // Call the begin() methods of each configured device in turn
-  unsigned long currentMicros = micros();
   for (IODevice *dev=_firstDevice; dev!=NULL; dev = dev->_nextDevice) {
-    dev->_nextEntryTime = currentMicros;
     dev->_begin();
   }
   _initPhase = false;
@@ -69,18 +67,24 @@ void IODevice::begin() {
 // doesn't need to invoke it.
 void IODevice::loop() {
   unsigned long currentMicros = micros();
-  // Call every device's loop function in turn, one per entry.
+  
+  IODevice *lastLoopDevice = _nextLoopDevice;  // So we know when to stop...
+  // Loop through devices until we find one ready to be serviced.
+  do {
     if (!_nextLoopDevice) _nextLoopDevice = _firstDevice;
-  // Check if device exists, and is due to run
+    if (_nextLoopDevice) {
-  if (_nextLoopDevice /* && ((long)(currentMicros-_nextLoopDevice->_nextEntryTime) >= 0) */ ) {
+      if (_nextLoopDevice->_deviceState != DEVSTATE_FAILED 
-    // Move _nextEntryTime on, so that we can guarantee that the device will continue to
+            && ((long)(currentMicros - _nextLoopDevice->_nextEntryTime)) >= 0) {
-    // be serviced if it doesn't update _nextEntryTime.
+        // Found one ready to run, so invoke its _loop method.
         _nextLoopDevice->_nextEntryTime = currentMicros;
-    // Invoke device's _loop function
         _nextLoopDevice->_loop(currentMicros);
-    // Move to next device.
+        _nextLoopDevice = _nextLoopDevice->_nextDevice;
+        break;
+      }
+      // Not this one, move to next one
       _nextLoopDevice = _nextLoopDevice->_nextDevice;
     }
+  } while (_nextLoopDevice != lastLoopDevice); // Stop looking when we've done all.
   
   // Report loop time if diags enabled
 #if defined(DIAG_LOOPTIMES)
@@ -127,7 +131,8 @@ bool IODevice::hasCallback(VPIN vpin) {
 
 // Display (to diagnostics) details of the device.
 void IODevice::_display() {
-  DIAG(F("Unknown device Vpins:%d-%d"), (int)_firstVpin, (int)_firstVpin+_nPins-1);
+  DIAG(F("Unknown device Vpins:%d-%d %S"), 
+    (int)_firstVpin, (int)_firstVpin+_nPins-1, _deviceState==DEVSTATE_FAILED ? F("OFFLINE") : F(""));
 }
 
 // Find device associated with nominated Vpin and pass configuration values on to it.
@@ -151,13 +156,18 @@ void IODevice::write(VPIN vpin, int value) {
 #endif
 }
 
-// Write analogue value to virtual pin(s).  If multiple devices are allocated the same pin
+// Write analogue value to virtual pin(s).  If multiple devices are allocated
-//  then only the first one found will be used.  Duration is the time that the 
+// the same pin then only the first one found will be used.
-//  operation is to be performed over (e.g. as an animation) in deciseconds (0-3276 sec)
+//
-void IODevice::writeAnalogue(VPIN vpin, int value, uint8_t profile, uint16_t duration) {
+// The significance of param1 and param2 may vary from device to device.
+// For servo controllers, param1 is the profile of the transition and param2
+// the duration, i.e. the time that the operation is to be animated over
+// in deciseconds (0-3276 sec)
+//
+void IODevice::writeAnalogue(VPIN vpin, int value, uint8_t param1, uint16_t param2) {
   IODevice *dev = findDevice(vpin);
   if (dev) {
-    dev->_writeAnalogue(vpin, value, profile, duration);
+    dev->_writeAnalogue(vpin, value, param1, param2);
     return;
   }
 #ifdef DIAG_IO
@@ -257,7 +267,7 @@ int IODevice::readAnalogue(VPIN vpin) {
       return dev->_readAnalogue(vpin);
   }
 #ifdef DIAG_IO
-  //DIAG(F("IODevice::readAnalogue(): Vpin %d not found!"), (int)vpin);
+  DIAG(F("IODevice::readAnalogue(): Vpin %d not found!"), (int)vpin);
 #endif
   return false;
 }

IODevice.h

@@ -163,6 +163,13 @@ public:
   
 protected:
   
+  // Constructor
+  IODevice(VPIN firstVpin=0, int nPins=0) {
+    _firstVpin = firstVpin;
+    _nPins = nPins;
+    _nextEntryTime = 0;
+  }
+
  // Method to perform initialisation of the device (optionally implemented within device class)
   virtual void _begin() {}
 
@@ -178,8 +185,8 @@ protected:
   };
 
   // Method to write an 'analogue' value (optionally implemented within device class)
-  virtual void _writeAnalogue(VPIN vpin, int value, uint8_t profile, uint16_t duration) {
+  virtual void _writeAnalogue(VPIN vpin, int value, uint8_t param1, uint16_t param2) {
-    (void)vpin; (void)value; (void) profile; (void)duration;
+    (void)vpin; (void)value; (void) param1; (void)param2;
   };
 
   // Function called to check whether callback notification is supported by this pin.
@@ -275,7 +282,7 @@ private:
   // Device-specific write functions.
   void _write(VPIN vpin, int value) override;
   void _writeAnalogue(VPIN vpin, int value, uint8_t profile, uint16_t duration) override;
-  int _read(VPIN vpin) override; // returns the busy status of the device
+  int _read(VPIN vpin) override; // returns the digital state or busy status of the device
   void _loop(unsigned long currentMicros) override;
   void updatePosition(uint8_t pin);
   void writeDevice(uint8_t pin, int value);
@@ -302,7 +309,6 @@ private:
   static const byte FLASH _bounceProfile[30];
 
   const unsigned int refreshInterval = 50; // refresh every 50ms
-  unsigned long _lastRefreshTime; // last seen value of micros() count
 
   // structures for setting up non-blocking writes to servo controller
   I2CRB requestBlock;

IO_AnalogueInputs.h

@@ -78,11 +78,11 @@ private:
 #endif
     } else {
       DIAG(F("ADS111x device not found, I2C:%x"), _i2cAddress);
+      _deviceState = DEVSTATE_FAILED;
     }
   }
   void _loop(unsigned long currentMicros) override {
 
-    if (currentMicros - _lastMicros >= scanInterval) {
     // Check that previous non-blocking write has completed, if not then wait
     uint8_t status = _i2crb.wait();
     if (status == I2C_STATUS_OK) {
@@ -97,8 +97,10 @@ private:
           #endif
         }
       }
-        if (status != I2C_STATUS_OK) 
+    }
-          DIAG(F("ADS111x I2C:x%d Error:%d"), _i2cAddress, status);
+    if (status != I2C_STATUS_OK) {
+      DIAG(F("ADS111x I2C:x%d Error:%d %S"), _i2cAddress, status, I2CManager.getErrorMessage(status));
+      _deviceState = DEVSTATE_FAILED;
     }
     // Move to next pin
     if (++_currentPin >= _nPins) _currentPin = 0;
@@ -111,15 +113,17 @@ private:
     // Write command, without waiting for completion.
     I2CManager.write(_i2cAddress, _outBuffer, 3, &_i2crb);
 
-      _lastMicros = currentMicros;
+    delayUntil(currentMicros + scanInterval);
-    }
   }
+
   int _readAnalogue(VPIN vpin) override {
     int pin = vpin - _firstVpin;
     return _value[pin];
   }
+  
   void _display() override {
-    DIAG(F("ADS111x I2C:x%x Configured on Vpins:%d-%d"), _i2cAddress, _firstVpin, _firstVpin+_nPins-1);
+    DIAG(F("ADS111x I2C:x%x Configured on Vpins:%d-%d %S"), _i2cAddress, _firstVpin, _firstVpin+_nPins-1,
+      _deviceState == DEVSTATE_FAILED ? F("OFFLINE") : F(""));
   }
 
   // ADC conversion rate is 250SPS, or 4ms per conversion.  Set the period between updates to 10ms. 
@@ -134,7 +138,6 @@ private:
   uint8_t _outBuffer[3];
   uint8_t _inBuffer[2];
   uint8_t _currentPin;  // ADC pin currently being scanned
-  unsigned long _lastMicros = 0;
   I2CRB _i2crb;
 };
 

IO_DFPlayer.h

@@ -66,6 +66,7 @@ private:
   HardwareSerial *_serial;
   bool _playing = false;
   uint8_t _inputIndex = 0;
+  unsigned long _commandSendTime; // Allows timeout processing
 
 public:
   DFPlayer(VPIN firstVpin, int nPins, HardwareSerial &serial) {
@@ -81,21 +82,32 @@ public:
 protected:
   void _begin() override {
     _serial->begin(9600);
-    _display();
+    _deviceState = DEVSTATE_INITIALISING;
+
+    // Send a query to the device to see if it responds
+    sendPacket(0x42); 
+    _commandSendTime = micros();
   }
 
-  void _loop(unsigned long) override {
+  void _loop(unsigned long currentMicros) override {
     // Check for incoming data on _serial, and update busy flag accordingly.
     // Expected message is in the form "7F FF 06 3D xx xx xx xx xx EF"
     while (_serial->available()) {
       int c = _serial->read();
-//      DIAG(F("Received: %x"), c);
       if (c == 0x7E) 
         _inputIndex = 1;
-      else if ((c==0xFF && _inputIndex==1) || (c==0x06 && _inputIndex==2) 
+      else if ((c==0xFF && _inputIndex==1)
-            || (c==0x3D && _inputIndex==3) || (_inputIndex >=4 && _inputIndex <= 8))
+            || (c==0x3D && _inputIndex==3) 
+            || (_inputIndex >=4 && _inputIndex <= 8))
         _inputIndex++;
-      else if (c==0xEF && _inputIndex==9) {
+      else if (c==0x06 && _inputIndex==2) { 
+        // Valid command prefix, so consider the device online.
+        _deviceState = DEVSTATE_NORMAL;
+        #ifdef DIAG_IO
+        _display();
+        #endif
+        _inputIndex++;
+      } else if (c==0xEF && _inputIndex==9) {
         // End of play
         #ifdef DIAG_IO
         DIAG(F("DFPlayer: Finished"));
@@ -104,6 +116,12 @@ protected:
         _inputIndex = 0;
       }
     }
+    // Check if the initial prompt to device has timed out.  Allow 1 second
+    if (_deviceState == DEVSTATE_INITIALISING && currentMicros - _commandSendTime > 1000000UL) {
+      DIAG(F("DFPlayer device not responding on serial port"));
+      _deviceState = DEVSTATE_FAILED;
+    }
+    delayUntil(currentMicros + 10000); // Only enter every 10ms
   }
 
   // Write with value 1 starts playing a song.  The relative pin number is the file number.
@@ -175,7 +193,8 @@ protected:
   }
 
   void _display() override {
-    DIAG(F("DFPlayer Configured on Vpins:%d-%d"), _firstVpin, _firstVpin+_nPins-1);
+    DIAG(F("DFPlayer Configured on Vpins:%d-%d %S"), _firstVpin, _firstVpin+_nPins-1,
+      (_deviceState==DEVSTATE_FAILED) ? F("OFFLINE") : F(""));
   }
   
 private:

IO_GPIOBase.h

@@ -59,7 +59,6 @@ protected:
   T _portPullup;
   // Interval between refreshes of each input port
   static const int _portTickTime = 4000;
-  unsigned long _lastLoopEntry = 0;
 
   // Virtual functions for interfacing with I2C GPIO Device
   virtual void _writeGpioPort() = 0;
@@ -105,10 +104,12 @@ void GPIOBase<T>::_begin() {
     _portMode = 0;  // default to input mode
     _portPullup = -1; // default to pullup enabled
     _portInputState = -1; 
-  }
     _setupDevice();
     _deviceState = DEVSTATE_NORMAL;
-  _lastLoopEntry = micros();
+  } else {
+    DIAG(F("%S I2C:x%x Device not detected"), _deviceName, _I2CAddress);
+    _deviceState = DEVSTATE_FAILED;
+  }
 }
 
 // Configuration parameters for inputs: 
@@ -172,27 +173,25 @@ void GPIOBase<T>::_loop(unsigned long currentMicros) {
     #endif
   }
 
-  // Check if interrupt configured.  If so, and pin is not pulled down, finish.
+  // Check if interrupt configured.  If not, or if it is active (pulled down), then
-  if (_gpioInterruptPin >= 0) {
+  //  initiate a scan.
-    if (digitalRead(_gpioInterruptPin)) return;
+  if (_gpioInterruptPin < 0 || !digitalRead(_gpioInterruptPin)) {
-  } else
-  // No interrupt pin.  Check if tick has elapsed.  If not, finish.
-  if (currentMicros - _lastLoopEntry < (unsigned long)_portTickTime) return;
-
     // TODO: Could suppress reads if there are no pins configured as inputs!
 
     // Read input
-  _lastLoopEntry = currentMicros;
     if (_deviceState == DEVSTATE_NORMAL) {
       _readGpioPort(false);  // Initiate non-blocking read
       _deviceState= DEVSTATE_SCANNING;
     }
   }
+  // Delay next entry until tick elapsed.
+  delayUntil(currentMicros + _portTickTime);
+}
 
 template <class T>
 void GPIOBase<T>::_display() {
-  DIAG(F("%S I2C:x%x Configured on Vpins:%d-%d"), _deviceName, _I2CAddress, 
+  DIAG(F("%S I2C:x%x Configured on Vpins:%d-%d %S"), _deviceName, _I2CAddress, 
-    _firstVpin, _firstVpin+_nPins-1);
+    _firstVpin, _firstVpin+_nPins-1, (_deviceState==DEVSTATE_FAILED) ? F("OFFLINE") : F(""));
 }
 
 template <class T>

IO_HCSR04.h

@@ -68,8 +68,6 @@ private:
   uint16_t _distance;
   // Active=1/inactive=0 state 
   uint8_t _value = 0;
-  // Time of last loop execution
-  unsigned long _lastExecutionTime;
   // Factor for calculating the distance (cm) from echo time (ms).
   //  Based on a speed of sound of 345 metres/second.
   const uint16_t factor = 58; // ms/cm
@@ -97,7 +95,6 @@ protected:
     pinMode(_trigPin, OUTPUT);
     pinMode(_echoPin, INPUT);
     ArduinoPins::fastWriteDigital(_trigPin, 0);
-    _lastExecutionTime = micros();
 #if defined(DIAG_IO)
     _display();
 #endif
@@ -116,13 +113,9 @@ protected:
 
   // _loop function - read HC-SR04 once every 50 milliseconds.
   void _loop(unsigned long currentMicros) override {
-    if (currentMicros - _lastExecutionTime > 50000UL) {
-      _lastExecutionTime = currentMicros;
-
     read_HCSR04device();
     // Delay next loop entry until 50ms have elapsed.
-      //delayUntil(currentMicros + 50000UL);
+    delayUntil(currentMicros + 50000UL);
-    }
   }
 
   void _display() override {

IO_PCA9685.cpp

@@ -107,12 +107,14 @@ void PCA9685::_begin() {
   #if defined(DIAG_IO)
     _display();
   #endif
-  }
+  } else
+    _deviceState = DEVSTATE_FAILED;
 }
 
 // Device-specific write function, invoked from IODevice::write().  
 // For this function, the configured profile is used.
 void PCA9685::_write(VPIN vpin, int value) {
+  if (_deviceState == DEVSTATE_FAILED) return;
   #ifdef DIAG_IO
   DIAG(F("PCA9685 Write Vpin:%d Value:%d"), vpin, value);
   #endif
@@ -137,6 +139,7 @@ void PCA9685::_write(VPIN vpin, int value) {
 //             4 (Bounce)  Servo 'bounces' at extremes.
 //            
 void PCA9685::_writeAnalogue(VPIN vpin, int value, uint8_t profile, uint16_t duration) {
+  if (_deviceState == DEVSTATE_FAILED) return;
   #ifdef DIAG_IO
   DIAG(F("PCA9685 WriteAnalogue Vpin:%d Value:%d Profile:%d Duration:%d"), 
     vpin, value, profile, duration);
@@ -172,6 +175,7 @@ void PCA9685::_writeAnalogue(VPIN vpin, int value, uint8_t profile, uint16_t dur
 // _read returns true if the device is currently in executing an animation, 
 //  changing the output over a period of time.
 int PCA9685::_read(VPIN vpin) {
+  if (_deviceState == DEVSTATE_FAILED) return 0;
   int pin = vpin - _firstVpin;
   struct ServoData *s = _servoData[pin];
   if (s == NULL) 
@@ -181,12 +185,10 @@ int PCA9685::_read(VPIN vpin) {
 }
 
 void PCA9685::_loop(unsigned long currentMicros) {
-  if (currentMicros - _lastRefreshTime >= refreshInterval * 1000) {
   for (int pin=0; pin<_nPins; pin++) {
     updatePosition(pin);
   }
-    _lastRefreshTime = currentMicros;
+  delayUntil(currentMicros + refreshInterval * 1000UL);
-  }
 }
 
 // Private function to reposition servo
@@ -238,7 +240,11 @@ void PCA9685::writeDevice(uint8_t pin, int value) {
   DIAG(F("PCA9685 I2C:x%x WriteDevice Pin:%d Value:%d"), _I2CAddress, pin, value);
   #endif
   // Wait for previous request to complete
-  requestBlock.wait();
+  uint8_t status = requestBlock.wait();
+  if (status != I2C_STATUS_OK) {
+    _deviceState = DEVSTATE_FAILED;
+    DIAG(F("PCA9685 I2C:x%x failed %S"), _I2CAddress, I2CManager.getErrorMessage(status));
+  } else {
     // Set up new request.
     outputBuffer[0] = PCA9685_FIRST_SERVO + 4 * pin;
     outputBuffer[1] = 0;
@@ -247,11 +253,12 @@ void PCA9685::writeDevice(uint8_t pin, int value) {
     outputBuffer[4] = value >> 8;
     I2CManager.queueRequest(&requestBlock);
   }
+}
 
 // Display details of this device.
 void PCA9685::_display() {
-  DIAG(F("PCA9685 I2C:x%x Configured on Vpins:%d-%d"), _I2CAddress, (int)_firstVpin, 
+  DIAG(F("PCA9685 I2C:x%x Configured on Vpins:%d-%d %S"), _I2CAddress, (int)_firstVpin, 
-    (int)_firstVpin+_nPins-1);
+    (int)_firstVpin+_nPins-1, (_deviceState==DEVSTATE_FAILED) ? F("OFFLINE") : F(""));
 }
 
 // Internal helper function for this device

IO_VL53L0X.h

@@ -44,7 +44,11 @@
  * all VL53L0X modules are turned off, the driver works through each module in turn by
  * setting XSHUT to HIGH to turn the module on,, then writing the module's desired I2C address.
  * In this way, many VL53L0X modules can be connected to the one I2C bus, each one 
- * using with a distinct I2C address.
+ * using a distinct I2C address.
+ * 
+ * WARNING:  If the device's XSHUT pin is not connected, then it is very prone to noise, 
+ * and the device may even reset when handled.  If you're not using XSHUT, then it's 
+ * best to tie it to +5V.
  * 
  * The driver is configured as follows:
  * 
@@ -98,7 +102,6 @@ private:
   bool _value;
   bool _initialising = true;
   uint8_t _entryCount = 0;
-  unsigned long _lastEntryTime = 0;
   bool _scanInProgress = false;
   // Register addresses
   enum : uint8_t {
@@ -136,6 +139,7 @@ protected:
       _entryCount = 0;
     }
   }
+
   void _loop(unsigned long currentMicros) override {
     if (_initialising) {
       switch (_entryCount++) {
@@ -156,11 +160,17 @@ protected:
           I2CManager.write(VL53L0X_I2C_DEFAULT_ADDRESS, 2, VL53L0X_REG_I2C_SLAVE_DEVICE_ADDRESS, _i2cAddress);
           break;
         case 3:
+          // After two more loops, check if device has been configured.
           if (I2CManager.exists(_i2cAddress)) {
+            #ifdef DIAG_IO
             _display();
+            #endif
             // Set 2.8V mode
             write_reg(VL53L0X_CONFIG_PAD_SCL_SDA__EXTSUP_HV, 
               read_reg(VL53L0X_CONFIG_PAD_SCL_SDA__EXTSUP_HV) | 0x01);
+          } else {
+            DIAG(F("VL53L0X I2C:x%x device not responding"), _i2cAddress);
+            _deviceState = DEVSTATE_FAILED;
           }
           _initialising = false;
           _entryCount = 0;
@@ -168,13 +178,16 @@ protected:
         default:
           break;
       }
-    } else if (_lastEntryTime - currentMicros > 10000UL) {
+    } else {
-      // Service device every 10ms
-      _lastEntryTime = currentMicros;
 
       if (!_scanInProgress) {
         // Not scanning, so initiate a scan
-        write_reg(VL53L0X_REG_SYSRANGE_START, 0x01);
+        uint8_t status = write_reg(VL53L0X_REG_SYSRANGE_START, 0x01);
+        if (status != I2C_STATUS_OK) {
+          DIAG(F("VL53L0X I2C:x%x Error:%d %S"), _i2cAddress, status, I2CManager.getErrorMessage(status));
+          _deviceState = DEVSTATE_FAILED;
+          _value = false;
+        } else
           _scanInProgress = true;
 
       } else {
@@ -198,8 +211,11 @@ protected:
           _scanInProgress = false;
         }
       }
+      // Next entry in 10 milliseconds.
+      delayUntil(currentMicros + 10000UL);
     }
   }
+
   // For analogue read, first pin returns distance, second pin is signal strength, and third is ambient level.
   int _readAnalogue(VPIN vpin) override {
     int pin = vpin - _firstVpin;
@@ -214,13 +230,16 @@ protected:
         return -1;
     }
   }
+
   // For digital read, return the same value for all pins.
   int _read(VPIN) override {
     return _value;
   }
+
   void _display() override {
-    DIAG(F("VL53L0X I2C:x%x Configured on Vpins:%d-%d On:%dmm Off:%dmm"),
+    DIAG(F("VL53L0X I2C:x%x Configured on Vpins:%d-%d On:%dmm Off:%dmm %S"),
-      _i2cAddress, _firstVpin, _firstVpin+_nPins-1, _onThreshold, _offThreshold);
+      _i2cAddress, _firstVpin, _firstVpin+_nPins-1, _onThreshold, _offThreshold,
+      (_deviceState==DEVSTATE_FAILED) ? F("OFFLINE") : F(""));
   }
   
 
@@ -228,12 +247,12 @@ private:
   inline uint16_t makeuint16(byte lsb, byte msb) {
     return (((uint16_t)msb) << 8) | lsb;
   }
-  void write_reg(uint8_t reg, uint8_t data) {
+  uint8_t write_reg(uint8_t reg, uint8_t data) {
     // write byte to register
     uint8_t outBuffer[2];
     outBuffer[0] = reg;
     outBuffer[1] = data;
-    I2CManager.write(_i2cAddress, outBuffer, 2);
+    return I2CManager.write(_i2cAddress, outBuffer, 2);
   }
   uint8_t read_reg(uint8_t reg) {
     // read byte from register register