HAL: Minor optimisations

Remove virtual method hasCallback().
Optimise findDevice() method (used by read, write etc.).
Simplify Sensor handling with regard to IO Devices that support callbacks.

IODevice.cpp

@@ -134,7 +134,7 @@ bool IODevice::exists(VPIN vpin) {
 bool IODevice::hasCallback(VPIN vpin) {
   IODevice *dev = findDevice(vpin);
   if (!dev) return false;
-  return dev->_hasCallback(vpin);
+  return dev->_hasCallback;
 }
 
 // Display (to diagnostics) details of the device.
@@ -221,10 +221,12 @@ void IODevice::addDevice(IODevice *newDevice) {
     newDevice->_begin();
 }
 
-// Private helper function to locate a device by VPIN.  Returns NULL if not found
+// Private helper function to locate a device by VPIN.  Returns NULL if not found.
+//  This is performance-critical, so minimises the calculation and function calls necessary.
 IODevice *IODevice::findDevice(VPIN vpin) { 
   for (IODevice *dev = _firstDevice; dev != 0; dev = dev->_nextDevice) {
-    if (dev->owns(vpin)) 
+    VPIN firstVpin = dev->_firstVpin;
+    if (vpin >= firstVpin && vpin < firstVpin+dev->_nPins)
       return dev;
   }
   return NULL;

IODevice.h

@@ -189,15 +189,6 @@ protected:
     (void)vpin; (void)value; (void) param1; (void)param2;
   };
 
-  // Function called to check whether callback notification is supported by this pin.
-  //  Defaults to no, if not overridden by the device.
-  //  The same value should be returned by all pins on the device, so only one need
-  //  be checked.
-  virtual bool _hasCallback(VPIN vpin) { 
-    (void) vpin;
-    return false; 
-  }
-
   // Method to read digital pin state (optionally implemented within device class)
   virtual int _read(VPIN vpin) { 
     (void)vpin; 
@@ -230,6 +221,9 @@ protected:
   VPIN _firstVpin;
   int _nPins;
 
+  // Flag whether the device supports callbacks.
+  bool _hasCallback = false;
+
   // Pin number of interrupt pin for GPIO extender devices.  The extender module will pull this
   //  pin low if an input changes state.
   int16_t _gpioInterruptPin = -1;

IO_GPIOBase.h

@@ -45,10 +45,6 @@ protected:
   int _read(VPIN vpin) override;
   void _display() override;
   void _loop(unsigned long currentMicros) override;
-  bool _hasCallback(VPIN vpin) {
-    (void)vpin;   // suppress compiler warning
-    return true;  // Enable callback if caller wants to use it.
-  }
 
   // Data fields
   uint8_t _I2CAddress; 
@@ -79,12 +75,13 @@ protected:
 
 // Constructor
 template <class T>
-GPIOBase<T>::GPIOBase(FSH *deviceName, VPIN firstVpin, uint8_t nPins, uint8_t I2CAddress, int interruptPin) {
+GPIOBase<T>::GPIOBase(FSH *deviceName, VPIN firstVpin, uint8_t nPins, uint8_t I2CAddress, int interruptPin) :
+  IODevice(firstVpin, nPins)
+{
   _deviceName = deviceName;
-  _firstVpin = firstVpin;
-  _nPins = nPins;
   _I2CAddress = I2CAddress;
   _gpioInterruptPin = interruptPin;
+  _hasCallback = true;
   // Add device to list of devices.
   addDevice(this);
 }

Sensors.cpp

@@ -103,12 +103,6 @@ void Sensor::checkAll(Print *stream){
       // Required time elapsed since last read cycle started,
       // so initiate new scan through the sensor list
       readingSensor = firstSensor;
-#ifdef USE_NOTIFY
-      if (firstSensor == firstPollSensor) 
-        pollSignalPhase = true;
-      else
-        pollSignalPhase = false;
-#endif
       lastReadCycle = thisTime;
     }
   }
@@ -117,12 +111,6 @@ void Sensor::checkAll(Print *stream){
   bool pause = false;
   while (readingSensor != NULL && !pause) {
 
-#ifdef USE_NOTIFY
-    // Check if we have reached the start of the polled portion of the sensor list.
-    if (readingSensor == firstPollSensor)
-      pollSignalPhase = true;
-#endif
-
     // Where the sensor is attached to a pin, read pin status.  For sources such as LCN,
     // which don't have an input pin to read, the LCN class calls setState() to update inputState when
     // a message is received.  The IODevice::read() call returns 1 for active pins (0v) and 0 for inactive (5v).
@@ -130,10 +118,8 @@ void Sensor::checkAll(Print *stream){
     // routine when an input signal change is detected, and this updates the inputState directly,
     // so these inputs don't need to be polled here.
     VPIN pin = readingSensor->data.pin;
-#ifdef USE_NOTIFY
+    if (readingSensor->pollingRequired && pin != VPIN_NONE)
-    if (pollSignalPhase)
+      readingSensor->inputState = IODevice::read(pin);
-#endif
-      if (pin!=VPIN_NONE) readingSensor->inputState = IODevice::read(pin);
 
     // Check if changed since last time, and process changes.
     if (readingSensor->inputState == readingSensor->active) {
@@ -156,22 +142,12 @@ void Sensor::checkAll(Print *stream){
     // Move to next sensor in list.
     readingSensor = readingSensor->nextSensor;
 
-    // Currently process max of 16 sensors per entry for polled sensors, and
+    // Currently process max of 16 sensors per entry.
-    //  16 per entry for sensors notified by callback.
+    // Performance measurements taken during development indicate that, with 128 sensors configured
-    // Performance measurements taken during development indicate that, with 64 sensors configured
+    // on 8x 16-pin MCP23017 GPIO expanders with polling (no change notification), all inputs can be read from the devices
-    // on 8x 8-pin PCF8574 GPIO expanders, all inputs can be read within 1.4ms (400Mhz I2C bus speed), and a
+    // within 1.4ms (400Mhz I2C bus speed), and a full cycle of checking 128 sensors for changes takes under a millisecond.
-    // full cycle of scanning 64 sensors for changes takes between 1.9 and 3.2 milliseconds.
     sensorCount++;
-#ifdef USE_NOTIFY
+    if (sensorCount >= 16) pause = true;
-    if (pollSignalPhase) {
-#endif
-      if (sensorCount >= 16) pause = true;
-#ifdef USE_NOTIFY
-    } else 
-    {
-      if (sensorCount >= 16) pause = true;
-    }
-#endif
   }
 
 } // Sensor::checkAll
@@ -223,23 +199,18 @@ Sensor *Sensor::create(int snum, VPIN pin, int pullUp){
   tt = (Sensor *)calloc(1,sizeof(Sensor));
   if (!tt) return tt;     // memory allocation failure
 
-#ifdef USE_NOTIFY
+  if (pin == VPIN_NONE) 
-  if (pin == VPIN_NONE || IODevice::hasCallback(pin)) {
+    tt->pollingRequired = false;
-    // Callback available, or no pin to read, so link sensor on to the start of the list
+  #ifdef USE_NOTIFY
-    tt->nextSensor = firstSensor;
+  else if (IODevice::hasCallback(pin)) 
-    firstSensor = tt;
+    tt->pollingRequired = false;
-    if (lastSensor == NULL) lastSensor = tt;  // This is only item in list.
+  #endif
-  } else {
+  else 
-    // No callback, so add to end of list so it's polled.
+    tt->pollingRequired = true;
-    if (lastSensor != NULL) lastSensor->nextSensor = tt;
+
-    lastSensor = tt;
+  // Add to the start of the list
-    if (!firstSensor) firstSensor = tt;
-    if (!firstPollSensor) firstPollSensor = tt;
-  }
-#else
   tt->nextSensor = firstSensor;
   firstSensor = tt;
-#endif
 
   tt->data.snum = snum;
   tt->data.pin = pin;
@@ -248,9 +219,8 @@ Sensor *Sensor::create(int snum, VPIN pin, int pullUp){
   tt->inputState = 0;
   tt->latchDelay = minReadCount;
 
-  int params[] = {pullUp};
   if (pin != VPIN_NONE) 
-    IODevice::configure(pin, IODevice::CONFIGURE_INPUT, 1, params);   
+    IODevice::configureInput(pin, pullUp);   
     // Generally, internal pull-up resistors are not, on their own, sufficient 
     // for external infrared sensors --- each sensor must have its own 1K external pull-up resistor
 
@@ -343,6 +313,5 @@ unsigned long Sensor::lastReadCycle=0;
 #ifdef USE_NOTIFY
 Sensor *Sensor::firstPollSensor = NULL;
 Sensor *Sensor::lastSensor = NULL;
-bool Sensor::pollSignalPhase = false;
 bool Sensor::inputChangeCallbackRegistered = false;
 #endif

Sensors.h

@@ -45,14 +45,6 @@ struct SensorData {
 class Sensor{
   // The sensor list is a linked list where each sensor's 'nextSensor' field points to the next.
   //   The pointer is null in the last on the list.
-  //   To partition the sensor into those sensors which require polling through cyclic calls
-  //   to 'IODevice::read(vpin)', and those which support callback on change, 'firstSensor' 
-  //   points to the start of the overall list, and 'lastSensor' points to the end of the list
-  //   (the last sensor object). This structure allows sensors to be added to the start or the
-  //   end of the list easily.  So if an input pin supports change notification, it is placed at the 
-  //   end of the list.  If not, it is placed at the beginning.  And the pointer 'firstPollSensor' 
-  //   is set to the first of the sensor objects that requires scanning.  Thus, we can iterate
-  //   through the whole list, or just through the part that requires scanning.
 
 public:
   SensorData data;
@@ -74,6 +66,7 @@ public:
   // Constructor
   Sensor(); 
   Sensor *nextSensor;
+
   void setState(int state);
   static void load();
   static void store();
@@ -88,9 +81,9 @@ public:
   static const unsigned int minReadCount = 1; // number of additional scans before acting on change
                                         // E.g. 1 means that a change is ignored for one scan and actioned on the next.
                                         // Max value is 63
+  bool pollingRequired = true;
 
 #ifdef USE_NOTIFY
-  static bool pollSignalPhase;
   static void inputChangeCallback(VPIN vpin, int state);
   static bool inputChangeCallbackRegistered;
 #endif