Fix for analog expansion apparently not taking I2C address

The problem was due to the fact that I2C change address in the expansion is done in the main loop
So the expansion gets the request from the controller to change the address, but the the actual change is performed after a while in the main loop
There is no way to understand how much this can take because depends on the channel setting (for example RTD is slow and when is active the main loop is slower)
To solve this problem 3 actions were taken:
1. in the expansion the main loop only execute the Module::update() if the address is not set up (this ensure the main loop is as fast as possible)
2. in the controller the timeout for I2C set up address has been increased up to five seconds (but kept at 50 ms for any other message)
3. the controller has now a flag that continue to ask for the confirmation of a changed I2C address until the confirmation is received (send_new_address)
   This flag allow the controller to set up a new address for an expansion without a valid address is received only if the previous one has confirmed it has the new address
   The problem in fact was related to the fact that the expansion actually gets the address (in the main loop) but it was not able to respond in time
   So the controller was setting the new address of the next expansion without understanding that the old one was there

Author: maidnl

src/OptaAnalog.cpp

@@ -288,6 +288,16 @@ void OptaAnalog::debug_with_leds() {
 
 void OptaAnalog::update() {
   Module::update();
+
+  /* Opta analog has a slow main that depends on the operation it is performing
+     at the very beginning (when the espansion has not an address yet) it is 
+     necessary to go faster so the main is skipped untill the expansion gets 
+     a valid address */
+
+  if(wire_i2c_address <= OPTA_DEFAULT_SLAVE_I2C_ADDRESS || wire_i2c_address >= OPTA_CONTROLLER_FIRST_TEMPORARY_ADDRESS) {
+    return;
+  }
+
 #ifdef DEBUG_UPDATE_FW
   debug_with_leds();
   updateLedStatus();

src/OptaBlueModule.cpp

@@ -155,9 +155,8 @@ void Module::reset() {
 
   setStatusLedWaitingForAddress();
   /* put address to invalid */
+  wire_i2c_address = OPTA_DEFAULT_SLAVE_I2C_ADDRESS;
 
-  address = OPTA_DEFAULT_SLAVE_I2C_ADDRESS;
-
   /* detect_in (toward Controller) as Output */
   pinMode(detect_in, OUTPUT);
   /* put detect in pin to LOW -> signal the MODULE wants an address */
@@ -176,7 +175,7 @@ void Module::reset() {
 
   /* put I2C address to the default one */
   if (Module::expWire != nullptr) {
-    Module::expWire->begin(address);
+    Module::expWire->begin(wire_i2c_address);
   }
 
 }
@@ -187,15 +186,15 @@ void Module::reset() {
 /* --------------------------------------------------------------------------
  */
 Module::Module()
-    : address(OPTA_DEFAULT_SLAVE_I2C_ADDRESS), rx_num(0), reboot_required(false),
+    : rx_num(0), reboot_required(false),
       reset_required(false), ans_buffer(nullptr),
       expansion_type(OPTA_CONTROLLER_CUSTOM_MIN_TYPE), reboot_sent(0),
       detect_in(DETECT_IN), detect_out(DETECT_OUT) {
   Module::expWire = &Wire;
 }
 
 Module::Module(TwoWire *tw, int _detect_in, int _detect_out)
-    : address(OPTA_DEFAULT_SLAVE_I2C_ADDRESS), rx_num(0),
+    : rx_num(0),
       reboot_required(false), reset_required(false), ans_buffer(nullptr),
       expansion_type(OPTA_CONTROLLER_CUSTOM_MIN_TYPE), reboot_sent(0),
       detect_in(_detect_in), detect_out(_detect_out) {
@@ -207,7 +206,7 @@ Module::Module(TwoWire *tw, int _detect_in, int _detect_out)
 /* --------------------------------------------------------------------------
  */
 bool Module::addressAcquired() {
-  return ((address > OPTA_DEFAULT_SLAVE_I2C_ADDRESS));
+  return ((wire_i2c_address > OPTA_DEFAULT_SLAVE_I2C_ADDRESS));
 }
 
 /* --------------------------------------------------------------------------
@@ -241,14 +240,14 @@ void Module::begin() {
 /* --------------------------------------------------------------------------
  */
 void Module::update() {
-
   if(reset_I2C_bus) {
     /* 20240515_moved_I2C_reset moved reset I2C to main */
     pinMode(detect_out, INPUT_PULLUP);
     delay(1);
     /* accept the address only if detect out is HIGH */
     if (digitalRead(detect_out) == HIGH) {
-      setAddress(address);
+      wire_i2c_address = rx_i2c_address;
+      setAddress(wire_i2c_address);
     }
     reset_I2C_bus = false;
   }
@@ -301,7 +300,7 @@ bool Module::parse_set_address() {
   /* ------------------------------------------------------------------------ */
   if (checkSetMsgReceived(rx_buffer, ARG_ADDRESS, LEN_ADDRESS,
                           MSG_SET_ADDRESS_LEN)) {
-    address = rx_buffer[BP_PAYLOAD_START_POS];
+    rx_i2c_address = rx_buffer[BP_PAYLOAD_START_POS];
     set_address_msg_received = true;
     return true;
   }
@@ -399,7 +398,7 @@ bool Module::parse_reboot() {
 int Module::prepare_ans_get_address_and_type() {
   /* ----------------------------------------------------------------------
    */
-  tx_buffer[BP_PAYLOAD_START_POS] = address;
+  tx_buffer[BP_PAYLOAD_START_POS] = wire_i2c_address;
   tx_buffer[BP_PAYLOAD_START_POS + 1] = expansion_type;
   return prepareGetAns(tx_buffer, ANS_ARG_ADDRESS_AND_TYPE,
                        ANS_LEN_ADDRESS_AND_TYPE, ANS_MSG_ADDRESS_AND_TYPE_LEN);
@@ -561,7 +560,7 @@ void Module::updatePinStatus() {
   } else {
 #if defined DEBUG_SERIAL && defined DEBUG_UPDATE_PIN_ENABLE
     Serial.print("ADDRESS ACQUIRED ");
-    Serial.println(address, HEX);
+    Serial.println(wire_i2c_address, HEX);
 #endif
     setStatusLedHasAddress();
     #ifdef USE_CONFIRM_RX_MESSAGE

src/OptaBlueModule.h

@@ -102,10 +102,11 @@ class Module {
   int prepare_ans_reboot();
   int prepare_ans_get_flash();
 
-  uint8_t getI2CAddress() { return address; }
+  uint8_t getI2CAddress() { return wire_i2c_address; }
 
 protected:
-  uint8_t address;
+  volatile uint8_t rx_i2c_address = OPTA_DEFAULT_SLAVE_I2C_ADDRESS;
+  uint8_t wire_i2c_address = OPTA_DEFAULT_SLAVE_I2C_ADDRESS;
   uint8_t rx_num;
   volatile bool reboot_required;
   volatile bool reset_required;

src/OptaController.cpp

@@ -188,7 +188,7 @@ uint8_t Controller::send(int add, int device, unsigned int type, int n, int r) {
         _send(add, n, r);
 
         if (r > 0) {
-          if (wait_for_device_answer(device, r)) {
+          if (wait_for_device_answer(device, r, OPTA_CONTROLLER_WAIT_REQUEST_TIMEOUT)) {
             return SEND_RESULT_OK;
           }
           return SEND_RESULT_COMM_TIMEOUT;
@@ -377,7 +377,7 @@ bool Controller::rebootExpansion(uint8_t device) {
 
     _send(exp_add[device], msg_opta_reboot(), req);
 
-    if (wait_for_device_answer(OPTA_BLUE_UNDEFINED_DEVICE_NUMBER, req)) {
+    if (wait_for_device_answer(OPTA_BLUE_UNDEFINED_DEVICE_NUMBER, req, OPTA_CONTROLLER_WAIT_REQUEST_TIMEOUT)) {
       if (parse_opta_reboot()) {
         delay(OPTA_CONTROLLER_DELAY_AFTER_REBOOT);
         return true;
@@ -562,18 +562,29 @@ void Controller::checkForExpansions() {
       tmp_exp_add[i] = 0;
     }
     num_of_exp = 0;
+    
+    for(int i = OPTA_CONTROLLER_FIRST_AVAILABLE_ADDRESS + 1; i < OPTA_CONTROLLER_FIRST_AVAILABLE_ADDRESS + 2*OPTA_CONTROLLER_MAX_EXPANSION_NUM; i++) {
+      _send(i, msg_opta_reset(), 0);
+    }
+    delay(OPTA_CONTROLLER_SETUP_INIT_DELAY);
   }
 
   tmp_address = OPTA_CONTROLLER_FIRST_TEMPORARY_ADDRESS;
+  bool send_new_address = true;
 
   while (enter_while) {
+
+    if(send_new_address) {
+
 #if defined DEBUG_SERIAL && defined DEBUG_ASSIGN_ADDRESS_CONTROLLER
     Serial.println("[LOG]:  - DETECT pin is LOW (expansions without address)");
-    Serial.println("        - Sending SET address message");
+    Serial.println("        - Sending SET temp address message 0x" + String(tmp_address,HEX));
 #endif
-    _send(OPTA_DEFAULT_SLAVE_I2C_ADDRESS, msg_set_address(tmp_address), 0);
+      _send(OPTA_DEFAULT_SLAVE_I2C_ADDRESS, msg_set_address(tmp_address), 0);
+      delay(OPTA_CONTROLLER_DELAY_AFTER_SET_ADDRESS);
+    }
 
-    delay(OPTA_CONTROLLER_DELAY_AFTER_SET_ADDRESS);
+    send_new_address = false;
 
 #if defined DEBUG_SERIAL && defined DEBUG_ASSIGN_ADDRESS_CONTROLLER
     Serial.println("        - Sending GET address message");
@@ -598,10 +609,26 @@ void Controller::checkForExpansions() {
        tmp_num_of_exp so that tmp_exp_add array is filled in a circular way
      */
 
-    if (wait_for_device_answer(OPTA_BLUE_UNDEFINED_DEVICE_NUMBER, req)) {
-      parse_address_and_type(tmp_address);
+    if (wait_for_device_answer(OPTA_BLUE_UNDEFINED_DEVICE_NUMBER, req, OPTA_CONTROLLER_TIMEOUT_FOR_SETUP_MESSAGE)) {
+      if(parse_address_and_type(tmp_address)) {
+        #ifdef USE_CONFIRM_RX_MESSAGE
+        delay(5);
+        _send(tmp_address, msg_confirm_rx_address(),0);
+        #endif
+        send_new_address = true;
+        #if defined DEBUG_SERIAL && defined DEBUG_ASSIGN_ADDRESS_CONTROLLER
+        Serial.println("        GOT IT! ");
+        #endif
+      } 
+      else {
+        #if defined DEBUG_SERIAL && defined DEBUG_ASSIGN_ADDRESS_CONTROLLER
+        Serial.println("          TIMEOUT ");
+        #endif  
+      }
     }
 
+    
+
 #if defined DEBUG_SERIAL && defined DEBUG_ASSIGN_ADDRESS_CONTROLLER
     Serial.print("TEMPORARY Number of expansions found ");
     Serial.println(tmp_num_of_exp);
@@ -696,7 +723,7 @@ void Controller::checkForExpansions() {
       after 3 failed attemps the address is skipped and then the
       tmp_num_of_exp is decreased in the else branch */
 
-      if (wait_for_device_answer(OPTA_BLUE_UNDEFINED_DEVICE_NUMBER, req)) {
+      if (wait_for_device_answer(OPTA_BLUE_UNDEFINED_DEVICE_NUMBER, req, OPTA_CONTROLLER_TIMEOUT_FOR_SETUP_MESSAGE)) {
         if (parse_address_and_type(address)) {
           remain_in_while_loop = (tmp_num_of_exp != initial_value);
         }
@@ -724,7 +751,7 @@ void Controller::checkForExpansions() {
       if (exp_type[i] >= OPTA_CONTROLLER_CUSTOM_MIN_TYPE) {
         _send(exp_add[i], msg_get_product_type(), CTRL_ANS_MSG_GET_PRODUCT_LEN);
         if (wait_for_device_answer(OPTA_BLUE_UNDEFINED_DEVICE_NUMBER,
-                                   CTRL_ANS_MSG_GET_PRODUCT_LEN)) {
+                                   CTRL_ANS_MSG_GET_PRODUCT_LEN, OPTA_CONTROLLER_WAIT_REQUEST_TIMEOUT)) {
           if (parse_get_product()) {
             exp_type[i] = next_available_custom_type;
             next_available_custom_type++;
@@ -820,6 +847,20 @@ uint8_t Controller::msg_set_address(uint8_t add) {
                        MSG_SET_ADDRESS_LEN);
 }
 
+/* +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ */
+ #ifdef USE_CONFIRM_RX_MESSAGE
+uint8_t Controller::msg_confirm_rx_address() {
+  tx_buffer[CONFIRM_ADDRESS_FIRST_POS] = CONFIRM_ADDRESS_FIRST_VALUE; 
+  tx_buffer[CONFIRM_ADDRESS_SECOND_POS] = CONFIRM_ADDRESS_SECOND_VALUE; 
+
+  
+  return prepareSetMsg(tx_buffer, ARG_CONFIRM_ADDRESS_RX, LEN_CONFIRM_ADDRESS_RX,
+                       CONFIRM_ADDRESS_RX_LEN);
+
+}
+#endif
+
+
 /* +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ */
 
 uint8_t Controller::msg_get_product_type() {
@@ -933,7 +974,9 @@ bool Controller::parse_address_and_type(int slave_address) {
           Serial.println("EXPANSION_OPTA_DIGITAL_MEC");
         } else if (tmp_exp_type[tmp_num_of_exp] == EXPANSION_OPTA_DIGITAL_STS) {
           Serial.println("EXPANSION_OPTA_DIGITAL_STS");
-        } else {
+        } else if (tmp_exp_type[tmp_num_of_exp] == EXPANSION_OPTA_ANALOG) {
+          Serial.println("EXPANSION_OPTA_ANALOG");
+        }else {
           Serial.println("expansion code unknown!!!!!!");
         }
 #endif
@@ -1015,10 +1058,10 @@ void Controller::_send(int add, int n, int r) {
 /* when an answer is requested from expansion this function wait for an
    answer and put it into the rx buffer
    return true if an answer is available */
-bool Controller::wait_for_device_answer(uint8_t device, uint8_t wait_for) {
+bool Controller::wait_for_device_answer(uint8_t device, uint8_t wait_for, uint16_t timeout) {
   uint8_t rx = 0;
   unsigned long int start = millis();
-  while (millis() - start < OPTA_CONTROLLER_WAIT_REQUEST_TIMEOUT &&
+  while (millis() - start < timeout &&
          rx < wait_for) {
     while (Wire.available()) {
       uint8_t rec = Wire.read();

src/OptaController.h

@@ -204,7 +204,7 @@ class Controller {
 
   /* ---------------  generic message handling functions ----------------- */
 
-  bool wait_for_device_answer(uint8_t device, uint8_t wait_for);
+  bool wait_for_device_answer(uint8_t device, uint8_t wait_for, uint16_t timeout);
 
   /* ---------------- message preparation functions ---------------------- */
 
@@ -213,12 +213,16 @@ class Controller {
   uint8_t msg_opta_reset();
   uint8_t msg_opta_reboot();
   uint8_t msg_get_product_type();
+  #ifdef USE_CONFIRM_RX_MESSAGE
+  uint8_t msg_confirm_rx_address();
+  #endif
   /* -------------------- parse message functions ------------------------ */
 
   bool parse_get_product();
   bool parse_address_and_type(int slave_address);
   bool parse_opta_reboot();
 
+
   void _send(int add, int n, int r);
 
   bool is_detect_high();

src/OptaControllerCfg.h

@@ -36,6 +36,8 @@
    It exits as soon as the message is received */
 #define OPTA_CONTROLLER_WAIT_REQUEST_TIMEOUT 50
 
+#define OPTA_CONTROLLER_TIMEOUT_FOR_SETUP_MESSAGE 100
+
 #define OPTA_CONTROLLER_DELAY_AFTER_REBOOT 600
 
 /* this is the time the controller leaves to expansion to "set up" themselves

src/OptaDigital.cpp

@@ -464,7 +464,7 @@ void OptaDigital::update() {
            Module */
   Module::update();
 
-  // if (Module::addressAcquired()) {
+  
   /* always "refresh" the header of the answers */
   ans_get_din_buffer[BP_CMD_POS] = BP_ANS_GET;
   ans_get_din_buffer[BP_ARG_POS] = ANS_ARG_OPDIG_DIGITAL;
@@ -474,6 +474,18 @@ void OptaDigital::update() {
   ans_get_all_ain_buffer[BP_ARG_POS] = ANS_ARG_OPTDIG_ALL_ANALOG_IN;
   ans_get_all_ain_buffer[BP_LEN_POS] = ANS_LEN_OPTDIG_ALL_ANALOG_IN;
 
+  /* Added to have the same behaviour of Opta Analog
+     [Opta analog has a slow main that depends on the operation it is performing
+     at the very beginning (when the espansion has not an address yet) it is 
+     necessary to go faster so the main is skipped untill the expansion gets 
+     a valid address]
+     This would not be necessary in case of Digital because the main loop is fast
+      */
+  if(wire_i2c_address <= OPTA_DEFAULT_SLAVE_I2C_ADDRESS || wire_i2c_address >= OPTA_CONTROLLER_FIRST_TEMPORARY_ADDRESS) {
+    return;
+  }
+
+
   /* update output */
   _updateDigitalOut();
   /* update input*/
@@ -530,7 +542,7 @@ void OptaDigital::update() {
 
     R_ADC_ScanStart(&(opta_adc.ctrl));
   }
-  //}
+  
 }
 
 #ifdef EN_DIGITAL_STANDALONE

src/OptaModuleProtocol.h

@@ -53,6 +53,17 @@
 #define CTRL_ANS_MSG_GET_PRODUCT_LEN LEN_ANS_GET_PRODUCT_TYPE
 #endif
 
+
+#ifdef USE_CONFIRM_RX_MESSAGE
+#define ARG_CONFIRM_ADDRESS_RX 0x26
+#define LEN_CONFIRM_ADDRESS_RX 2
+#define CONFIRM_ADDRESS_FIRST_POS (BP_HEADER_DIM + 0)
+#define CONFIRM_ADDRESS_SECOND_POS (BP_HEADER_DIM + 1)
+#define CONFIRM_ADDRESS_FIRST_VALUE 0xC9
+#define CONFIRM_ADDRESS_SECOND_VALUE 0xB1
+#define CONFIRM_ADDRESS_RX_LEN  (BP_HEADER_DIM + LEN_CONFIRM_ADDRESS_RX)
+#endif
+
 /* ######################## */
 /* VERSION related messages */
 /* ######################## */

tests/testStressAnalog/testStressAnalog.ino

@@ -190,7 +190,7 @@ void initReverse_2() {
 void setup() {
 /* -------------------------------------------------------------------------- */    
    Serial.begin(115200);
-   delay(2000);
+   delay(5000);
 
    OptaController.begin();
    initReverse_1();
@@ -313,14 +313,14 @@ void loop() {
    if(rtd_in) {
    	for(int i = 0; i < 8; i++) {
    		if(rtd_in.isChRtd(i)) {
-   			Serial.println("RTD expansion - channel " + String(i) + " reading value " + String(rtd_in.getRtd(i)) + " ohm");
+   			Serial.println("RTD expansion " + String(rtd_in.getIndex()) + " - channel " + String(i) + " reading value " + String(rtd_in.getRtd(i)) + " ohm");
    		}
    		else if(rtd_in.isChDigitalInput(i)) {
    			rtd_in.updateDigitalInputs();
    			if(rtd_in.digitalRead(i)==HIGH){
-   				Serial.println("RTD expansion - channel " + String(i) + " reading value HIGH (DIGITAL)");
+   				Serial.println("RTD expansion " + String(rtd_in.getIndex()) + " - channel " + String(i) + " reading value HIGH (DIGITAL)");
    			} else if(rtd_in.digitalRead(i)==LOW){
-   				Serial.println("RTD expansion - channel " + String(i) + " reading value LOW (DIGITAL)");
+   				Serial.println("RTD expansion " + String(rtd_in.getIndex()) + " - channel " + String(i) + " reading value LOW (DIGITAL)");
    			}
    		}
    	}