[PPM-277] Replace VS message ACTION_CONTROL_BACKHAUL_ROAM_REQUEST by standard one #1578
Conversation
Need to replace vendor-specific message ACTION_CONTROL_BACKHAUL_ROAM_REQUEST by standard one BACKHAUL_STEERING_REQUEST_MESSAGE. Modify method steer_sta() replace creation of vendor specific on standard one with standard tlv, fill up tlv and send message to the agent. https://jira.prplfoundation.org/browse/PPM-277 Signed-off-by: Vladyslav Tupikin <[email protected]>
VladyslavTupikin
requested review from
adam1985d,
arnout,
kantera800,
mariomaz,
morantr,
rmelotte,
tomereli and
vitalybu
as code owners
VladyslavTupikin
changed the title
|
|
||
| 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); |
There was a problem hiding this comment.
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.
| @@ -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 = | |||
| message_com::create_vs_message<beerocks_message::cACTION_CONTROL_BACKHAUL_ROAM_REQUEST>( | |||
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
The message is on beerocks_message_control.h which is not included in this file specifically. Probably included from another file.
| @@ -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 = | |||
There was a problem hiding this comment.
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 -
| auto roam_request = | |
| auto roam_request_1905 = |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Totaly redundant change suggestion
This is a strong statement.
Please read carefully the justification for the change I asked.
| @@ -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 = | |||
There was a problem hiding this comment.
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.
| @@ -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 = | |||
| message_com::create_vs_message<beerocks_message::cACTION_CONTROL_BACKHAUL_ROAM_REQUEST>( | |||
There was a problem hiding this comment.
The message is on beerocks_message_control.h which is not included in this file specifically. Probably included from another file.
Need to replace the vendor-specific message
ACTION_CONTROL_BACKHAUL_ROAM_REQUESTby standard oneBACKHAUL_STEERING_REQUEST_MESSAGE.Modify method
steer_sta()replace the creation of the vendor specific on standard one with standard tlv, fill up tlv and send message to the agent.https://jira.prplfoundation.org/browse/PPM-277