Merge branch 'rotary-encoder-send-position' of https://github.com/DCC-EX/CommandStation-EX into rotary-encoder-send-position

DCCTimerAVR.cpp

@@ -199,7 +199,7 @@ void ADCee::scan() {
     waiting = false;
     id++;
     mask = mask << 1;
-    if (id > highestPin) { // the 1 has been shifted out
+    if (id > highestPin) {
       id = 0;
       mask = 1;
     }
@@ -230,8 +230,8 @@ void ADCee::scan() {
       id++;
       mask = mask << 1;
       if (id > highestPin) {
-	id = 0;
-	mask = 1;
+      	id = 0;
+	      mask = 1;
       }
     }
   }

DCCTimerSTM32.cpp

@@ -36,23 +36,25 @@
 #include "DIAG.h"
 
 #if defined(ARDUINO_NUCLEO_F411RE)
-// Nucleo-64 boards don't have Serial1 defined by default
+// Nucleo-64 boards don't have additional serial ports defined by default
 HardwareSerial Serial1(PB7, PA15);  // Rx=PB7, Tx=PA15 -- CN7 pins 17 and 21 - F411RE
 // Serial2 is defined to use USART2 by default, but is in fact used as the diag console
 // via the debugger on the Nucleo-64. It is therefore unavailable for other DCC-EX uses like WiFi, DFPlayer, etc.
 // Let's define Serial6 as an additional serial port (the only other option for the Nucleo-64s)
-HardwareSerial Serial3(PA12, PA11);  // Rx=PA12, Tx=PA11 -- CN10 pins 12 and 14 - F411RE
+HardwareSerial Serial6(PA12, PA11);  // Rx=PA12, Tx=PA11 -- CN10 pins 12 and 14 - F411RE
 #elif defined(ARDUINO_NUCLEO_F446RE)
-// Nucleo-64 boards don't have Serial1 defined by default
+// Nucleo-64 boards don't have additional serial ports defined by default
+// On the F446RE, Serial1 isn't really useable as it's Rx/Tx pair sit on already used D2/D10 pins
 // HardwareSerial Serial1(PA10, PB6);  // Rx=PA10 (D2), Tx=PB6 (D10) -- CN10 pins 17 and 9 - F446RE 
 // Serial2 is defined to use USART2 by default, but is in fact used as the diag console
 // via the debugger on the Nucleo-64. It is therefore unavailable for other DCC-EX uses like WiFi, DFPlayer, etc.
-HardwareSerial Serial1(PC11, PC10);  // Rx=PC11, Tx=PC10 -- USART3 - F446RE
-HardwareSerial Serial3(PD2, PC12);  // Rx=PC7, Tx=PC6 -- UART5 - F446RE
-// NB: USART3 and USART6 are available but as yet undefined
+// On the F446RE, Serial3 and Serial5 are easy to use:
+HardwareSerial Serial3(PC11, PC10);  // Rx=PC11, Tx=PC10 -- USART3 - F446RE
+HardwareSerial Serial5(PD2, PC12);  // Rx=PC7, Tx=PC6 -- UART5 - F446RE
+// On the F446RE, Serial4 and Serial6 also use pins we can't readily map while using the Arduino pins
 #elif defined(ARDUINO_NUCLEO_F412ZG) || defined(ARDUINO_NUCLEO_F429ZI) || defined(ARDUINO_NUCLEO_F446ZE)
 // Nucleo-144 boards don't have Serial1 defined by default
-HardwareSerial Serial1(PG9, PG14);  // Rx=PG9, Tx=PG14 -- USART6
+HardwareSerial Serial6(PG9, PG14);  // Rx=PG9, Tx=PG14 -- USART6
 // Serial3 is defined to use USART3 by default, but is in fact used as the diag console
 // via the debugger on the Nucleo-144. It is therefore unavailable for other DCC-EX uses like WiFi, DFPlayer, etc.
 #else
@@ -233,13 +235,16 @@ void DCCTimer::reset() {
     while(true) {};
 }
 
-#define NUM_ADC_INPUTS NUM_ANALOG_INPUTS
-
 // TODO: may need to use uint32_t on STMF4xx variants with > 16 analog inputs!
 #if defined(ARDUINO_NUCLEO_F446RE) || defined(ARDUINO_NUCLEO_F429ZI) || defined(ARDUINO_NUCLEO_F446ZE)
 #warning STM32 board selected not fully supported - only use ADC1 inputs 0-15 for current sensing!
 #endif
+// For now, define the max of 16 ports - some variants have more, but this not **yet** supported
+#define NUM_ADC_INPUTS 16
+// #define NUM_ADC_INPUTS NUM_ANALOG_INPUTS
+
 uint16_t ADCee::usedpins = 0;
+uint8_t ADCee::highestPin = 0;
 int * ADCee::analogvals = NULL;
 uint32_t * analogchans = NULL;
 bool adc1configured = false;
@@ -310,6 +315,7 @@ int ADCee::init(uint8_t pin) {
   analogvals[id] = value;     // Store sampled value
   analogchans[id] = adcchan;  // Keep track of which ADC channel is used for reading this pin
   usedpins |= (1 << id);      // This pin is now ready
+  if (id > highestPin) highestPin = id; // Store our highest pin in use
 
   DIAG(F("ADCee::init(): value=%d, channel=%d, id=%d"), value, adcchan, id);
 
@@ -352,7 +358,7 @@ void ADCee::scan() {
     waiting = false;
     id++;
     mask = mask << 1;
-    if (mask == 0) { // the 1 has been shifted out
+    if (id > highestPin) { // the 1 has been shifted out
       id = 0;
       mask = 1;
     }
@@ -374,9 +380,9 @@ void ADCee::scan() {
       }
       id++;
       mask = mask << 1;
-      if (mask == 0) { // the 1 has been shifted out
-	id = 0;
-	mask = 1;
+      if (id > highestPin) {
+      	id = 0;
+      	mask = 1;
       }
     }
   }

IO_RotaryEncoder.h

@@ -59,50 +59,98 @@
 
 class RotaryEncoder : public IODevice {
 public:
-  // Constructor
-  RotaryEncoder(VPIN firstVpin, int nPins, I2CAddress i2cAddress){
-    _firstVpin = firstVpin;
-    _nPins = nPins;
-    _I2CAddress = i2cAddress;
-    addDevice(this);
-  }
+  
   static void create(VPIN firstVpin, int nPins, I2CAddress i2cAddress) {
     if (checkNoOverlap(firstVpin, nPins, i2cAddress)) new RotaryEncoder(firstVpin, nPins, i2cAddress);
   }
 
 private:
+  // Constructor
+  RotaryEncoder(VPIN firstVpin, int nPins, I2CAddress i2cAddress){
+    _firstVpin = firstVpin;
+    _nPins = nPins;
+    if (_nPins > 3) {
+      _nPins = 3;
+      DIAG(F("RotaryEncoder WARNING:%d vpins defined, only 3 supported"), _nPins);
+    }
+    _I2CAddress = i2cAddress;
+    addDevice(this);
+  }
+
   // Initiate the device
   void _begin() {
+    uint8_t _status;
+    // Attempt to initilalise device
     I2CManager.begin();
     if (I2CManager.exists(_I2CAddress)) {
-      byte _getVersion[1] = {RE_VER};
-      I2CManager.read(_I2CAddress, _versionBuffer, 3, _getVersion, 1);
-      _majorVer = _versionBuffer[0];
-      _minorVer = _versionBuffer[1];
-      _patchVer = _versionBuffer[2];
-      _buffer[0] = RE_OP;
-      I2CManager.write(_I2CAddress, _buffer, 1);
+      // Send RE_OP, must receive RE_OP to be online
+      _sendBuffer[0] = RE_OP;
+      _status = I2CManager.read(_I2CAddress, _rcvBuffer, 1, _sendBuffer, 1);
+      if (_status == I2C_STATUS_OK) {
+        if (_rcvBuffer[0] == RE_OP) {
+          _sendBuffer[0] = RE_VER;
+          if (I2CManager.read(_I2CAddress, _versionBuffer, 3, _sendBuffer, 1) == I2C_STATUS_OK) {
+            _majorVer = _versionBuffer[0];
+            _minorVer = _versionBuffer[1];
+            _patchVer = _versionBuffer[2];
+          }
+        } else {
+          DIAG(F("RotaryEncoder I2C:%s garbage received: %d"), _I2CAddress.toString(), _rcvBuffer[0]);
+          _deviceState = DEVSTATE_FAILED;
+          return;
+        }
+      } else {
+        DIAG(F("RotaryEncoder I2C:%s ERROR connecting"), _I2CAddress.toString());
+        _deviceState = DEVSTATE_FAILED;
+        return;
+      }
+      // byte _getVersion[1] = {RE_VER};
+      // I2CManager.read(_I2CAddress, _versionBuffer, 3, _getVersion, 1);
+      // _majorVer = _versionBuffer[0];
+      // _minorVer = _versionBuffer[1];
+      // _patchVer = _versionBuffer[2];
+      // _buffer[0] = RE_OP;
+      // I2CManager.write(_I2CAddress, _buffer, 1);
 #ifdef DIAG_IO
       _display();
 #endif
     } else {
-        _deviceState = DEVSTATE_FAILED;
+      DIAG(F("RotaryEncoder I2C:%s device not found"), _I2CAddress.toString());
+      _deviceState = DEVSTATE_FAILED;
     }
   }
 
   void _loop(unsigned long currentMicros) override {
-    I2CManager.read(_I2CAddress, _buffer, 1);
-    _position = _buffer[0];
-    // This here needs to have a change check, ie. position is a different value.
-  #if defined(EXRAIL_ACTIVE)
+    if (_deviceState == DEVSTATE_FAILED) return;  // Return if device has failed
+    if (_i2crb.isBusy()) return;                  // Return if I2C operation still in progress
+
+    if (currentMicros - _lastPositionRead > _positionRefresh) {
+      _lastPositionRead = currentMicros;
+      _sendBuffer[0] = RE_READ;
+      I2CManager.read(_I2CAddress, _rcvBuffer, 1, _sendBuffer, 1, &_i2crb); // Read position from encoder
+      _position = _rcvBuffer[0];
+      // If EXRAIL is active, we need to trigger the ONCHANGE() event handler if it's in use
+#if defined(EXRAIL_ACTIVE)
       if (_position != _previousPosition) {
         _previousPosition = _position;
-        RMFT2::changeEvent(_firstVpin,1);
+        RMFT2::changeEvent(_firstVpin, 1);
       } else {
-        RMFT2::changeEvent(_firstVpin,0);
+        RMFT2::changeEvent(_firstVpin, 0);
       }
-  #endif
-    delayUntil(currentMicros + 100000);
+#endif
+    }
+  //   I2CManager.read(_I2CAddress, _buffer, 1);
+  //   _position = _buffer[0];
+  //   // This here needs to have a change check, ie. position is a different value.
+  // #if defined(EXRAIL_ACTIVE)
+  //     if (_position != _previousPosition) {
+  //       _previousPosition = _position;
+  //       RMFT2::changeEvent(_firstVpin,1);
+  //     } else {
+  //       RMFT2::changeEvent(_firstVpin,0);
+  //     }
+  // #endif
+  //   delayUntil(currentMicros + 100000);
   }
 
   // Device specific read function
@@ -122,7 +170,8 @@ private:
   void _writeAnalogue(VPIN vpin, int position, uint8_t profile, uint16_t duration) override {
     if (vpin == _firstVpin + 2) {
       if (position >= 0 && position <= 255) {
-        byte _positionBuffer[2] = {RE_MOVE, position};
+        byte newPosition = position & 0xFF;
+        byte _positionBuffer[2] = {RE_MOVE, newPosition};
         I2CManager.write(_I2CAddress, _positionBuffer, 2);
       }
     }
@@ -136,15 +185,20 @@ private:
   int8_t _position;
   int8_t _previousPosition = 0;
   uint8_t _versionBuffer[3];
-  uint8_t _buffer[1];
+  uint8_t _sendBuffer[1];
+  uint8_t _rcvBuffer[1];
   uint8_t _majorVer = 0;
   uint8_t _minorVer = 0;
   uint8_t _patchVer = 0;
+  I2CRB _i2crb;
+  unsigned long _lastPositionRead = 0;
+  const unsigned long _positionRefresh = 100000UL;    // Delay refreshing position for 100ms
 
   enum {
     RE_VER = 0xA0,   // Flag to retrieve rotary encoder version from the device
     RE_OP = 0xA1,    // Flag for normal operation
-    RE_MOVE = 0xA2,  // Flag for sending a position update
+    RE_MOVE = 0xA2,  // Flag for sending a position update from the device driver to the encoder
+    RE_READ = 0xA3,  // Flag to read the current position of the encoder
   };
 
 };

WifiInterface.cpp

@@ -52,20 +52,32 @@ Stream * WifiInterface::wifiStream;
  
 #if (defined(ARDUINO_AVR_MEGA) || defined(ARDUINO_AVR_MEGA2560))
 #define NUM_SERIAL 3
+#define SERIAL1 Serial1
+#define SERIAL3 Serial3
+#endif
+
+#if defined(ARDUINO_ARCH_STM32)
+// Handle serial ports availability on STM32 for variants!
+// #undef NUM_SERIAL
+#if defined(ARDUINO_NUCLEO_F411RE)
+#define NUM_SERIAL 3
+#define SERIAL1 Serial1
+#define SERIAL3 Serial6
+#elif defined(ARDUINO_NUCLEO_F446RE)
+#define NUM_SERIAL 3
+#define SERIAL1 Serial3
+#define SERIAL3 Serial5
+#elif defined(ARDUINO_NUCLEO_F412ZG) || defined(ARDUINO_NUCLEO_F429ZI) || defined(ARDUINO_NUCLEO_F446ZE)
+#define NUM_SERIAL 2
+#define SERIAL1 Serial6
+#endif
 #endif
 
 #ifndef NUM_SERIAL
 #define NUM_SERIAL 1
+#define SERIAL1 Serial1
 #endif
 
-// For STM32 we need to define Serial3 in the platform specific
-// DCCTimerSTM32.cpp file, we here make the assumption that it
-// exists to link against.
-#ifdef ARDUINO_ARCH_STM32
-#if NUM_SERIAL > 2
-extern HardwareSerial Serial3;
-#endif
-#endif
 bool WifiInterface::setup(long serial_link_speed, 
                           const FSH *wifiESSID,
                           const FSH *wifiPassword,
@@ -85,13 +97,14 @@ bool WifiInterface::setup(long serial_link_speed,
   (void) channel;
 #endif  
 
+// See if the WiFi is attached to the first serial port
 #if NUM_SERIAL > 0 && !defined(SERIAL1_COMMANDS)
-  Serial1.begin(serial_link_speed);
-  wifiUp = setup(Serial1, wifiESSID, wifiPassword, hostname, port, channel);
+  SERIAL1.begin(serial_link_speed);
+  wifiUp = setup(SERIAL1, wifiESSID, wifiPassword, hostname, port, channel);
 #endif
 
 // Other serials are tried, depending on hardware.
-// Currently only the Arduino Mega 2560 has usable Serial2
+// Currently only the Arduino Mega 2560 has usable Serial2 (Nucleo-64 boards use Serial 2 for console!)
 #if defined(ARDUINO_AVR_MEGA2560)
 #if NUM_SERIAL > 1 && !defined(SERIAL2_COMMANDS)
   if (wifiUp == WIFI_NOAT)
@@ -107,8 +120,8 @@ bool WifiInterface::setup(long serial_link_speed,
 #if NUM_SERIAL > 2 && !defined(SERIAL3_COMMANDS)
   if (wifiUp == WIFI_NOAT)
   {
-    Serial3.begin(serial_link_speed);
-    wifiUp = setup(Serial3, wifiESSID, wifiPassword, hostname, port, channel);
+    SERIAL3.begin(serial_link_speed);
+    wifiUp = setup(SERIAL3, wifiESSID, wifiPassword, hostname, port, channel);
   }
 #endif
 

defines.h

@@ -147,8 +147,6 @@
   #ifndef I2C_USE_WIRE
   #define I2C_USE_WIRE
   #endif
-  #undef NUM_SERIAL
-  #define NUM_SERIAL 3
 
 /* TODO when ready 
 #elif defined(ARDUINO_ARCH_RP2040)

platformio.ini

@@ -188,7 +188,7 @@ platform = ststm32
 board = nucleo_f446re
 framework = arduino
 lib_deps = ${env.lib_deps}
-build_flags = -std=c++17  -Os -g2 -Wunused-variable -DDIAG_LOOPTIMES ; -DDIAG_IO 
+build_flags = -std=c++17  -Os -g2 -Wunused-variable ; -DDIAG_LOOPTIMES ; -DDIAG_IO 
 monitor_speed = 115200
 monitor_echo = yes