Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the stability issues in controller and agent #221

Merged
merged 3 commits into from
Mar 21, 2025
Merged

Fix the stability issues in controller and agent #221

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ea1aa9b
Fix the stability issues in controller and agent
amarnathhullur Mar 12, 2025
7e17e68
Merge branch 'rdkcentral:main' into fix
AswiniViswanathan Mar 20, 2025
bf6bcac
Merge branch 'main' into fix
amarnathhullur Mar 21, 2025
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
3 changes: 2 additions & 1 deletion inc/em_base.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1858,7 +1858,9 @@ typedef enum {
em_state_agent_onewifi_bssconfig_ind,
em_state_agent_autoconfig_renew_pending,
em_state_agent_topo_synchronized,
em_state_agent_channel_pref_query,
em_state_agent_channel_selection_pending,
em_state_agent_channel_select_configuration_pending,
em_state_agent_channel_report_pending,
em_state_agent_channel_scan_result_pending,
em_state_agent_configured,
Expand All @@ -1867,7 +1869,6 @@ typedef enum {
em_state_agent_topology_notify,
em_state_agent_ap_cap_report,
em_state_agent_client_cap_report,
em_state_agent_channel_pref_query,
em_state_agent_sta_link_metrics,
em_state_agent_steer_btm_res_pending,
em_state_agent_beacon_report_pending,
Expand Down
95 changes: 67 additions & 28 deletions src/agent/em_agent.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,8 +288,8 @@ void em_agent_t::handle_autoconfig_renew(em_bus_event_t *evt)
unsigned int num;

if (m_orch->is_cmd_type_in_progress(evt->type) == true) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this check removed? Is the check not required?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does not allow autoconfig renew simultaneously for both 2.4 and 5G

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reverted since its not required

printf("handle_autoconfig_renew in progress\n");
} else if ((num = m_data_model.analyze_autoconfig_renew(evt, pcmd)) == 0) {
printf("handle_autoconfig_renew in progress\n");
} else if ((num = m_data_model.analyze_autoconfig_renew(evt, pcmd)) == 0) {
printf("handle_autoconfig_renew cmd creation failed\n");
} else if (m_orch->submit_commands(pcmd, num) > 0) {
}
Expand Down Expand Up @@ -878,34 +878,73 @@ em_t *em_agent_t::find_em_for_msg_type(unsigned char *data, unsigned int len, em
cmdu = (em_cmdu_t *)(data + sizeof(em_raw_hdr_t));

switch (htons(cmdu->type)) {
case em_msg_type_autoconf_resp:
case em_msg_type_autoconf_renew:
if (em_msg_t(data + (sizeof(em_raw_hdr_t) + sizeof(em_cmdu_t)),
len - (sizeof(em_raw_hdr_t) + sizeof(em_cmdu_t))).get_freq_band(&band) == false) {
printf("%s:%d: Could not find frequency band\n", __func__, __LINE__);
return NULL;
}
case em_msg_type_autoconf_resp:
found = false;
if (em_msg_t(data + (sizeof(em_raw_hdr_t) + sizeof(em_cmdu_t)),
len - (sizeof(em_raw_hdr_t) + sizeof(em_cmdu_t))).get_freq_band(&band) == false) {
printf("%s:%d: Could not find frequency band\n", __func__, __LINE__);
return NULL;
}

em = (em_t *)hash_map_get_first(m_em_map);
while (em != NULL) {
if (!(em->is_al_interface_em())) {
if (em->is_matching_freq_band(&band) == true) {
if ((em->get_state() != em_state_agent_autoconfig_renew_pending) && (em->get_state() !=em_state_agent_wsc_m2_pending) && (em->get_state() != em_state_agent_owconfig_pending) ) {
found = true;
break;
} else {
printf("%s:%d: Found matching band%d but incorrect em state %d\n", __func__, __LINE__, band, em->get_state());
}
}
}
em = (em_t *)hash_map_get_next(m_em_map, em);
}
if (found == false) {
printf("%s:%d: Could not find em with matching band%d and expected state \n", __func__, __LINE__, band);
return NULL;
}
em = (em_t *)hash_map_get_first(m_em_map);
while (em != NULL) {
if (!(em->is_al_interface_em())) {
if (em->is_matching_freq_band(&band) == true) {
if ((em->get_state() != em_state_agent_autoconfig_renew_pending) && (em->get_state() !=em_state_agent_wsc_m2_pending) && (em->get_state() != em_state_agent_owconfig_pending) ) {
found = true;
break;
} else {
printf("%s:%d: Found matching band%d but incorrect em state %d\n", __func__, __LINE__, band, em->get_state());
}
}
}
em = (em_t *)hash_map_get_next(m_em_map, em);
}
if (found == false) {
printf("%s:%d: Could not find em with matching band%d and expected state \n", __func__, __LINE__, band);
return NULL;
}

break;
break;
case em_msg_type_autoconf_renew:
found = false;
if (em_msg_t(data + (sizeof(em_raw_hdr_t) + sizeof(em_cmdu_t)),
len - (sizeof(em_raw_hdr_t) + sizeof(em_cmdu_t))).get_freq_band(&band) == false) {
printf("%s:%d: Could not find frequency band\n", __func__, __LINE__);
return NULL;
}

if (em_msg_t(data + (sizeof(em_raw_hdr_t) + sizeof(em_cmdu_t)),
len - (sizeof(em_raw_hdr_t) + sizeof(em_cmdu_t))).get_al_mac_address(ruid) == false) {
printf("%s:%d: Could not find radio_id for em_msg_type_topo_query\n", __func__, __LINE__);
return NULL;
}
dm_easy_mesh_t::macbytes_to_string(ruid, mac_str1);
if ((em = (em_t *)hash_map_get(m_em_map, mac_str1)) != NULL) {
printf("%s:%d: Found existing AL MAC:%s\n", __func__, __LINE__, mac_str1);
} else {
return NULL;
}
em = (em_t *)hash_map_get_first(m_em_map);
while (em != NULL) {
if (!(em->is_al_interface_em())) {
if (em->is_matching_freq_band(&band) == true) {
if ((em->get_state() != em_state_agent_autoconfig_renew_pending) && (em->get_state() !=em_state_agent_wsc_m2_pending) && (em->get_state() != em_state_agent_owconfig_pending) ) {
found = true;
break;
} else {
printf("%s:%d: Found matching band%d but incorrect em state %d\n", __func__, __LINE__, band, em->get_state());
return NULL;
}
}
}
em = (em_t *)hash_map_get_next(m_em_map, em);
}
if (found == false) {
printf("%s:%d: Could not find em with matching band%d and expected state \n", __func__, __LINE__, band);
return NULL;
}
break;
case em_msg_type_chirp_notif:

case em_msg_type_autoconf_wsc:
Expand Down
2 changes: 1 addition & 1 deletion src/dm/dm_bss_list.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ dm_orch_type_t dm_bss_list_t::get_dm_orch_type(db_client_t& db_client, const dm_
pbss = get_bss(key);

if (pbss != NULL) {
if (entry_exists_in_table(db_client, bss_mac_str) == false) {
if (entry_exists_in_table(db_client, key) == false) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a significant change to have the key be different, Has this change been properly tested?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes amarnath. I did test it and it works fine. Issue is while adding an entry key is construction as combination of bss mac haultype but this check uses only mac.

return dm_orch_type_db_insert;
}

Expand Down
2 changes: 1 addition & 1 deletion src/dm/dm_easy_mesh_list.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ void dm_easy_mesh_list_t::put_radio(const char *key, const dm_radio_t *radio)
}
*pradio = *radio;

if ((em = m_mgr->create_node(&pradio->m_radio_info.intf, (em_freq_band_t)pradio->m_radio_info.media_data.band, dm, false,
if ((em = m_mgr->create_node(&pradio->m_radio_info.intf, (em_freq_band_t)pradio->m_radio_info.band, dm, false,
em_profile_type_3, em_service_type_ctrl)) != NULL) {
printf("%s:%d Node created successfully\n", __func__, __LINE__);
}
Expand Down
5 changes: 3 additions & 2 deletions src/dm/dm_radio_list.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ int dm_radio_list_t::update_db(db_client_t& db_client, dm_orch_type_t op, void *
switch (op) {
case dm_orch_type_db_insert:
ret = insert_row(db_client, key, radio_mac_str, info->enabled,
info->media_data.media_type, info->media_data.band, info->media_data.center_freq_index_1, info->media_data.center_freq_index_1,
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the band in media_data which was earlier used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

media_data is a new variable introduced recently, not exactly sure on the exact purpose. Band is the variable which is updated in webconfig translator and also on the agent side implementation.

Copy link
Contributor

Choose a reason for hiding this comment

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

Should the webconfig translator and the agent needs to change to update media_data.band instead of using info.band?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will discuss with soumya and get the information on media_data struct and update it the right way, but for M2 we have to updated the band variable.

info->media_data.media_type, info->band, info->media_data.center_freq_index_1, info->media_data.center_freq_index_1,
info->number_of_bss, info->number_of_unassoc_sta, info->noise, info->utilization,
info->traffic_sep_combined_fronthaul,
info->traffic_sep_combined_backhaul, info->steering_policy, info->channel_util_threshold, info->rcpi_steering_threshold,
Expand All @@ -207,7 +207,7 @@ int dm_radio_list_t::update_db(db_client_t& db_client, dm_orch_type_t op, void *

case dm_orch_type_db_update:
ret = update_row(db_client, radio_mac_str, info->enabled,
info->media_data.media_type, info->media_data.band, info->media_data.center_freq_index_1, info->media_data.center_freq_index_1,
info->media_data.media_type, info->band, info->media_data.center_freq_index_1, info->media_data.center_freq_index_1,
info->number_of_bss, info->number_of_unassoc_sta, info->noise, info->utilization,
info->traffic_sep_combined_fronthaul,
info->traffic_sep_combined_backhaul, info->steering_policy, info->channel_util_threshold, info->rcpi_steering_threshold,
Expand Down Expand Up @@ -261,6 +261,7 @@ int dm_radio_list_t::sync_db(db_client_t& db_client, void *ctx)
info.enabled = db_client.get_number(ctx, 3);
info.media_data.media_type = db_client.get_number(ctx, 4);
info.media_data.band = db_client.get_number(ctx, 5);
info.band = (em_freq_band_t) db_client.get_number(ctx, 5);
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the need for this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

media_data is not updated because of which frequency band is also not updated. Modified to use band variable which is updated from M1.

info.media_data.center_freq_index_1 = db_client.get_number(ctx, 6);
info.media_data.center_freq_index_2 = db_client.get_number(ctx, 7);
info.number_of_bss = db_client.get_number(ctx, 8);
Expand Down
8 changes: 6 additions & 2 deletions src/em/channel/em_channel.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1723,12 +1723,13 @@ int em_channel_t::handle_channel_sel_req(unsigned char *buff, unsigned int len)
get_mgr()->io_process(em_bus_event_type_channel_sel_req, reinterpret_cast<unsigned char *> (&op_class), sizeof(op_class_channel_sel));

printf("%s:%d Received channel selection request \n",__func__, __LINE__);

set_state(em_state_agent_channel_select_configuration_pending);
return 0;
}

int em_channel_t::handle_channel_sel_rsp(unsigned char *buff, unsigned int len)
{
printf("%s:%d Received channel selection response \n",__func__, __LINE__);
set_state(em_state_ctrl_channel_selected);
return 0;
}
Expand Down Expand Up @@ -1756,6 +1757,7 @@ int em_channel_t::handle_operating_channel_rprt(unsigned char *buff, unsigned in
tlv_len -= static_cast<int> (sizeof(em_tlv_t) + htons(tlv->len));
tlv = reinterpret_cast<em_tlv_t *> (reinterpret_cast<unsigned char *> (tlv) + sizeof(em_tlv_t) + htons(tlv->len));
}
printf("%s:%d Operating channel report recv\n", __func__, __LINE__);
set_state(em_state_ctrl_configured);

return 0;
Expand Down Expand Up @@ -1950,7 +1952,9 @@ void em_channel_t::process_msg(unsigned char *data, unsigned int len)
break;

case em_msg_type_channel_sel_rsp:
handle_channel_sel_rsp(data, len);
if (get_service_type() == em_service_type_ctrl) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the understanding that this needs to be processed only in controller?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes thats correct

handle_channel_sel_rsp(data, len);
}
break;

case em_msg_type_op_channel_rprt:
Expand Down
76 changes: 55 additions & 21 deletions src/em/config/em_configuration.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ int em_configuration_t::send_autoconfig_renew_msg()
tlv = reinterpret_cast<em_tlv_t *> (tmp);
tlv->type = em_tlv_type_al_mac_address;
tlv->len = htons(sizeof(mac_address_t));
memcpy(tlv->value,get_current_cmd()->get_al_interface_mac(), sizeof(mac_address_t));
memcpy(tlv->value, dm->get_agent_al_interface_mac(), sizeof(mac_address_t));

tmp += (sizeof (em_tlv_t) + sizeof(mac_address_t));
len += static_cast<unsigned int> (sizeof (em_tlv_t) + sizeof(mac_address_t));
Expand All @@ -255,7 +255,7 @@ int em_configuration_t::send_autoconfig_renew_msg()
tlv = reinterpret_cast<em_tlv_t *> (tmp);
tlv->type = em_tlv_type_supported_freq_band;
tlv->len = htons(sizeof(unsigned char));
freq_band = static_cast<em_freq_band_t> (get_band() >> 1);
freq_band = static_cast<em_freq_band_t> (get_band());
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a significant change as well. Why is the right shift not required now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

em_tlv_type_supported_freq_band spec shows different mapping
tlvValue 1 octet
0x00: 802.11 2.4 GHz
0x01: 802.11 5 GHz
0x02: 802.11 60 GHz

RF band is used in some ieee packets so below format is mentioned,
RF Band Value Description
0x01 2.4GHz
0x02 5.0GH

Copy link
Contributor

@bcarlson-dev bcarlson-dev Mar 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Can you add some documentation to the em_freq_band_t struct detailing this discrepancy and that the same struct is used for both versions so a conversion is necessary? Additionally it might be nice explicitly assign the hex values to the enum cases to make it a bit more explicit at quick glance which version is being used by default.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

typedef enum {
em_freq_band_24, //IEEE-1905-1-2013 table 6-23
em_freq_band_5,
em_freq_band_60,
em_freq_band_unknown
} em_freq_band_t;
In the header file, the spec is mentioned.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, however, based on that reference, someone would think that it is only used for AutoconfigFreqBand and maybe SupportedFreqBand (6.4.17). It is used other places though as a general enum representing 2.4, 5, and 60 GHz (as we see with the RF band), so I think a more detailed documentation should be added saying that this enum is used in places outside of 1905 TLVs at which point integer conversion may be necessary. It might even be a good idea to call out the RF Band specifically as a conversion that occurs based on its different representation in the IEEE spec.

It doesn't hurt, especially when you are using the same enum across two different specs, with two different representations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure Ben, I will add few more details in comments

Copy link
Contributor

Choose a reason for hiding this comment

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

Merging this after indicating Aswini to update the comment in the next commit.

memcpy(&tlv->value, &freq_band, sizeof(unsigned char));

tmp += (sizeof (em_tlv_t) + 1);
Expand Down Expand Up @@ -826,7 +826,9 @@ void em_configuration_t::handle_ap_vendor_operational_bss(unsigned char *value,
em_ap_vendor_operational_bss_t *bss;
em_ap_vendor_op_bss_t *ap;
unsigned int i, j, all_bss_len = 0;

dm_bss_t *dm_bss;
dm_easy_mesh_t *dm;
dm = get_data_model();
ap = reinterpret_cast<em_ap_vendor_op_bss_t *> (value);
radio = ap->radios;
printf("%s:%d Number of radios: %d\n", __func__, __LINE__, ap->radios_num);
Expand All @@ -838,6 +840,19 @@ void em_configuration_t::handle_ap_vendor_operational_bss(unsigned char *value,
for (j = 0; j < radio->bss_num; j++) {
dm_easy_mesh_t::macbytes_to_string(bss->bssid, bss_mac_str);
printf("%s:%d: BSSID=%s haul type=%d\n", __func__, __LINE__, bss_mac_str, bss->haultype);
dm_bss = dm->get_bss(radio->ruid, bss->bssid);
if (dm_bss == NULL) {
dm_bss = &dm->m_bss[dm->m_num_bss];
dm->set_num_bss(dm->get_num_bss() + 1);
}
// fill up id first
strncpy(dm_bss->m_bss_info.id.net_id, dm->m_device.m_device_info.id.net_id, strlen(dm->m_device.m_device_info.id.net_id) + 1);
memcpy(dm_bss->m_bss_info.id.dev_mac, dm->m_device.m_device_info.intf.mac, sizeof(mac_address_t));
memcpy(dm_bss->m_bss_info.id.ruid, radio->ruid, sizeof(mac_address_t));
memcpy(dm_bss->m_bss_info.id.bssid, bss->bssid, sizeof(mac_address_t));
memcpy(dm_bss->m_bss_info.ruid.mac, radio->ruid, sizeof(mac_address_t));
memcpy(dm_bss->m_bss_info.bssid.mac, bss->bssid, sizeof(mac_address_t));
dm_bss->m_bss_info.id.haul_type = (em_haul_type_t) bss->haultype;
bss = reinterpret_cast<em_ap_vendor_operational_bss_t *>(reinterpret_cast<unsigned char *> (bss) + sizeof(em_ap_vendor_operational_bss_t));
all_bss_len += sizeof(em_ap_vendor_operational_bss_t);
}
Expand Down Expand Up @@ -1609,7 +1624,19 @@ int em_configuration_t::handle_topology_response(unsigned char *buff, unsigned i

tlv = reinterpret_cast<em_tlv_t *> (buff + sizeof(em_raw_hdr_t) + sizeof(em_cmdu_t));
tmp_len = len - static_cast<unsigned int> (sizeof(em_raw_hdr_t) + sizeof(em_cmdu_t));


while ((tlv->type != em_tlv_type_eom) && (tmp_len > 0)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like 1628 to 1637 is same as that of 1640 to 1651, why is this while loop repeated.

Copy link
Contributor

Choose a reason for hiding this comment

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

Need more comments

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As discussed the loop is required since we have to process one TLV after another in specific order only

if (tlv->type != em_tlv_type_vendor_operational_bss) {
tmp_len -= static_cast<unsigned int> (sizeof(em_tlv_t) + htons(tlv->len));
tlv = reinterpret_cast<em_tlv_t *> (reinterpret_cast<unsigned char *> (tlv) + sizeof(em_tlv_t) + htons(tlv->len));
continue;
} else {
handle_ap_vendor_operational_bss(tlv->value, tlv->len);
break;
}
}
tlv = reinterpret_cast<em_tlv_t *> (buff + sizeof(em_raw_hdr_t) + sizeof(em_cmdu_t));
tmp_len = len - static_cast<unsigned int> (sizeof(em_raw_hdr_t) + sizeof(em_cmdu_t));
while ((tlv->type != em_tlv_type_eom) && (tmp_len > 0)) {
if (tlv->type != em_tlv_type_operational_bss) {
tmp_len -= static_cast<unsigned int> (sizeof(em_tlv_t) + htons(tlv->len));
Expand Down Expand Up @@ -1656,16 +1683,6 @@ int em_configuration_t::handle_topology_response(unsigned char *buff, unsigned i
return -1;
}

while ((tlv->type != em_tlv_type_eom) && (tmp_len > 0)) {
if (tlv->type != em_tlv_type_vendor_operational_bss) {
tmp_len -= static_cast<unsigned int> (sizeof(em_tlv_t) + htons(tlv->len));
tlv = reinterpret_cast<em_tlv_t *> (reinterpret_cast<unsigned char *> (tlv) + sizeof(em_tlv_t) + htons(tlv->len));
continue;
} else {
handle_ap_vendor_operational_bss(tlv->value, tlv->len);
break;
}
}

dm->set_db_cfg_param(db_cfg_type_policy_list_update, "");
return ret;
Expand Down Expand Up @@ -2023,12 +2040,12 @@ unsigned short em_configuration_t::create_m2_msg(unsigned char *buff, em_haul_ty
len += static_cast<unsigned short int> (sizeof(data_elem_attr_t) + size);
tmp += (sizeof(data_elem_attr_t) + size);

// rf bands
// rf bands Table 6.1.3 - RF Band
attr = reinterpret_cast<data_elem_attr_t *> (tmp);
attr->id = htons(attr_id_rf_bands);
size = 1;
attr->len = htons(size);
rf_band = get_band();
rf_band = static_cast<em_freq_band_t> (1 << get_band());
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure about this change. Need discussion

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As mentioned before, adhering to spec.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is fine but I think there should be some documentation here to avoid the understandable confusion here on why an em_freq_band_t is being converted to an em_freq_band_t.

Maybe something as simple as "Converting from EM TLV Freq Band representation to RF Freq Band Representation. See em_freq_band_t for more information"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it. Will add them

memcpy(attr->val, &rf_band, size);

len += static_cast<unsigned short int> (sizeof(data_elem_attr_t) + size);
Expand Down Expand Up @@ -2278,7 +2295,7 @@ unsigned short em_configuration_t::create_m1_msg(unsigned char *buff)
len += static_cast<unsigned short int> (sizeof(data_elem_attr_t) + size);
tmp += (sizeof(data_elem_attr_t) + size);

// rf bands
// rf bands Table 6.1.3 - RF Band
attr = reinterpret_cast<data_elem_attr_t *> (tmp);
attr->id = htons(attr_id_rf_bands);
size = 1;
Expand Down Expand Up @@ -2891,11 +2908,26 @@ int em_configuration_t::handle_wsc_m1(unsigned char *buff, unsigned int len)
mac_addr_str_t mac_str;
em_device_info_t dev_info;
dm_easy_mesh_t *dm;
em_freq_band_t band;
em_freq_band_t band;
dm_radio_t *radio;
unsigned int found = 0, i = 0;

dm = get_data_model();
dm = get_data_model();
memset(&dev_info, 0, sizeof(em_device_info_t));


for (i = 0; i < dm->m_num_radios; i++) {
radio = dm->get_radio(i);
if (memcmp(radio->get_radio_interface_mac(), get_radio_interface_mac(), sizeof(mac_address_t)) != 0) {
continue;
}
found++;
break;
}

if (found == 0) {
printf("%s:%d Failed to find the radio\n", __func__, __LINE__);
return -1;
}
m_m1_length = len;
memcpy(m_m1_msg, buff, m_m1_length);

Expand Down Expand Up @@ -2946,9 +2978,11 @@ int em_configuration_t::handle_wsc_m1(unsigned char *buff, unsigned int len)
} else if (id == attr_id_primary_device_type) {
} else if (id == attr_id_device_name) {
} else if (id == attr_id_rf_bands) {
band = static_cast<em_freq_band_t> (attr->val[0]);
band = static_cast<em_freq_band_t> (attr->val[0] >> 1);
printf("%s:%d Freq band = %d \n", __func__, __LINE__,band);
set_band(band);
radio->get_radio_info()->band = band;
dm->set_db_cfg_param(db_cfg_type_radio_list_update, "");
} else if (id == attr_id_assoc_state) {
} else if (id == attr_id_device_password_id) {
} else if (id == attr_id_cfg_error) {
Expand Down
6 changes: 5 additions & 1 deletion src/em/em.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,11 @@ void em_t::orch_execute(em_cmd_t *pcmd)
break;

case em_cmd_type_set_channel:
m_sm.set_state(em_state_ctrl_channel_select_pending);
if (pcmd->get_orch_op() == dm_orch_type_channel_sel) {
m_sm.set_state(em_state_ctrl_channel_select_pending);
} else if ((pcmd->get_orch_op() == dm_orch_type_channel_cnf) && (m_sm.get_state() == em_state_ctrl_channel_selected)) {
m_sm.set_state(em_state_ctrl_channel_cnf_pending);
}
break;

case em_cmd_type_scan_channel:
Expand Down
5 changes: 3 additions & 2 deletions src/orch/em_orch_agent.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ unsigned int em_orch_agent_t::build_candidates(em_cmd_t *pcmd)
}
break;
case em_cmd_type_cfg_renew:
if (memcmp(pcmd->get_data_model()->get_radio(num)->get_radio_info()->intf.mac, em->get_radio_interface_mac(), sizeof(mac_address_t)) == 0) {
if ((memcmp(pcmd->get_data_model()->get_radio(num)->get_radio_info()->intf.mac, em->get_radio_interface_mac(), sizeof(mac_address_t)) == 0) && (!(em->is_al_interface_em()))) {
queue_push(pcmd->m_em_candidates, em);
count++;
}
Expand Down Expand Up @@ -369,6 +369,7 @@ unsigned int em_orch_agent_t::build_candidates(em_cmd_t *pcmd)
printf("%s:%d em_cmd_type_channel_pref_query radio cannot be found.\n", __func__, __LINE__);
break;
}

if ((memcmp(radio->get_radio_interface_mac(),em->get_radio_interface_mac(),sizeof(mac_address_t)) == 0)
&& (em->get_state() >= em_state_agent_topo_synchronized)
&& (em->get_state() < em_state_agent_configured)) {
Expand All @@ -386,7 +387,7 @@ unsigned int em_orch_agent_t::build_candidates(em_cmd_t *pcmd)
printf("%s:%d channel sel radio cannot be found.\n", __func__, __LINE__);
break;
}
if ((memcmp(radio->get_radio_interface_mac(),em->get_radio_interface_mac(),sizeof(mac_address_t)) == 0) && (em->get_state() == em_state_agent_channel_selection_pending)) {
if ((memcmp(radio->get_radio_interface_mac(),em->get_radio_interface_mac(),sizeof(mac_address_t)) == 0) && (em->get_state() == em_state_agent_channel_select_configuration_pending)) {
queue_push(pcmd->m_em_candidates, em);
count++;
dm_easy_mesh_t::macbytes_to_string(em->get_radio_interface_mac(), dst_mac_str);
Expand Down
Loading