From 23585e8187caa925cdd0994e8b9e806eb50e078f Mon Sep 17 00:00:00 2001 From: Itay Elenzweig Date: Sun, 26 Jul 2020 09:50:15 +0000 Subject: [PATCH 1/5] bml: BML_CLIENT: change sta_mac type to unit8_t The type of the mac address parameter is incorrect. Changed sta_mac from char[6] to uint8_t[6]. Signed-off-by: Itay Elenzweig --- controller/src/beerocks/bml/bml_defs.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/controller/src/beerocks/bml/bml_defs.h b/controller/src/beerocks/bml/bml_defs.h index 9e080d44ff..57ec4e71af 100644 --- a/controller/src/beerocks/bml/bml_defs.h +++ b/controller/src/beerocks/bml/bml_defs.h @@ -599,7 +599,7 @@ struct BML_CLIENT_CONFIG { 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; From 458697e712fdef30fc8813e435cdc1ebd55f52e3 Mon Sep 17 00:00:00 2001 From: Itay Elenzweig Date: Sun, 26 Jul 2020 09:53:02 +0000 Subject: [PATCH 2/5] beerocks_cli_bml: Fix client APIs - In client_set_client changed incoming type from INT_ARG to STRING_ARG. Since we are using optional parameters, we need to retrieve them from the CLI as strings, then convert them to integers. - Added int conversion in the client_set_client LOG print. To print the incoming values in client_set_client, need to convert them to type int, otherwise they will be printed as Char - Added tlvf::mac_to_string in the client_get_client LOG print. The received mac address needs to be converted to a readable format to be printed. Signed-off-by: Itay Elenzweig --- controller/src/beerocks/cli/beerocks_cli_bml.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/controller/src/beerocks/cli/beerocks_cli_bml.cpp b/controller/src/beerocks/cli/beerocks_cli_bml.cpp index 7883fe482b..42d7afe154 100644 --- a/controller/src/beerocks/cli/beerocks_cli_bml.cpp +++ b/controller/src/beerocks/cli/beerocks_cli_bml.cpp @@ -563,8 +563,8 @@ void cli_bml::setFunctionsMapAndArray() " 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(&cli_bml::client_set_client_caller), 2, 4, STRING_ARG, INT_ARG, - INT_ARG, INT_ARG); + static_cast(&cli_bml::client_set_client_caller), 2, 4, STRING_ARG, STRING_ARG, + STRING_ARG, STRING_ARG); insertCommandToMap("bml_client_get_client", "", "Get client with the given STA MAC.", static_cast(&cli_bml::client_get_client_caller), 1, 1, STRING_ARG); @@ -2230,9 +2230,9 @@ int cli_bml::client_set_client(const std::string &sta_mac, int8_t selected_bands { 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 + << " stay_on_selected_device: " << int(stay_on_selected_device) << std::endl; BML_CLIENT_CONFIG cfg{ .stay_on_initial_radio = stay_on_initial_radio, @@ -2285,7 +2285,7 @@ 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 From d2363efc400beae743b1761f88a5cbb4f2e65033 Mon Sep 17 00:00:00 2001 From: Itay Elenzweig Date: Mon, 27 Jul 2020 17:28:04 +0300 Subject: [PATCH 3/5] controller: bml: fix return values Changed returning error values in the code to a negative value as expected by the BML. PPM-5. Signed-off-by: Itay Elenzweig --- controller/src/beerocks/bml/internal/bml_internal.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/controller/src/beerocks/bml/internal/bml_internal.cpp b/controller/src/beerocks/bml/internal/bml_internal.cpp index 43f5e6e84f..dcc4462972 100644 --- a/controller/src/beerocks/bml/internal/bml_internal.cpp +++ b/controller/src/beerocks/bml/internal/bml_internal.cpp @@ -1141,13 +1141,13 @@ int bml_internal::process_cmdu_header(std::shared_ptr beerocks_ ->addClass(); 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_ ->addClass(); 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 @@ -1776,7 +1776,7 @@ int bml_internal::get_dcs_scan_results(const sMacAddr &mac, BML_NEIGHBOR_AP *res if (!m_prmChannelScanResultsGet->wait_for(iOpTimeout)) { LOG(WARNING) << "Timeout while waiting for results get response..."; - iRet = -BML_RET_TIMEOUT; + iRet = (-BML_RET_TIMEOUT); } // Clear the scan results members @@ -1931,7 +1931,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 From 24ff59cf9dcf659f2300d3f6addc7b451d0ed8a2 Mon Sep 17 00:00:00 2001 From: Itay Elenzweig Date: Mon, 27 Jul 2020 17:28:51 +0300 Subject: [PATCH 4/5] controller: bml: refactor client_get_client Refactored the response handling for the "get_client" implementation. Fixed broken return on error in the "get_client" implementation. PPM-5. Signed-off-by: Itay Elenzweig --- .../beerocks/bml/internal/bml_internal.cpp | 56 ++++++++++++------- 1 file changed, 37 insertions(+), 19 deletions(-) diff --git a/controller/src/beerocks/bml/internal/bml_internal.cpp b/controller/src/beerocks/bml/internal/bml_internal.cpp index dcc4462972..eacb9c2757 100644 --- a/controller/src/beerocks/bml/internal/bml_internal.cpp +++ b/controller/src/beerocks/bml/internal/bml_internal.cpp @@ -1207,6 +1207,13 @@ int bml_internal::process_cmdu_header(std::shared_ptr 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(); @@ -1215,25 +1222,28 @@ int bml_internal::process_cmdu_header(std::shared_ptr beerocks_ 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; + 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 @@ -2033,7 +2043,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; } From 02824b5173f0cd88d7f108a14f54616ceaa7cd08 Mon Sep 17 00:00:00 2001 From: Itay Elenzweig Date: Tue, 28 Jul 2020 07:48:32 +0300 Subject: [PATCH 5/5] controller: bml: verify client_config parameters As part of the persistent DB, some configuration parameters can be set to "NOT_CONFIGURED" meaning they will not be stored persistently. Verify that the given client configuration parameters are configured before sending the change request. Signed-off-by: Itay Elenzweig --- controller/src/beerocks/bml/internal/bml_internal.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/controller/src/beerocks/bml/internal/bml_internal.cpp b/controller/src/beerocks/bml/internal/bml_internal.cpp index eacb9c2757..7244f4cb88 100644 --- a/controller/src/beerocks/bml/internal/bml_internal.cpp +++ b/controller/src/beerocks/bml/internal/bml_internal.cpp @@ -1964,6 +1964,13 @@ 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.stay_on_selected_device == 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( cmdu_tx);