Feature/ppm 5 handle smart steering bml requests and fix code review comments

ci/cppcheck/cppcheck_existing_issues.txt

@@ -96,7 +96,6 @@ controller/src/beerocks/master/son_actions.cpp: performance: Function parameter
 controller/src/beerocks/master/son_actions.cpp: performance: Function parameter 'sta_mac' should be passed by const reference. [passedByValue]                                                     std::string sta_mac, std::string bssid)
 controller/src/beerocks/master/son_actions.cpp: style: Local variable 'prev_task_id' shadows outer variable [shadowVariable]        int prev_task_id = database.get_association_handling_task_id(mac);
 controller/src/beerocks/master/son_actions.cpp: style: The scope of the variable 'prev_task_id' can be reduced. [variableScope]    int prev_task_id;
-controller/src/beerocks/master/son_management.cpp: style: Condition '!client_list.size()' is always true [knownConditionTrueFalse]        if (!client_list.size()) {
 controller/src/beerocks/master/son_management.cpp: style: Variable 'op_error_code' is assigned a value that is never used. [unreadVariable]            op_error_code = eChannelScanOperationCode::SCAN_IN_PROGRESS;
 controller/src/beerocks/master/son_management.cpp: style: Variable 'op_error_code' is assigned a value that is never used. [unreadVariable]        auto op_error_code = eChannelScanOperationCode::SUCCESS;
 controller/src/beerocks/master/son_management.cpp: style: Variable 'result_status' is assigned a value that is never used. [unreadVariable]            result_status = last_scan_success;

controller/src/beerocks/bml/bml_defs.h

@@ -589,17 +589,14 @@ struct BML_CLIENT_CONFIG {
     // 1 for true, 0 for false, -1 for "not configured".
     int8_t stay_on_initial_radio;
 
-    // 1 for true, 0 for false, -1 for "not configured".
-    int8_t stay_on_selected_device;
-
     // Bitwise value of selected bands for the client.
     // Correlates to BML_CLIENT_SELECTED_BANDS
     int8_t selected_bands;
 };
 
 struct BML_CLIENT {
     // Client MAC.
-    char sta_mac[BML_MAC_ADDR_LEN];
+    uint8_t sta_mac[BML_MAC_ADDR_LEN];
 
     // Time of last client configuration edit (in Seconds)
     uint32_t timestamp_sec;
@@ -610,9 +607,6 @@ struct BML_CLIENT {
     // 1 for true, 0 for false, -1 for "not configured".
     int8_t stay_on_initial_radio;
 
-    // 1 for true, 0 for false, -1 for "not configured".
-    int8_t stay_on_selected_device;
-
     // Bitwise value of selected bands for the client.
     // Correlates to BML_CLIENT_SELECTED_BANDS
     int8_t selected_bands;

controller/src/beerocks/bml/internal/bml_internal.cpp

@@ -1141,13 +1141,13 @@ int bml_internal::process_cmdu_header(std::shared_ptr<beerocks_header> beerocks_
                     ->addClass<beerocks_message::cACTION_BML_CLIENT_GET_CLIENT_LIST_RESPONSE>();
             if (!response) {
                 LOG(ERROR) << "addClass cACTION_BML_CLIENT_GET_CLIENT_LIST_RESPONSE failed";
-                return BML_RET_OP_FAILED;
+                return (-BML_RET_OP_FAILED);
             }
 
             if (!m_client_list || !m_client_list_size) {
                 LOG(ERROR) << "The pointer to the user data buffer is null!";
                 m_prmClientListGet->set_value(false);
-                break;
+                return (-BML_RET_INVALID_ARGS);
             }
 
             if (response->result() != 0) {
@@ -1195,7 +1195,7 @@ int bml_internal::process_cmdu_header(std::shared_ptr<beerocks_header> beerocks_
                     ->addClass<beerocks_message::cACTION_BML_CLIENT_SET_CLIENT_RESPONSE>();
             if (!response) {
                 LOG(ERROR) << "addClass cACTION_BML_CLIENT_SET_CLIENT_RESPONSE failed";
-                return BML_RET_OP_FAILED;
+                return (-BML_RET_OP_FAILED);
             }
 
             ///Signal any waiting threads
@@ -1207,33 +1207,44 @@ int bml_internal::process_cmdu_header(std::shared_ptr<beerocks_header> beerocks_
         } break;
         case beerocks_message::ACTION_BML_CLIENT_GET_CLIENT_RESPONSE: {
             LOG(DEBUG) << "ACTION_BML_CLIENT_GET_CLIENT_RESPONSE received";
+
+            if (!m_prmClientGet) {
+                LOG(WARNING) << "Received ACTION_BML_CLIENT_GET_CLIENT_RESPONSE response, "
+                             << "but no one is waiting...";
+                break;
+            }
+
             auto response =
                 beerocks_header
                     ->addClass<beerocks_message::cACTION_BML_CLIENT_GET_CLIENT_RESPONSE>();
             if (!response) {
                 LOG(ERROR) << "addClass cACTION_BML_CLIENT_GET_CLIENT_RESPONSE failed";
-                return BML_RET_OP_FAILED;
+                return (-BML_RET_OP_FAILED);
             }
 
-            //Signal any waiting threads
-            if (m_prmClientGet) {
-                bool promise_result = false;
-                if (m_client) {
-                    std::copy_n(response->client().sta_mac.oct, BML_MAC_ADDR_LEN,
-                                m_client->sta_mac);
-                    m_client->timestamp_sec           = response->client().timestamp_sec;
-                    m_client->stay_on_initial_radio   = response->client().stay_on_initial_radio;
-                    m_client->stay_on_selected_device = response->client().stay_on_selected_device;
-                    m_client->selected_bands          = response->client().selected_bands;
-                    m_client->time_life_delay_days    = response->client().time_life_delay_days;
-                    //Resolve promise to "true"
-                    promise_result = true;
-                }
-                m_prmClientGet->set_value(promise_result);
-            } else {
-                LOG(WARNING) << "Received ACTION_BML_CLIENT_GET_CLIENT_RESPONSE response, "
-                             << "but no one is waiting...";
+            if (response->result() != 0) {
+                LOG(ERROR) << "CLIENT_GET_CLIENT_REQUEST failed with error: " << response->result();
+                m_prmClientGet->set_value(false);
+                break;
+            }
+
+            if (!m_client) {
+                LOG(ERROR) << "The pointer to the user client data is null!";
+                m_prmClientGet->set_value(false);
+                break;
             }
+
+            std::copy_n(response->client().sta_mac.oct, BML_MAC_ADDR_LEN, m_client->sta_mac);
+            m_client->timestamp_sec         = response->client().timestamp_sec;
+            m_client->stay_on_initial_radio = response->client().stay_on_initial_radio;
+            // TODO: add stay_on_selected_device to BML_CLIENT when support is added
+            //m_client->stay_on_selected_device = response->client().stay_on_selected_device;
+            m_client->selected_bands       = response->client().selected_bands;
+            m_client->single_band          = response->client().single_band;
+            m_client->time_life_delay_days = response->client().time_life_delay_days;
+
+            //Resolve promise to "true"
+            m_prmClientGet->set_value(true);
         } break;
         default: {
             LOG(WARNING) << "unhandled header BML action type 0x" << std::hex
@@ -1931,7 +1942,7 @@ int bml_internal::client_get_client_list(char *client_list, unsigned int *client
     LOG(DEBUG) << "Promise resolved, received " << client_mac_list.size() << " clients";
     if (!result) {
         LOG(ERROR) << "Get client list request failed";
-        return -BML_RET_OP_FAILED;
+        return (-BML_RET_OP_FAILED);
     }
 
     // Translate sMacAddr list to string
@@ -1954,6 +1965,12 @@ int bml_internal::client_set_client(const sMacAddr &sta_mac, const BML_CLIENT_CO
 {
     LOG(DEBUG) << "client_set_client";
 
+    if ((client_config.stay_on_initial_radio == BML_PARAMETER_NOT_CONFIGURED) &&
+        (client_config.selected_bands == BML_PARAMETER_NOT_CONFIGURED)) {
+        LOG(WARNING) << "No parameter is requested to be configured, returning";
+        return (-BML_RET_INVALID_ARGS);
+    }
+
     auto request =
         message_com::create_vs_message<beerocks_message::cACTION_BML_CLIENT_SET_CLIENT_REQUEST>(
             cmdu_tx);
@@ -1963,9 +1980,11 @@ int bml_internal::client_set_client(const sMacAddr &sta_mac, const BML_CLIENT_CO
         return (-BML_RET_OP_FAILED);
     }
 
-    request->sta_mac()                               = sta_mac;
-    request->client_config().stay_on_initial_radio   = client_config.stay_on_initial_radio;
-    request->client_config().stay_on_selected_device = client_config.stay_on_selected_device;
+    request->sta_mac()                             = sta_mac;
+    request->client_config().stay_on_initial_radio = client_config.stay_on_initial_radio;
+    // TODO: add stay_on_selected_device to BML_CLIENT when support is added
+    //       currently, it is only available as infrastructure in the request/response message
+    request->client_config().stay_on_selected_device = PARAMETER_NOT_CONFIGURED;
     request->client_config().selected_bands          = client_config.selected_bands;
 
     int result = 0;
@@ -2033,7 +2052,15 @@ int bml_internal::client_get_client(const sMacAddr &sta_mac, BML_CLIENT *client)
     // Clear the promise holder
     m_prmClientGet = nullptr;
 
-    LOG_IF(iRet != BML_RET_OK, ERROR) << "Get client request returned with error code:" << iRet;
+    if (iRet != BML_RET_OK) {
+        LOG(ERROR) << "Get client request returned with error code:" << iRet;
+        return (iRet);
+    }
+
+    if (!prmClientGet.get_value()) {
+        LOG(ERROR) << "Get client request failed";
+        return (-BML_RET_OP_FAILED);
+    }
 
     return BML_RET_OK;
 }

controller/src/beerocks/cli/beerocks_cli_bml.cpp

@@ -561,10 +561,9 @@ void cli_bml::setFunctionsMapAndArray()
         "bml_client_set_client", "<sta_mac> [<params>]",
         "Set client with the given STA MAC:"
         " selected_bands - Bitwise parameter, 1 for 2.4G, 2 for 5G, 3 for both, 0 for Disabled"
-        " stay_on_initial_radio - 1 for true, 0 for false or (default) -1 for not configured,"
-        " stay_on_selected_device - 1 for true, 0 for false or (default) -1 for not configured",
-        static_cast<pFunction>(&cli_bml::client_set_client_caller), 2, 4, STRING_ARG, INT_ARG,
-        INT_ARG, INT_ARG);
+        " stay_on_initial_radio - 1 for true, 0 for false or (default) -1 for not configured,",
+        static_cast<pFunction>(&cli_bml::client_set_client_caller), 2, 3, STRING_ARG, STRING_ARG,
+        STRING_ARG);
     insertCommandToMap("bml_client_get_client", "<sta_mac>", "Get client with the given STA MAC.",
                        static_cast<pFunction>(&cli_bml::client_get_client_caller), 1, 1,
                        STRING_ARG);
@@ -1345,13 +1344,11 @@ int cli_bml::client_set_client_caller(int numOfArgs)
      * Optional:
      *  selected_bands=<selected_bands>
      *  stay_on_initial_radio=<stay_on_initial_radio>
-     *  stay_on_selected_device=<stay_on_selected_device>
     ]*/
     std::string::size_type pos;
     std::string sta_mac(network_utils::WILD_MAC_STRING);
-    int8_t selected_bands          = BML_PARAMETER_NOT_CONFIGURED;
-    int8_t stay_on_initial_radio   = BML_PARAMETER_NOT_CONFIGURED;
-    int8_t stay_on_selected_device = BML_PARAMETER_NOT_CONFIGURED;
+    int8_t selected_bands        = BML_PARAMETER_NOT_CONFIGURED;
+    int8_t stay_on_initial_radio = BML_PARAMETER_NOT_CONFIGURED;
     if (numOfArgs > 1) {
         sta_mac = args.stringArgs[0];
         for (int i = 1; i < numOfArgs; i++) { //first optional arg
@@ -1362,14 +1359,9 @@ int cli_bml::client_set_client_caller(int numOfArgs)
                        std::string::npos) {
                 stay_on_initial_radio = string_utils::stoi(
                     args.stringArgs[i].substr(pos + sizeof("stay_on_initial_radio")));
-            } else if ((pos = args.stringArgs[i].find("stay_on_selected_device=")) !=
-                       std::string::npos) {
-                stay_on_selected_device = string_utils::stoi(
-                    args.stringArgs[i].substr(pos + sizeof("stay_on_selected_device")));
             }
         }
-        return client_set_client(sta_mac, selected_bands, stay_on_initial_radio,
-                                 stay_on_selected_device);
+        return client_set_client(sta_mac, selected_bands, stay_on_initial_radio);
     }
     return -1;
 }
@@ -2222,22 +2214,19 @@ int cli_bml::client_get_client_list()
  * @param [in] sta_mac MAC address of requested client.
  * @param [in] selected_bands comma-seperated selected bands.
  * @param [in] stay_on_initial_radio Whather to stay on initial radio or not.
- * @param [in] stay_on_selected_device Whather to stay on selected device or not.
  * @return 0 on success.
  */
 int cli_bml::client_set_client(const std::string &sta_mac, int8_t selected_bands,
-                               int8_t stay_on_initial_radio, int8_t stay_on_selected_device)
+                               int8_t stay_on_initial_radio)
 {
     std::cout << "client_set_client: " << std::endl
               << "  sta_mac: " << sta_mac << std::endl
-              << "  selected_bands: " << selected_bands << std::endl
-              << "  stay_on_initial_radio: " << stay_on_initial_radio << std::endl
-              << "  stay_on_selected_device: " << stay_on_selected_device << std::endl;
+              << "  selected_bands: " << int(selected_bands) << std::endl
+              << "  stay_on_initial_radio: " << int(stay_on_initial_radio) << std::endl;
 
     BML_CLIENT_CONFIG cfg{
-        .stay_on_initial_radio   = stay_on_initial_radio,
-        .stay_on_selected_device = stay_on_selected_device,
-        .selected_bands          = selected_bands,
+        .stay_on_initial_radio = stay_on_initial_radio,
+        .selected_bands        = selected_bands,
     };
 
     int ret = bml_client_set_client(ctx, sta_mac.c_str(), &cfg);
@@ -2270,13 +2259,13 @@ int cli_bml::client_get_client(const std::string &sta_mac)
             if (val == BML_CLIENT_SELECTED_BANDS_DISABLED)
                 return "Disabled";
             if (val & BML_CLIENT_SELECTED_BANDS_24G)
-                ret += "2.4 Ghz,";
+                ret += "2.4GHz,";
             if (val & BML_CLIENT_SELECTED_BANDS_5G)
-                ret += "5 Ghz,";
+                ret += "5GHz,";
             if (val & BML_CLIENT_SELECTED_BANDS_6G)
-                ret += "6 Ghz,";
+                ret += "6GHz,";
             if (val & BML_CLIENT_SELECTED_BANDS_60G)
-                ret += "60 Ghz,";
+                ret += "60GHz,";
 
             // remove ending comma
             if (!ret.empty() && (ret.back() == ',')) {
@@ -2285,12 +2274,10 @@ int cli_bml::client_get_client(const std::string &sta_mac)
 
             return ret;
         };
-        std::cout << "client: " << client.sta_mac << std::endl
+        std::cout << "client: " << tlvf::mac_to_string(client.sta_mac) << std::endl
                   << " timestamp_sec: " << client.timestamp_sec << std::endl
                   << " stay_on_initial_radio: " << client_bool_print(client.stay_on_initial_radio)
                   << std::endl
-                  << " stay_on_selected_device: "
-                  << client_bool_print(client.stay_on_selected_device) << std::endl
                   << " selected_bands: " << client_selected_bands_print(client.selected_bands)
                   << std::endl
                   << " single_band: " << client_bool_print(client.single_band) << std::endl

controller/src/beerocks/cli/beerocks_cli_bml.h

@@ -216,7 +216,7 @@ class cli_bml : public cli {
                              bool is_single_scan = false);
     int client_get_client_list();
     int client_set_client(const std::string &sta_mac, int8_t selected_bands,
-                          int8_t stay_on_initial_radio, int8_t stay_on_selected_device);
+                          int8_t stay_on_initial_radio);
     int client_get_client(const std::string &sta_mac);
     template <typename T> const std::string string_from_int_array(T *arr, size_t arr_max_size);
     // Variable