[VOL-2799] : Fix ONU delete issue on openolt agent

Change-Id: I87605da5f4081aeef0c8d779a49531375c471bde
diff --git a/.gitignore b/.gitignore
index 3db5238..2ebaf57 100644
--- a/.gitignore
+++ b/.gitignore
@@ -33,3 +33,6 @@
 
 # IntelliJ Files
 .idea
+
+# vscode
+.vscode
diff --git a/VERSION b/VERSION
index a724a9c..197c4d5 100644
--- a/VERSION
+++ b/VERSION
@@ -1 +1 @@
-2.4.0-dev
+2.4.0
diff --git a/agent/src/core_api_handler.cc b/agent/src/core_api_handler.cc
index c993c48..c6ced48 100644
--- a/agent/src/core_api_handler.cc
+++ b/agent/src/core_api_handler.cc
@@ -770,7 +770,7 @@
     BCMOLT_MSG_FIELD_SET(&interface_obj, discovery.control, BCMOLT_CONTROL_STATE_ENABLE);
     BCMOLT_MSG_FIELD_SET(&interface_obj, discovery.interval, 5000);
     BCMOLT_MSG_FIELD_SET(&interface_obj, discovery.onu_post_discovery_mode,
-        BCMOLT_ONU_POST_DISCOVERY_MODE_ACTIVATE);
+        BCMOLT_ONU_POST_DISCOVERY_MODE_NONE);
     BCMOLT_MSG_FIELD_SET(&interface_obj, itu.automatic_onu_deactivation.los, true);
     BCMOLT_MSG_FIELD_SET(&interface_obj, itu.automatic_onu_deactivation.onu_alarms, true);
     BCMOLT_MSG_FIELD_SET(&interface_obj, itu.automatic_onu_deactivation.tiwi, true);
@@ -981,50 +981,85 @@
     bcmolt_serial_number serial_number; /**< ONU serial number */
     bcmolt_bin_str_36 registration_id; /**< ONU registration ID */
 
+    bcmolt_onu_set_onu_state onu_oper; /* declare main API struct */
+    bcmolt_onu_state onu_state;
+
     onu_key.onu_id = onu_id;
     onu_key.pon_ni = intf_id;
     BCMOLT_CFG_INIT(&onu_cfg, onu, onu_key);
     BCMOLT_FIELD_SET_PRESENT(&onu_cfg.data, onu_cfg_data, onu_state);
-    #ifdef TEST_MODE
+#ifdef TEST_MODE
     // It is impossible to mock the setting of onu_cfg.data.onu_state because
     // the actual bcmolt_cfg_get passes the address of onu_cfg.hdr and we cannot
     // set the onu_cfg.data.onu_state. So a new stub function is created and address
     // of onu_cfg is passed. This is one-of case where we need to add test specific
     // code in production code.
     err = bcmolt_cfg_get__onu_state_stub(dev_id, &onu_cfg);
-    #else
+#else
     err = bcmolt_cfg_get(dev_id, &onu_cfg.hdr);
-    #endif
+#endif
+    OPENOLT_LOG(INFO, openolt_log_id, "Activate ONU : old state = %d, current state = %d\n",
+            onu_cfg.data.onu_old_state, onu_cfg.data.onu_state);
     if (err == BCM_ERR_OK) {
-        if ((onu_cfg.data.onu_state == BCMOLT_ONU_STATE_PROCESSING ||
-             onu_cfg.data.onu_state == BCMOLT_ONU_STATE_ACTIVE) ||
-           (onu_cfg.data.onu_state == BCMOLT_ONU_STATE_INACTIVE &&
-             onu_cfg.data.onu_old_state == BCMOLT_ONU_STATE_NOT_CONFIGURED))
+        if (onu_cfg.data.onu_state == BCMOLT_ONU_STATE_ACTIVE) {
+            OPENOLT_LOG(INFO, openolt_log_id, "ONU is already in ACTIVE state, \
+not processing this request for pon_intf=%d onu_id=%d\n", intf_id, onu_id);
             return Status::OK;
+        } else if (onu_cfg.data.onu_state != BCMOLT_ONU_STATE_NOT_CONFIGURED &&
+                onu_cfg.data.onu_state != BCMOLT_ONU_STATE_INACTIVE) {
+            // We need the ONU to be in NOT_CONFIGURED or INACTIVE state to further process the request
+            OPENOLT_LOG(ERROR, openolt_log_id, "ONU in an invalid state to process the request, \
+state=%d pon_intf=%d onu_id=%d\n", onu_cfg.data.onu_state, intf_id, onu_id);
+            return bcm_to_grpc_err(err, "Failed to activate ONU, invalid ONU state");
+        }
+    } else {
+        // This should never happen. BAL GET should succeed for non-existant ONUs too. The state of such ONUs will be NOT_CONFIGURED
+        OPENOLT_LOG(ERROR, openolt_log_id, "ONU state query failed pon_intf=%d onu_id=%d\n", intf_id, onu_id);
+        return bcm_to_grpc_err(err, "onu get failed");
     }
 
-    OPENOLT_LOG(INFO, openolt_log_id,  "Enabling ONU %d on PON %d : vendor id %s, \
+    // If the ONU is not configured at all we need to first configure it
+    if (onu_cfg.data.onu_state == BCMOLT_ONU_STATE_NOT_CONFIGURED) {
+        OPENOLT_LOG(INFO, openolt_log_id,  "Configuring ONU %d on PON %d : vendor id %s, \
 vendor specific %s, pir %d\n", onu_id, intf_id, vendor_id,
-        vendor_specific_to_str(vendor_specific).c_str(), pir);
+                vendor_specific_to_str(vendor_specific).c_str(), pir);
 
-    memcpy(serial_number.vendor_id.arr, vendor_id, 4);
-    memcpy(serial_number.vendor_specific.arr, vendor_specific, 4);
-    BCMOLT_CFG_INIT(&onu_cfg, onu, onu_key);
-    BCMOLT_MSG_FIELD_SET(&onu_cfg, itu.serial_number, serial_number);
-    BCMOLT_MSG_FIELD_SET(&onu_cfg, itu.auto_learning, BCMOS_TRUE);
-    /*set burst and data profiles to fec disabled*/
-    if (board_technology == "XGS-PON") {
-        BCMOLT_MSG_FIELD_SET(&onu_cfg, itu.xgpon.ranging_burst_profile, 2);
-        BCMOLT_MSG_FIELD_SET(&onu_cfg, itu.xgpon.data_burst_profile, 1);
-    } else if (board_technology == "GPON") {
-        BCMOLT_MSG_FIELD_SET(&onu_cfg, itu.gpon.ds_ber_reporting_interval, 1000000);
-        BCMOLT_MSG_FIELD_SET(&onu_cfg, itu.gpon.omci_port_id, onu_id);
+        memcpy(serial_number.vendor_id.arr, vendor_id, 4);
+        memcpy(serial_number.vendor_specific.arr, vendor_specific, 4);
+        BCMOLT_CFG_INIT(&onu_cfg, onu, onu_key);
+        BCMOLT_MSG_FIELD_SET(&onu_cfg, itu.serial_number, serial_number);
+        BCMOLT_MSG_FIELD_SET(&onu_cfg, itu.auto_learning, BCMOS_TRUE);
+        /*set burst and data profiles to fec disabled*/
+        if (board_technology == "XGS-PON") {
+            BCMOLT_MSG_FIELD_SET(&onu_cfg, itu.xgpon.ranging_burst_profile, 2);
+            BCMOLT_MSG_FIELD_SET(&onu_cfg, itu.xgpon.data_burst_profile, 1);
+        } else if (board_technology == "GPON") {
+            BCMOLT_MSG_FIELD_SET(&onu_cfg, itu.gpon.ds_ber_reporting_interval, 1000000);
+            BCMOLT_MSG_FIELD_SET(&onu_cfg, itu.gpon.omci_port_id, onu_id);
+        }
+        err = bcmolt_cfg_set(dev_id, &onu_cfg.hdr);
+        if (err != BCM_ERR_OK) {
+            OPENOLT_LOG(ERROR, openolt_log_id, "Failed to configure ONU %d on PON %d, err = %s\n", onu_id, intf_id, bcmos_strerror(err));
+            return bcm_to_grpc_err(err, "Failed to configure ONU");
+        }
     }
-    err = bcmolt_cfg_set(dev_id, &onu_cfg.hdr);
+
+    // Now that the ONU is configured, move the ONU to ACTIVE state
+    memset(&onu_cfg, 0, sizeof(bcmolt_onu_cfg));
+    BCMOLT_CFG_INIT(&onu_cfg, onu, onu_key);
+    BCMOLT_FIELD_SET_PRESENT(&onu_cfg.data, onu_cfg_data, onu_state);
+    BCMOLT_OPER_INIT(&onu_oper, onu, set_onu_state, onu_key);
+    BCMOLT_FIELD_SET(&onu_oper.data, onu_set_onu_state_data,
+            onu_state, BCMOLT_ONU_OPERATION_ACTIVE);
+    err = bcmolt_oper_submit(dev_id, &onu_oper.hdr);
     if (err != BCM_ERR_OK) {
-        OPENOLT_LOG(ERROR, openolt_log_id, "Failed to set activate ONU %d on PON %d, err = %s\n", onu_id, intf_id, bcmos_strerror(err));
+        OPENOLT_LOG(ERROR, openolt_log_id, "Failed to activate ONU %d on PON %d, err = %s\n", onu_id, intf_id, bcmos_strerror(err));
         return bcm_to_grpc_err(err, "Failed to activate ONU");
     }
+    // ONU will eventually get activated after we have submitted the operation request. The adapter will receive an asynchronous
+    // ONU_ACTIVATION_COMPLETED_INDICATION
+
+    OPENOLT_LOG(INFO, openolt_log_id, "Activated ONU, onu_id %d on PON %d\n", onu_id, intf_id);
 
     return Status::OK;
 }
@@ -1084,9 +1119,9 @@
 
     err = get_onu_status((bcmolt_interface)intf_id, onu_id, &onu_state);
     if (err == BCM_ERR_OK) {
-        if (onu_state == BCMOLT_ONU_STATE_ACTIVE) {
-            OPENOLT_LOG(INFO, openolt_log_id, "Onu is Active, onu_id: %d, waiting for onu deactivate complete response\n",
-                intf_id);
+        if (onu_state != BCMOLT_ONU_STATE_INACTIVE) {
+            OPENOLT_LOG(INFO, openolt_log_id, "waiting for onu deactivate complete response: intf_id=%d, onu_id=%d\n",
+                intf_id, onu_id);
             err = wait_for_onu_deactivate_complete(intf_id, onu_id);
             if (err) {
                 OPENOLT_LOG(ERROR, openolt_log_id, "failed to delete onu intf_id %d, onu_id %d\n",
@@ -1094,7 +1129,7 @@
                 return bcm_to_grpc_err(err, "Failed to delete ONU");
             }
         }
-        else if (onu_state == BCMOLT_ONU_STATE_INACTIVE) {
+        else {
             OPENOLT_LOG(INFO, openolt_log_id, "Onu is Inactive, onu_id: %d, not waiting for onu deactivate complete response\n",
                 intf_id);
         }
diff --git a/agent/test/src/test_core.cc b/agent/test/src/test_core.cc
index b08b92e..7f9926f 100644
--- a/agent/test/src/test_core.cc
+++ b/agent/test/src/test_core.cc
@@ -838,48 +838,30 @@
         }
 };
 
-// Test 1 - ActivateOnu success case
-TEST_F(TestActivateOnu, ActivateOnuSuccess) {
-    bcmos_errno onu_cfg_get_res = BCM_ERR_INTERNAL;
-    bcmos_errno onu_cfg_get_stub_res = BCM_ERR_INTERNAL;
+// Test 1 - ActivateOnu success case - ONU in BCMOLT_ONU_STATE_NOT_CONFIGURED state
+TEST_F(TestActivateOnu, ActivateOnuSuccessOnuNotConfigured) {
+    bcmos_errno onu_cfg_get_res = BCM_ERR_OK;
+    bcmos_errno onu_cfg_get_stub_res = BCM_ERR_OK;
     bcmos_errno onu_cfg_set_res = BCM_ERR_OK;
+    bcmos_errno onu_oper_submit_res = BCM_ERR_OK;
 
     bcmolt_onu_cfg onu_cfg;
     bcmolt_onu_key onu_key;
     BCMOLT_CFG_INIT(&onu_cfg, onu, onu_key);
-    onu_cfg.data.onu_state = BCMOLT_ONU_STATE_ACTIVE;
+    onu_cfg.data.onu_state = BCMOLT_ONU_STATE_NOT_CONFIGURED;
     EXPECT_GLOBAL_CALL(bcmolt_cfg_get__onu_state_stub, bcmolt_cfg_get__onu_state_stub(_, _))
                      .WillOnce(DoAll(SetArg1ToBcmOltOnuCfg(onu_cfg), Return(onu_cfg_get_stub_res)));
 
     ON_CALL(balMock, bcmolt_cfg_get(_, _)).WillByDefault(Return(onu_cfg_get_res));
     ON_CALL(balMock, bcmolt_cfg_set(_, _)).WillByDefault(Return(onu_cfg_set_res));
+    ON_CALL(balMock, bcmolt_oper_submit(_, _)).WillByDefault(Return(onu_oper_submit_res));
 
     Status status = ActivateOnu_(pon_id, onu_id, vendor_id.c_str(), vendor_specific.c_str(), pir);
     ASSERT_TRUE( status.error_message() == Status::OK.error_message() );
 }
 
-// Test 2 - ActivateOnu failure case
-TEST_F(TestActivateOnu, ActivateOnuFailure) {
-    bcmos_errno onu_cfg_get_res = BCM_ERR_INTERNAL;
-    bcmos_errno onu_cfg_get_stub_res = BCM_ERR_INTERNAL;
-    bcmos_errno onu_cfg_set_res = BCM_ERR_INTERNAL;
-
-    bcmolt_onu_cfg onu_cfg;
-    bcmolt_onu_key onu_key;
-    BCMOLT_CFG_INIT(&onu_cfg, onu, onu_key);
-    onu_cfg.data.onu_state = BCMOLT_ONU_STATE_ACTIVE;
-    EXPECT_GLOBAL_CALL(bcmolt_cfg_get__onu_state_stub, bcmolt_cfg_get__onu_state_stub(_, _))
-                     .WillOnce(DoAll(SetArg1ToBcmOltOnuCfg(onu_cfg), Return(onu_cfg_get_stub_res)));
-
-    ON_CALL(balMock, bcmolt_cfg_get(_, _)).WillByDefault(Return(onu_cfg_get_res));
-    ON_CALL(balMock, bcmolt_cfg_set(_, _)).WillByDefault(Return(onu_cfg_set_res));
-
-    Status status = ActivateOnu_(pon_id, onu_id, vendor_id.c_str(), vendor_specific.c_str(), pir);
-    ASSERT_FALSE( status.error_message() == Status::OK.error_message() );
-}
-
-// Test 3 - ActivateOnu - Onu already under processing case
-TEST_F(TestActivateOnu, ActivateOnuProcessing) {
+// Test 2 - ActivateOnu success case - ONU already in BCMOLT_ONU_STATE_ACTIVE state
+TEST_F(TestActivateOnu, ActivateOnuSuccessOnuAlreadyActive) {
     bcmos_errno onu_cfg_get_res = BCM_ERR_OK;
     bcmos_errno onu_cfg_get_stub_res = BCM_ERR_OK;
 
@@ -889,12 +871,89 @@
     onu_cfg.data.onu_state = BCMOLT_ONU_STATE_ACTIVE;
     EXPECT_GLOBAL_CALL(bcmolt_cfg_get__onu_state_stub, bcmolt_cfg_get__onu_state_stub(_, _))
                      .WillOnce(DoAll(SetArg1ToBcmOltOnuCfg(onu_cfg), Return(onu_cfg_get_stub_res)));
+
     ON_CALL(balMock, bcmolt_cfg_get(_, _)).WillByDefault(Return(onu_cfg_get_res));
 
     Status status = ActivateOnu_(pon_id, onu_id, vendor_id.c_str(), vendor_specific.c_str(), pir);
     ASSERT_TRUE( status.error_message() == Status::OK.error_message() );
 }
 
+// Test 3 - ActivateOnu success case - ONU in BCMOLT_ONU_STATE_INACTIVE state
+TEST_F(TestActivateOnu, ActivateOnuSuccessOnuInactive) {
+    bcmos_errno onu_cfg_get_res = BCM_ERR_OK;
+    bcmos_errno onu_cfg_get_stub_res = BCM_ERR_OK;
+    bcmos_errno onu_oper_submit_res = BCM_ERR_OK;
+
+    bcmolt_onu_cfg onu_cfg;
+    bcmolt_onu_key onu_key;
+    BCMOLT_CFG_INIT(&onu_cfg, onu, onu_key);
+    onu_cfg.data.onu_state = BCMOLT_ONU_STATE_INACTIVE;
+    EXPECT_GLOBAL_CALL(bcmolt_cfg_get__onu_state_stub, bcmolt_cfg_get__onu_state_stub(_, _))
+                     .WillOnce(DoAll(SetArg1ToBcmOltOnuCfg(onu_cfg), Return(onu_cfg_get_stub_res)));
+
+    ON_CALL(balMock, bcmolt_cfg_get(_, _)).WillByDefault(Return(onu_cfg_get_res));
+    ON_CALL(balMock, bcmolt_oper_submit(_, _)).WillByDefault(Return(onu_oper_submit_res));
+
+
+    Status status = ActivateOnu_(pon_id, onu_id, vendor_id.c_str(), vendor_specific.c_str(), pir);
+    ASSERT_TRUE( status.error_message() == Status::OK.error_message() );
+}
+
+// Test 4 - ActivateOnu failure case - ONU in invalid state (for this ex: BCMOLT_ONU_STATE_LOW_POWER_DOZE)
+TEST_F(TestActivateOnu, ActivateOnuFailOnuInvalidState) {
+    bcmos_errno onu_cfg_get_res = BCM_ERR_OK;
+    bcmos_errno onu_cfg_get_stub_res = BCM_ERR_OK;
+
+    bcmolt_onu_cfg onu_cfg;
+    bcmolt_onu_key onu_key;
+    BCMOLT_CFG_INIT(&onu_cfg, onu, onu_key);
+    onu_cfg.data.onu_state = BCMOLT_ONU_STATE_LOW_POWER_DOZE; // some invalid state which we dont recognize or process
+    EXPECT_GLOBAL_CALL(bcmolt_cfg_get__onu_state_stub, bcmolt_cfg_get__onu_state_stub(_, _))
+                     .WillOnce(DoAll(SetArg1ToBcmOltOnuCfg(onu_cfg), Return(onu_cfg_get_stub_res)));
+
+    ON_CALL(balMock, bcmolt_cfg_get(_, _)).WillByDefault(Return(onu_cfg_get_res));
+
+    Status status = ActivateOnu_(pon_id, onu_id, vendor_id.c_str(), vendor_specific.c_str(), pir);
+    ASSERT_FALSE( status.error_message() == Status::OK.error_message() );
+}
+
+// Test 5 - ActivateOnu failure case - cfg_get failure
+TEST_F(TestActivateOnu, ActivateOnuFailCfgGetFail) {
+    bcmos_errno onu_cfg_get_stub_res = BCM_ERR_INTERNAL;// return cfg_get failure
+
+    bcmolt_onu_cfg onu_cfg;
+    bcmolt_onu_key onu_key;
+    BCMOLT_CFG_INIT(&onu_cfg, onu, onu_key);
+    onu_cfg.data.onu_state = BCMOLT_ONU_STATE_ACTIVE;
+    EXPECT_GLOBAL_CALL(bcmolt_cfg_get__onu_state_stub, bcmolt_cfg_get__onu_state_stub(_, _))
+                     .WillOnce(DoAll(SetArg1ToBcmOltOnuCfg(onu_cfg), Return(onu_cfg_get_stub_res)));
+
+    Status status = ActivateOnu_(pon_id, onu_id, vendor_id.c_str(), vendor_specific.c_str(), pir);
+    ASSERT_FALSE( status.error_message() == Status::OK.error_message() );
+}
+
+// Test 6 - ActivateOnu failure case - oper_submit failure
+TEST_F(TestActivateOnu, ActivateOnuFailOperSubmitFail) {
+    bcmos_errno onu_cfg_get_res = BCM_ERR_OK;
+    bcmos_errno onu_cfg_get_stub_res = BCM_ERR_OK;
+    bcmos_errno onu_cfg_set_res = BCM_ERR_OK;
+    bcmos_errno onu_oper_submit_res = BCM_ERR_INTERNAL; // return oper_submit failure
+
+    bcmolt_onu_cfg onu_cfg;
+    bcmolt_onu_key onu_key;
+    BCMOLT_CFG_INIT(&onu_cfg, onu, onu_key);
+    onu_cfg.data.onu_state = BCMOLT_ONU_STATE_NOT_CONFIGURED;
+    EXPECT_GLOBAL_CALL(bcmolt_cfg_get__onu_state_stub, bcmolt_cfg_get__onu_state_stub(_, _))
+                     .WillOnce(DoAll(SetArg1ToBcmOltOnuCfg(onu_cfg), Return(onu_cfg_get_stub_res)));
+
+    ON_CALL(balMock, bcmolt_cfg_get(_, _)).WillByDefault(Return(onu_cfg_get_res));
+    ON_CALL(balMock, bcmolt_cfg_set(_, _)).WillByDefault(Return(onu_cfg_set_res));
+    ON_CALL(balMock, bcmolt_oper_submit(_, _)).WillByDefault(Return(onu_oper_submit_res));
+
+    Status status = ActivateOnu_(pon_id, onu_id, vendor_id.c_str(), vendor_specific.c_str(), pir);
+    ASSERT_FALSE( status.error_message() == Status::OK.error_message() );
+}
+
 ////////////////////////////////////////////////////////////////////////////
 // For testing DeactivateOnu functionality
 ////////////////////////////////////////////////////////////////////////////