RDKB-63723: Link Quality Measurement metrics for Gateway#1022
RDKB-63723: Link Quality Measurement metrics for Gateway#1022pramod7456 wants to merge 1 commit intordkcentral:mainfrom
Conversation
pramod7456
commented
Mar 30, 2026
Impacted Platforms: All OneWifi platforms Reason for change: Openwrt is upgraded so put the same changes in upgraded script. Test Procedure: verified the connected clients score and callbacks for station. Risks: Low Signed-off-by:pramod.7456@gmail.com
There was a problem hiding this comment.
Pull request overview
This PR adds Link Quality Measurement support across the OneWifi stack, including a new quality manager implementation, a LinkReport WebConfig subdocument, and control-plane plumbing (RFC knob, RBUS methods/events, and stats pipeline integration) to publish/consume link quality metrics.
Changes:
- Introduces new
quality_mgrandmath_utilscomponents/libraries and wires them into builds. - Adds a new
LinkReportWebConfig subdoc (encode/decode + registration) to publish batch link-quality samples. - Adds LinkQuality app + RFC/config/bus handlers to start/stop collection and expose link quality data/flags.
Reviewed changes
Copilot reviewed 54 out of 54 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
source/webconfig/wifi_webconfig_link_report.c |
New LinkReport subdoc encode/decode implementation. |
source/webconfig/wifi_decoder.c |
Adds decode_link_report() to parse LinkReport JSON into report_batch_t. |
source/webconfig/wifi_encoder.c |
Adds encoding helper for link score samples. |
source/webconfig/wifi_webconfig.c |
Registers the new LinkReport subdoc in webconfig_init(). |
include/wifi_webconfig.h |
Adds new subdoc/object types and decoded data field for qmgr reports. |
include/wifi_base.h |
Introduces sample_t, link_report_t, report_batch_t, RFC field, and station flag. |
source/utils/quality_mgr/* |
New quality manager (qmgr/linkq) implementation + C wrapper interface. |
source/apps/linkquality/* |
New LinkQuality app that bridges qmgr reports to WebConfig/bus events. |
source/apps/wifi_apps_mgr.c |
Adds apps_mgr_link_quality_event() dispatcher. |
source/stats/wifi_stats_assoc_client.c |
Feeds associated-client stats into LinkQuality pipeline + rapid-disconnect hooks. |
source/db/wifi_db_apis.c, source/db/wifi_db.c, source/db/wifi_db.h |
Adds link_quality_rfc handling (schema/cache/update) and a new init hook. |
source/core/wifi_ctrl*.c |
Adds RFC handling, RBUS methods for link quality data/flags, and app start logic. |
config/rdkb-wifi.ovsschema, config/TR181-*.XML |
Adds the RFC parameter into schema and TR-181 model. |
Various Makefile.am / build makefiles |
Adds include paths and links new libs/apps into builds. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| report_batch_t *report = data->u.decoded.qmgr_report; | ||
|
|
||
|
|
||
| json = cJSON_CreateObject(); | ||
| if (json == NULL) { | ||
| wifi_util_error_print(WIFI_CTRL, "%s:%d: json create object failed\n", __func__, __LINE__); | ||
| return webconfig_error_encode; | ||
| } | ||
|
|
||
| data->u.encoded.json = json; | ||
| cJSON_AddStringToObject(json, "Version", "1.0"); | ||
| cJSON_AddStringToObject(json, "SubDocName", "LinkReport"); | ||
| obj_array = cJSON_CreateArray(); | ||
| cJSON_AddItemToObject(json, "LinkReport", obj_array); | ||
| for (i = 0; i < report->link_count ;i++) | ||
| { | ||
| link_report_t link = report->links[i]; | ||
| obj = cJSON_CreateObject(); | ||
| cJSON_AddItemToArray(obj_array, obj); | ||
| cJSON_AddStringToObject(obj, "Mac", link.mac); | ||
| cJSON_AddNumberToObject(obj, "VapIndex", link.vap_index); | ||
| cJSON_AddStringToObject(obj, "ReportingTime", link.reporting_time); | ||
| cJSON_AddNumberToObject(obj, "Threshold", link.threshold); | ||
| cJSON_AddBoolToObject(obj, "Alarm", link.alarm); | ||
| if (encode_link_score_sample_object(&link, obj) != webconfig_error_none) { | ||
| wifi_util_error_print(WIFI_WEBCONFIG, "%s:%d: Failed to encode link object\n", __func__, __LINE__); | ||
| cJSON_Delete(json); | ||
| return webconfig_error_encode; | ||
| } | ||
|
|
||
|
|
||
| } | ||
| // Convert JSON object to string | ||
| str = cJSON_Print(json); | ||
|
|
||
| data->u.encoded.raw = (webconfig_subdoc_encoded_raw_t)calloc(strlen(str) + 1, sizeof(char)); |
There was a problem hiding this comment.
encode_link_report_subdoc() dereferences data->u.decoded.qmgr_report without validating it (and then unconditionally loops over report->link_count). If qmgr_report is NULL this will crash; additionally cJSON_Print() can return NULL and the following strlen(str) will crash. Please add NULL checks for report and for the returned JSON string before using them, and return webconfig_error_encode on failure.
| report_batch_t *report = calloc(1, sizeof(report_batch_t)); | ||
| if (!report) { | ||
| wifi_util_error_print(WIFI_WEBCONFIG, "%s:%d: report calloc Failed\n", __func__, __LINE__); | ||
| return webconfig_error_decode; | ||
| } | ||
|
|
||
| report->link_count = link_count; | ||
| report->links = calloc(link_count, sizeof(link_report_t)); | ||
| if (!report->links) { | ||
| wifi_util_error_print(WIFI_WEBCONFIG, "%s:%d: report->links calloc Failed\n", __func__, __LINE__); | ||
| free(report); | ||
| return webconfig_error_decode; | ||
| } | ||
|
|
||
| for (size_t i = 0; i < link_count; i++) { | ||
|
|
||
| cJSON *link_obj = cJSON_GetArrayItem(link_array, i); | ||
| link_report_t *lr = &report->links[i]; | ||
|
|
||
| /* Mac */ | ||
| cJSON *mac = cJSON_GetObjectItem(link_obj, "Mac"); | ||
| if (cJSON_IsString(mac)) { | ||
| strncpy(lr->mac, mac->valuestring, sizeof(lr->mac) - 1); | ||
| } | ||
|
|
||
| /* VapIndex */ | ||
| cJSON *vap = cJSON_GetObjectItem(link_obj, "VapIndex"); | ||
| lr->vap_index = cJSON_IsNumber(vap) ? vap->valueint : 0; | ||
|
|
||
| /* Threshold */ | ||
| cJSON *threshold = cJSON_GetObjectItem(link_obj, "Threshold"); | ||
| lr->threshold = cJSON_IsNumber(threshold) ? threshold->valuedouble : 0.0; | ||
|
|
||
| /* Alarm */ | ||
| cJSON *alarm = cJSON_GetObjectItem(link_obj, "Alarm"); | ||
| lr->alarm = cJSON_IsBool(alarm) ? cJSON_IsTrue(alarm) : 0; | ||
|
|
||
| /* ReportingTime */ | ||
| cJSON *rt = cJSON_GetObjectItem(link_obj, "ReportingTime"); | ||
| if (cJSON_IsString(rt)) { | ||
| snprintf(lr->reporting_time,sizeof(lr->reporting_time),"%s", | ||
| rt->valuestring); | ||
| } | ||
|
|
||
| /* Samples */ | ||
| cJSON *samples_array = cJSON_GetObjectItem(link_obj, "Samples"); | ||
| if (cJSON_IsArray(samples_array)) { | ||
|
|
||
| size_t sample_count = cJSON_GetArraySize(samples_array); | ||
| lr->sample_count = sample_count; | ||
| lr->samples = calloc(sample_count, sizeof(sample_t)); | ||
| if (lr->samples == NULL) { | ||
| // Allocation failed – handle early exit | ||
| wifi_util_error_print(WIFI_WEBCONFIG,"Failed to allocate memory for %zu samples\n", sample_count); | ||
| lr->sample_count = 0; | ||
| return webconfig_error_decode; | ||
| } |
There was a problem hiding this comment.
decode_link_report(): on failure allocating lr->samples, the function returns immediately without freeing the partially-built report object (report, report->links, and any samples allocated for earlier links). Please add structured cleanup on all error paths so allocations are freed before returning webconfig_error_decode.
| wifi_rfc_dml_parameters_t rfc_config = {0}; | ||
| wifi_mgr_t *g_wifidb; | ||
| g_wifidb = get_wifimgr_obj(); | ||
| rfc_config.link_quality_rfc = true; | ||
| pthread_mutex_lock(&g_wifidb->data_cache_lock); | ||
| memcpy(config,&rfc_config,sizeof(wifi_rfc_dml_parameters_t)); |
There was a problem hiding this comment.
wifidb_init_rfc_config_default() initializes a zeroed local struct and memcpy()s it over *config, only setting link_quality_rfc=true. This unintentionally resets every other RFC field to 0/false, changing defaults globally. Please update only the intended field (or delegate to the existing default initializer that sets all RFC defaults) instead of overwriting the whole struct.
| wifi_rfc_dml_parameters_t rfc_config = {0}; | |
| wifi_mgr_t *g_wifidb; | |
| g_wifidb = get_wifimgr_obj(); | |
| rfc_config.link_quality_rfc = true; | |
| pthread_mutex_lock(&g_wifidb->data_cache_lock); | |
| memcpy(config,&rfc_config,sizeof(wifi_rfc_dml_parameters_t)); | |
| wifi_mgr_t *g_wifidb; | |
| g_wifidb = get_wifimgr_obj(); | |
| pthread_mutex_lock(&g_wifidb->data_cache_lock); | |
| config->link_quality_rfc = true; |
| "MemwrapTool=%d levl_enabled_rfc=%d tcm_enabled_rfc=%d wpa3_compatibility_enable=%d | ||
| link_quality_rfc=%d\r\n", |
There was a problem hiding this comment.
This wifi_util_dbg_print() format string is split across two lines inside a single string literal (newline between %d and link_quality_rfc), which will not compile in C. Please either keep it on one line, or use two adjacent string literals ("...%d " "link_quality_rfc=%d\r\n") so the compiler concatenates them.
| "MemwrapTool=%d levl_enabled_rfc=%d tcm_enabled_rfc=%d wpa3_compatibility_enable=%d | |
| link_quality_rfc=%d\r\n", | |
| "MemwrapTool=%d levl_enabled_rfc=%d tcm_enabled_rfc=%d wpa3_compatibility_enable=%d " | |
| "link_quality_rfc=%d\r\n", |
| json = data->u.encoded.json; | ||
| if (json == NULL) { | ||
| wifi_util_error_print(WIFI_CTRL, "%s:%d: NULL json pointer\n", __func__, __LINE__); | ||
| return webconfig_error_decode; | ||
| } | ||
|
|
||
| if (decode_link_report(json, &data->u.decoded.qmgr_report) != webconfig_error_none) { | ||
| /* use qmgr_report */ | ||
| wifi_util_error_print(WIFI_WEBCONFIG," %s:%d Failed in decoding link report\n",__func__,__LINE__); | ||
| return webconfig_error_decode; | ||
| } | ||
| return webconfig_error_none; |
There was a problem hiding this comment.
decode_link_report_subdoc() returns without deleting data->u.encoded.json. In this codebase, decode_*_subdoc functions are responsible for calling cJSON_Delete(json) before returning (webconfig_set() does not free it). Please ensure json is deleted on both success and error paths to avoid leaking the parsed cJSON tree.
| event = (wifi_event_t *)create_wifi_event((len), type, sub_type); | ||
| if (event == NULL) { | ||
| wifi_util_error_print(WIFI_APPS, "%s %d failed to allocate memory to event\n",__FUNCTION__, __LINE__); | ||
| return RETURN_ERR; | ||
| } | ||
|
|
||
| if (arg == NULL) { | ||
| event->u.core_data.msg = NULL; | ||
| event->u.core_data.len = 0; | ||
| } else { | ||
| /* copy msg to data */ | ||
| event->u.core_data.msg = arg; | ||
| event->u.core_data.len = len; | ||
| event->event_type = type; | ||
| event->sub_type = sub_type; | ||
| } |
There was a problem hiding this comment.
apps_mgr_link_quality_event(): create_wifi_event(len, ...) allocates event->u.core_data.msg when len != 0, but this code overwrites that pointer with arg, leaking the allocated buffer. Either copy into the allocated buffer (memcpy) or create the event with msg_len=0 and then set msg/len so no unused allocation is made.
| void wifidb_init_rfc_config_default(wifi_rfc_dml_parameters_t *config) | ||
| { | ||
| wifi_rfc_dml_parameters_t rfc_config = {0}; | ||
| wifi_mgr_t *g_wifidb; | ||
| g_wifidb = get_wifimgr_obj(); | ||
| rfc_config.link_quality_rfc = true; | ||
| pthread_mutex_lock(&g_wifidb->data_cache_lock); | ||
| memcpy(config,&rfc_config,sizeof(wifi_rfc_dml_parameters_t)); | ||
| pthread_mutex_unlock(&g_wifidb->data_cache_lock); | ||
| wifi_util_info_print(WIFI_CTRL,"%s:%d\n",__func__,__LINE__); | ||
| return ; | ||
| } |
There was a problem hiding this comment.
wifi_db.c defines wifidb_init_rfc_config_default(), but the same symbol is already defined in source/db/wifi_db_apis.c. Building both translation units will result in a multiple-definition linker error. Please remove/rename one of the definitions and call the single canonical implementation.
| void wifidb_init_rfc_config_default(wifi_rfc_dml_parameters_t *config) | |
| { | |
| wifi_rfc_dml_parameters_t rfc_config = {0}; | |
| wifi_mgr_t *g_wifidb; | |
| g_wifidb = get_wifimgr_obj(); | |
| rfc_config.link_quality_rfc = true; | |
| pthread_mutex_lock(&g_wifidb->data_cache_lock); | |
| memcpy(config,&rfc_config,sizeof(wifi_rfc_dml_parameters_t)); | |
| pthread_mutex_unlock(&g_wifidb->data_cache_lock); | |
| wifi_util_info_print(WIFI_CTRL,"%s:%d\n",__func__,__LINE__); | |
| return ; | |
| } | |
| extern void wifidb_init_rfc_config_default(wifi_rfc_dml_parameters_t *config); |
| m_seq[i].set_max(number_t( 25, 0)); | ||
| m_seq[i].set_min(number_t(0, 0)); | ||
| } else if (strncmp(m_linkq_params[i].name, "UPLINK_PHY", strlen("UPLINKK_PHY")) == 0) { | ||
| m_seq[i].set_max(number_t( m_stats_arr[0].dev.cli_MaxUplinkRate, 0)); | ||
| m_seq[i].set_min(number_t(0, 0)); | ||
| } |
There was a problem hiding this comment.
linkq_t::init(): the UPLINK_PHY branch uses strlen("UPLINKK_PHY") (extra 'K'), so the comparison will never match and the uplink PHY max/min will not be initialized. This breaks normalization/scoring for uplink PHY. Please fix the string literal to "UPLINK_PHY" (and keep the same length basis as the other branches).
| double score; | ||
| double snr; | ||
| double per; | ||
| double phy; | ||
| char time[1024]; | ||
| } sample_t; |
There was a problem hiding this comment.
sample_t.time is defined as char[1024], but the code formats timestamps like "%Y-%m-%d %H:%M:%S" which fits in a few dozen bytes. This inflates every stored sample and increases report allocations significantly. Please reduce this buffer to a tighter, documented size (consistent with reporting_time[32]) or store time as an integer/epoch.