Fix GPIO Expander initial output state.

Previously, pullups were enabled on GPIO Expander digital pins by default, even if the pin was only ever used as an output.  This could lead to a spurious HIGH state being seen by external equipment before the output is initialised to LOW.  To avoid this, the pin pullup is now not enabled until a configure or read operation is issued for the pin.

IODevice.h

@@ -120,7 +120,7 @@ public:
   }
 
   // User-friendly function for configuring a servo pin.
-  inline static bool configureServo(VPIN vpin, uint16_t activePosition, uint16_t inactivePosition, uint8_t profile, uint16_t duration, uint8_t initialState=0) {
+  inline static bool configureServo(VPIN vpin, uint16_t activePosition, uint16_t inactivePosition, uint8_t profile=0, uint16_t duration=0, uint8_t initialState=0) {
     int params[] = {(int)activePosition, (int)inactivePosition, profile, (int)duration, initialState};
     return IODevice::configure(vpin, CONFIGURE_SERVO, 5, params);
   }
@@ -203,7 +203,7 @@ protected:
 
   // Method to perform updates on an ongoing basis (optionally implemented within device class)
   virtual void _loop(unsigned long currentMicros) {
-    (void)currentMicros; // Suppress compiler warning.
+    delayUntil(currentMicros + 0x7fffffff); // Largest time in the future!  Effectively disable _loop calls.
   };
 
   // Method for displaying info on DIAG output (optionally implemented within device class)

IO_GPIOBase.h

@@ -53,6 +53,7 @@ protected:
   T _portOutputState;
   T _portMode;
   T _portPullup;
+  T _portInUse;
   // Interval between refreshes of each input port
   static const int _portTickTime = 4000;
 
@@ -100,7 +101,8 @@ void GPIOBase<T>::_begin() {
 #endif
     _portMode = 0;  // default to input mode
     _portPullup = -1; // default to pullup enabled
-    _portInputState = -1; 
+    _portInputState = -1;
+    _portInUse = 0;
     _setupDevice();
     _deviceState = DEVSTATE_NORMAL;
   } else {
@@ -126,11 +128,15 @@ bool GPIOBase<T>::_configure(VPIN vpin, ConfigTypeEnum configType, int paramCoun
     _portPullup |= mask;
   else
     _portPullup &= ~mask;
+  // Mark that port has been accessed
+  _portInUse |= mask;
+  // Set input mode
+  _portMode &= ~mask;
 
   // Call subclass's virtual function to write to device
+  _writePortModes();
   _writePullups();
-  // Re-read port following change
-  _readGpioPort();
+  // Port change will be notified on next loop entry.
 
   return true;
 }
@@ -149,6 +155,8 @@ void GPIOBase<T>::_loop(unsigned long currentMicros) {
         I2CManager.getErrorMessage(status));
     }
     _processCompletion(status);
+  // Set unused pin and write mode pin value to 1
+    _portInputState |= ~_portInUse | _portMode;
 
     // Scan for changes in input states and invoke callback (if present)
     T differences = lastPortStates ^ _portInputState;
@@ -199,8 +207,9 @@ void GPIOBase<T>::_write(VPIN vpin, int value) {
   DIAG(F("%S I2C:x%x Write Pin:%d Val:%d"), _deviceName, _I2CAddress, pin, value);
   #endif
 
-  // Set port mode output
+  // Set port mode output if currently not output mode
   if (!(_portMode & mask)) {
+    _portInUse |= mask;
     _portMode |= mask;
     _writePortModes();
   }
@@ -220,12 +229,16 @@ int GPIOBase<T>::_read(VPIN vpin) {
   int pin = vpin - _firstVpin;
   T mask = 1 << pin;
 
-  // Set port mode to input
-  if (_portMode & mask) {
+  // Set port mode to input if currently output or first use
+  if ((_portMode | ~_portInUse) & mask) {
     _portMode &= ~mask;
+    _portInUse |= mask;
+    _writePullups();
     _writePortModes();
     // Port won't have been read yet, so read it now.
     _readGpioPort();
+  // Set unused pin and write mode pin value to 1
+    _portInputState |= ~_portInUse | _portMode;
     #ifdef DIAG_IO
     DIAG(F("%S I2C:x%x PortStates:%x"), _deviceName, _I2CAddress, _portInputState);
     #endif

IO_MCP23008.h

@@ -42,14 +42,18 @@ private:
     I2CManager.write(_I2CAddress, 2, REG_GPIO, _portOutputState);
   }
   void _writePullups() override {
-    I2CManager.write(_I2CAddress, 2, REG_GPPU, _portPullup);  
+    // Set pullups only for in-use pins.  This prevents pullup being set for a pin that
+    //  is intended for use as an output but hasn't been written to yet.
+    I2CManager.write(_I2CAddress, 2, REG_GPPU, _portPullup & _portInUse);  
   }
   void _writePortModes() override {
-    // Each bit is 1 for an input, 0 for an output, i.e. inverted.
-    I2CManager.write(_I2CAddress, 2, REG_IODIR, ~_portMode);
-    // Enable interrupt-on-change for pins that are inputs (_portMode=0)
+    // Write 0 to IODIR for in-use pins that are outputs, 1 for others.
+    uint8_t temp = ~(_portMode & _portInUse);
+    I2CManager.write(_I2CAddress, 2, REG_IODIR, temp);
+    // Enable interrupt-on-change for in-use pins that are inputs (_portMode=0)
+    temp = ~_portMode & _portInUse;
     I2CManager.write(_I2CAddress, 2, REG_INTCON, 0x00);
-    I2CManager.write(_I2CAddress, 2, REG_GPINTEN, ~_portMode);
+    I2CManager.write(_I2CAddress, 2, REG_GPINTEN, temp);
   }
   void _readGpioPort(bool immediate) override {
     if (immediate) {

IO_MCP23017.h

@@ -48,14 +48,19 @@ private:
     I2CManager.write(_I2CAddress, 3, REG_GPIOA, _portOutputState, _portOutputState>>8);
   }
   void _writePullups() override {
-    I2CManager.write(_I2CAddress, 3, REG_GPPUA, _portPullup, _portPullup>>8);  
+    // Set pullups only for in-use pins.  This prevents pullup being set for a pin that
+    //  is intended for use as an output but hasn't been written to yet.
+    uint16_t temp = _portPullup & _portInUse;
+    I2CManager.write(_I2CAddress, 3, REG_GPPUA, temp, temp>>8);  
   }
   void _writePortModes() override {
-    // Write 1 to IODIR for pins that are inputs, 0 for outputs (i.e. _portMode inverted)
-    I2CManager.write(_I2CAddress, 3, REG_IODIRA, ~_portMode, (~_portMode)>>8);
-    // Enable interrupt for those pins which are inputs (_portMode=0)
+    // Write 0 to IODIR for in-use pins that are outputs, 1 for others.
+    uint16_t temp = ~(_portMode & _portInUse);
+    I2CManager.write(_I2CAddress, 3, REG_IODIRA, temp, temp>>8);
+    // Enable interrupt for in-use pins which are inputs (_portMode=0)
+    temp = ~_portMode & _portInUse;
     I2CManager.write(_I2CAddress, 3, REG_INTCONA, 0x00, 0x00);
-    I2CManager.write(_I2CAddress, 3, REG_GPINTENA, ~_portMode, (~_portMode)>>8);
+    I2CManager.write(_I2CAddress, 3, REG_GPINTENA, temp, temp>>8);
   }
   void _readGpioPort(bool immediate) override {
     if (immediate) {

IO_PCF8574.h

@@ -17,6 +17,24 @@
  *  along with CommandStation.  If not, see <https://www.gnu.org/licenses/>.
  */
 
+/* 
+ * The PCF8574 is a simple device; it only has one register.  The device 
+ * input/output mode and pullup are configured through this, and the 
+ * output state is written and the input state read through it too.
+ * 
+ * This is accomplished by having a weak resistor in series with the output,
+ * and a read-back of the other end of the resistor.  As an output, the 
+ * pin state is set to 1 or 0, and the output voltage goes to +5V or 0V
+ * (through the weak resistor).
+ * 
+ * In order to use the pin as an input, the output is written as
+ * a '1' in order to pull up the resistor.  Therefore the input will be
+ * 1 unless the pin is pulled down externally, in which case it will be 0.
+ * 
+ * As a consequence of this approach, it is not possible to use the device for
+ * inputs without pullups.
+ */
+
 #ifndef IO_PCF8574_H
 #define IO_PCF8574_H
 
@@ -70,7 +88,7 @@ private:
     if (status == I2C_STATUS_OK) 
       _portInputState = ((uint16_t)inputBuffer[0]) & 0xff;
     else  
-      _portInputState = 0xff;
+      _portInputState = 0xff; 
   }
 
   // Set up device ports