Skip to content
This repository was archived by the owner on Sep 7, 2020. It is now read-only.

[PPM-277] Replace VS message ACTION_CONTROL_BACKHAUL_ROAM_REQUEST by standard one #1578

Open
VladyslavTupikin wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[PPM-277] Replace VS message ACTION_CONTROL_BACKHAUL_ROAM_REQUEST by standard one #1578

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 18 additions & 6 deletions controller/src/beerocks/master/tasks/client_steering_task.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include <bcl/network/network_utils.h>
#include <beerocks/tlvf/beerocks_message_1905_vs.h>
#include <easylogging++.h>
#include <tlvf/wfa_map/tlvBackhaulSteeringRequest.h>
#include <tlvf/wfa_map/tlvClientAssociationControlRequest.h>
#include <tlvf/wfa_map/tlvSteeringRequest.h>

Expand Down Expand Up @@ -146,14 +147,25 @@ void client_steering_task::steer_sta()
TASK_LOG(DEBUG) << "SLAVE " << sta_mac
<< " has an active socket, sending BACKHAUL_ROAM_REQUEST";
auto roam_request =
Copy link
Copy Markdown
Collaborator

@RanRegev RanRegev Aug 5, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we are in a transition from vs to 1905 I think it would be clearer in the code to see the difference in the variables' names. for example -

Suggested change
auto roam_request =
auto roam_request_1905 =

Copy link
Copy Markdown
Collaborator

@morantr morantr Aug 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Totaly redundant change suggestion. The only use for it is to check that we did not receive nullptr. It could be called as well cmdu_header.

Copy link
Copy Markdown
Collaborator

@RanRegev RanRegev Aug 12, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Totaly redundant change suggestion

This is a strong statement.
Please read carefully the justification for the change I asked.

message_com::create_vs_message<beerocks_message::cACTION_CONTROL_BACKHAUL_ROAM_REQUEST>(
Copy link
Copy Markdown
Collaborator

@RanRegev RanRegev Aug 5, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You added the #include to BACKHAUL_STEERING_REQUEST_MESSAGE.
Question: is it possible to remove the #include for cACTION_CONTROL_BACKHAUL_ROAM_REQUEST
(or is it part of a large .h file with many messages)

Copy link
Copy Markdown
Collaborator

@morantr morantr Aug 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The message is on beerocks_message_control.h which is not included in this file specifically. Probably included from another file.

cmdu_tx, 0);
if (roam_request == nullptr) {
LOG(ERROR) << "Failed building message!";
cmdu_tx.create(0, ieee1905_1::eMessageType::BACKHAUL_STEERING_REQUEST_MESSAGE);
if (!roam_request) {
LOG(ERROR) << "Failed building BACKHAUL_STEERING_REQUEST_MESSAGE!";
return;
}
roam_request->params().bssid = tlvf::mac_from_string(target_bssid);
roam_request->params().channel = database.get_node_channel(target_bssid);

auto bh_steer_req_tlv = cmdu_tx.addClass<wfa_map::tlvBackhaulSteeringRequest>();
if (!bh_steer_req_tlv) {
LOG(ERROR) << "Failed building addClass<wfa_map::tlvSteeringRequest!";
return;
}

bh_steer_req_tlv->backhaul_station_mac() = tlvf::mac_from_string(sta_mac);
bh_steer_req_tlv->target_bssid() = tlvf::mac_from_string(target_bssid);
bh_steer_req_tlv->target_channel_number() = database.get_node_channel(target_bssid);
Copy link
Copy Markdown
Collaborator

@vitalii-komisarenko vitalii-komisarenko Aug 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really like that the result of get_node_channel is not checked here, but since it is not checked in the other places of the codebase, it is probably fine.

bh_steer_req_tlv->operating_class() =
database.get_hostap_operating_class(tlvf::mac_from_string(target_bssid));
bh_steer_req_tlv->finalize();

son_actions::send_cmdu_to_agent(agent_mac, cmdu_tx, database, radio_mac);

// update bml listeners
Expand Down