RDKB-61190:[OneWifi] Implemented HE bus async method invoke API #541

Aniket0606 · 2025-08-18T22:50:36Z

Reason for change: Implemented the HE bus asynchronous method invoke
API to asynchronously set and get data between a
consumer and a producer.

Test Procedure: 1) Load Linux(RPI) or OpenWRT(BPI) HE bus Build. 2) Trigger "bus_method_async_invoke" API from code to test those changes. 3) Verify device memory.

Risks: Low
Priority: P1

Signed-off-by: [email protected]

CLAassistant · 2025-08-18T22:50:43Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ amarnathhullur
❌ Aniket0606
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

amarnathhullur

Can you also please list the testing (various unit test scenarios) addressed as part of this change?

amarnathhullur · 2025-08-18T23:33:11Z

source/platform/common/bus_common.c

+{
+    bus_data_prop_t *next;
+
+    if (p_data_prop->ref_count <= 1) {


shouldn't this be just == 1. If it goes below 1 it should already have been freed right and should not do anything.

amarnathhullur · 2025-08-18T23:33:32Z

source/platform/common/bus_common.c

+        free_bus_raw_data(&p_data_prop->value);
+        p_data_prop->is_data_set = false;
+        p_data_prop->ref_count = 0;
+    } else {


Should explicitly check for > 1 for this.

amarnathhullur · 2025-08-18T23:33:53Z

source/platform/common/bus_common.c

+
+    while(p_data_prop) {
+        next = p_data_prop->next_data;
+        if (p_data_prop->ref_count <= 1) {


Same comment as above everywhere ref_count is used.

amarnathhullur · 2025-08-18T23:35:46Z

source/platform/common/bus_common.c

+        p_data_prop->ref_count--;
+    }
+
+    while(p_data_prop) {


I dont understand what is the usecase of if above and then a while loop

Maybe above if can be folded in the while loop.

Pavlo-Uvarov · 2025-08-20T12:45:55Z

source/platform/common/bus_common.c

+        p_data_prop->is_data_set = false;
+        p_data_prop->ref_count = 0;
+    } else {
+        wifi_util_info_print(WIFI_BUS,"%s:%d memory:%p already have some references:%d\r\n",


Suggested change

wifi_util_info_print(WIFI_BUS,"%s:%d memory:%p already have some references:%d\r\n",

wifi_util_info_print(WIFI_BUS,"%s:%d memory:%p still have some references:%d\r\n",

Pavlo-Uvarov · 2025-08-20T12:59:40Z

source/platform/common/bus_common.c

+    while(p_data_prop) {
+        next = p_data_prop->next_data;
+        if (p_data_prop->ref_count <= 1) {
+            free_bus_raw_data(&p_data_prop->value);
+            free(p_data_prop);
+        } else {
+            wifi_util_info_print(WIFI_BUS,"%s:%d memory:%p already have some references:%d\r\n",
+                __func__, __LINE__, p_data_prop, p_data_prop->ref_count);
+            p_data_prop->ref_count--;
+        }
+        p_data_prop = next;
+    }


Suggested change

while(p_data_prop) {

next = p_data_prop->next_data;

if (p_data_prop->ref_count <= 1) {

free_bus_raw_data(&p_data_prop->value);

free(p_data_prop);

} else {

wifi_util_info_print(WIFI_BUS,"%s:%d memory:%p already have some references:%d\r\n",

__func__, __LINE__, p_data_prop, p_data_prop->ref_count);

p_data_prop->ref_count--;

}

p_data_prop = next;

}

next = p_data_prop->next_data;

while(next) {

if (next->ref_count <= 1) {

bus_data_prop_t *temp_next = next;

next = next->next_data;

free_bus_raw_data(&temp_next->value);

free(temp_next);

if (prev) {

prev->next_data = NULL;

}

} else {

wifi_util_info_print(WIFI_BUS,"%s:%d memory:%p still have some references:%d\r\n",

__func__, __LINE__, next, next->ref_count);

next->ref_count--;

if (!first) {

first = next;

}

if (prev) {

prev->next_data = next;

}

prev = next;

next = next->next_data;

++*num_prop;

}

}

return first;

Pavlo-Uvarov · 2025-08-20T13:15:03Z

source/platform/common/bus_common.c

+    } else {
+        wifi_util_info_print(WIFI_BUS,"%s:%d memory:%p already have some references:%d\r\n",
+                __func__, __LINE__, p_data_prop, p_data_prop->ref_count);
+        p_data_prop->ref_count--;


Suggested change

p_data_prop->ref_count--;

p_data_prop->ref_count--;

first = p_data_prop;

prev = p_data_prop;

++*num_prop;

Pavlo-Uvarov · 2025-08-20T13:46:06Z

source/platform/common/bus_common.c

+void bus_release_data_prop(bus_data_prop_t *p_data_prop)
+{


Suggested change

void bus_release_data_prop(bus_data_prop_t *p_data_prop)

{

bus_data_prop_t *bus_release_data_prop(bus_data_prop_t *p_data_prop, int *num_prop)

{

*num_prop = 0;

bus_data_prop_t *first = NULL;

bus_data_prop_t *prev = NULL;

Pavlo-Uvarov · 2025-08-20T13:51:23Z

source/platform/common/bus_common.c

+    bus_release_data_prop(&p_bus_obj->data_prop);
+    p_bus_obj->num_prop = 0;


Suggested change

bus_release_data_prop(&p_bus_obj->data_prop);

p_bus_obj->num_prop = 0;

p_bus_obj->data_prop = *bus_release_data_prop(&p_bus_obj->data_prop, &p_bus_obj->num_prop);

Pavlo-Uvarov · 2025-08-20T16:44:31Z

source/platform/linux/he_bus/src/he_bus_core.c

+    return NULL;
+}
+
+he_bus_error_t he_bus_async_method_invoke(he_bus_handle_t handle, char const *event_name,


Add declaration of this function to header file.

source/platform/openwrt/bus.c

@@ -201,6 +201,12 @@ static bus_error_t bus_remove_table_row(bus_handle_t *handle, char const *name)
    return bus_error_success;
 }

+bus_error_t bus_method_async_invoke(bus_handle_t *handle, void *param_name, char *event_name,


source/platform/rdkb/bus.c

@@ -1796,6 +1796,12 @@ static bus_error_t bus_remove_table_row(bus_handle_t *handle, char const *name)
    return convert_rbus_to_bus_error_code(rc);
 }

+bus_error_t bus_method_async_invoke(bus_handle_t *handle, void *param_name, char *event_name,


source/platform/common/bus_common.h

 typedef bus_error_t (* wifi_bus_reg_table_row_t)                (bus_handle_t *handle, char const *name, uint32_t row_index, char const *alias);
 typedef bus_error_t (* wifi_bus_unreg_table_row_t)              (bus_handle_t *handle, char const *name);
 typedef bus_error_t (* wifi_bus_add_table_row_t)                (bus_handle_t *handle, char const *name, char const *alias, uint32_t *row_index);
 typedef bus_error_t (* wifi_bus_remove_table_row_t)             (bus_handle_t *handle, char const *name);
 typedef bus_error_t (* wifi_bus_unreg_elements_t)              (bus_handle_t *handle, uint32_t num_of_element, bus_data_element_t *data_element);
-typedef bus_error_t (* wifi_bus_method_invoke_t)                (bus_handle_t *handle, void *paramName, char *event, raw_data_t *input_data, raw_data_t *output_data, uint8_t input_bus_data);
+typedef bus_error_t (* wifi_bus_method_async_invoke_t) (bus_handle_t *handle, void *param_name, char *event_name,


source/platform/common/bus_common.h

@@ -218,13 +252,14 @@ typedef bus_error_t (* wifi_bus_event_subscribe_ex_async_t)     (bus_handle_t *h
 typedef bus_error_t (* wifi_bus_event_unsubscribe_t)             (bus_handle_t *handle, char const* event_name);
 typedef bus_error_t (* wifi_bus_event_unsubs_ex_t)              (bus_handle_t *handle, bus_event_sub_t *l_sub_info_map, int num_sub);
 typedef bus_error_t (* wifi_bus_reg_elements_t)                 (bus_handle_t *handle, bus_data_element_t *data_element, uint32_t num_of_element);
-typedef bus_error_t (* wifi_bus_method_invoke_t)                (bus_handle_t *handle, void *paramName, char *event, raw_data_t *input_data, raw_data_t *output_data, uint8_t input_bus_data);
+typedef bus_error_t (* wifi_bus_method_invoke_t)                (bus_handle_t *handle, void *paramName, char *event, bus_data_obj_t *input_data, bus_data_obj_t *output_data, uint8_t input_bus_data);


source/platform/common/bus_common.h

 typedef bus_error_t (*bus_name_sub_handler_t)(char *eventName, bus_event_sub_action_t action, int32_t interval, bool* autoPublish);

 typedef bus_error_t (*bus_event_sub_handler_t)(char *event_name, raw_data_t *p_data, void *userData);
 typedef bus_error_t (*bus_event_sub_ex_async_handler_t)(char *event_name, bus_error_t ret, void *userData);

+typedef void (* wifi_bus_method_async_resp_handler_t) (char const* methodName, bus_error_t error, bus_data_prop_t *params, void *userData);


mateuszCieslak-GL · 2025-08-22T00:33:48Z

source/platform/linux/he_bus/src/he_bus_data_conversion.c

-            l_obj = he_bus_calloc(1, sizeof(he_bus_data_object_t));
-
+            l_obj = memory_alloc_for_he_bus_data_obj();
+            HE_BUS_CHECK_NULL_WITH_RC(l_obj, he_bus_error_out_of_resources);
            l_obj->next_data = NULL;


it is already null, calloc does that.

so no need for this line:
l_obj->next_data = NULL;

source/platform/linux/he_bus/src/he_bus_data_conversion.c

@@ -486,16 +517,29 @@ void free_raw_data_struct(he_bus_raw_data_t *p_data)

 void free_bus_msg_obj_data(he_bus_data_object_t *p_obj_data)
 {
-    free_raw_data_struct(&p_obj_data->data);
+    if (p_obj_data->ref_count <= 1) {


mateuszCieslak-GL · 2025-08-22T00:38:33Z

source/platform/linux/he_bus/src/he_bus_data_conversion.c

+
+uint32_t get_max_objs_cnt(he_bus_data_object_t *p_objs)
+{
+    uint32_t total_obj_size = 0;


total_obj_cnt. variable suggests something other than function name,.

mateuszCieslak-GL · 2025-08-22T00:39:59Z

source/platform/linux/he_bus/src/he_bus_data_conversion.c

+    return total_obj_size;
+}
+
+int set_obj_status(he_bus_data_object_t *p_objs, he_bus_error_t ret_status)


since this will always return OK, void or _Noreturn ( <stdnoreturn.h> from c11) seems more appropriate

Aniket0606 · 2025-08-22T12:23:09Z

source/platform/linux/he_bus/src/he_bus_dml.c

+
+    while(index < 3) {
+        payload_data.data_type = he_bus_data_type_uint32;
+        payload_data.raw_data.u32 = index + 11;


This is test code only. So, I added this number for testing.

mateuszCieslak-GL · 2025-08-22T21:00:54Z

source/platform/common/bus_common.c

+    return temp;
+}
+
+int bus_data_prop_retain(bus_data_prop_t *p_prop)


no, you're not, int bus_set_raw_data_prop(
line 899-900:
p_bus_prop->is_data_set = true;
bus_data_prop_retain(p_bus_prop);

Aniket0606 · 2025-08-26T17:43:14Z

source/platform/common/bus_common.c

+            cur->ref_count--;
+            prev = cur;
+        } else {
+            wifi_util_info_print(WIFI_BUS,"%s:%d memory:%p not have some references:%d\r\n",


mateuszCieslak-GL · 2025-08-25T12:44:53Z

source/platform/common/bus_common.c

+        } else {
+            wifi_util_info_print(WIFI_BUS,"%s:%d memory:%p not have some references:%d\r\n",
+                __func__, __LINE__, cur, cur->ref_count);
+            prev = cur;


since you've checked that 'cur' basically doesnt exist, shouldn't this be prev = next?. if it has ref_count = 0, this is error scenario and other members (such as next_data) might not exactly be whats you expect them to be..

mateuszCieslak-GL · 2025-08-25T12:50:32Z

source/platform/linux/he_bus/src/he_bus_data_conversion.c

+            __func__, __LINE__, p_obj_data, p_obj_data->ref_count);
+        p_obj_data->ref_count--;
+    } else {
+        he_bus_core_info_print("%s:%d memory:%p not have some references:%d\r\n",


as in bus_Common.c this is an error scenario, update log level please.

mateuszCieslak-GL · 2025-08-25T12:51:43Z

source/platform/linux/he_bus/src/he_bus_data_conversion.c

+            cur->ref_count--;
+            prev = cur;
+        } else {
+            he_bus_core_info_print("%s:%d memory:%p not have some references:%d\r\n",


same comment as in bus_Common.c . Error level + think about rationale of prev = cur.

mateuszCieslak-GL · 2025-08-25T12:56:16Z

source/platform/common/bus_common.c

+    BUS_CHECK_NULL_WITH_RC(p_bus_prop, RETURN_ERR);
+    BUS_CHECK_NULL_WITH_RC(p_raw_data, RETURN_ERR);
+
+    if (event_name == NULL) {


since most of the events will have name (correct me if im wrong), this would benefit from order change. If's are compiled with assumption to be taken, so
if (event_name != NULL) {
....
} else {
*p_raw_data = p_bus_prop->value;
}
will be more optimal here,

If a name isn't provided, we should return the first element. That's why I did this.

amarnathhullur · 2025-08-26T17:19:15Z

source/platform/common/bus_common.c

+            cur->ref_count--;
+            prev = cur;
+        } else {
+            wifi_util_info_print(WIFI_BUS,"%s:%d memory:%p not have some references:%d\r\n",


instead of wifi_util_info_print, put this as a wifi_util_error_print

Reason for change: Implemented the HE bus asynchronous method invoke API to asynchronously set and get data between a consumer and a producer. Test Procedure: 1) Load Linux(RPI) or OpenWRT(BPI) HE bus Build. 2) Trigger "bus_method_async_invoke" API from code to test those changes. 3) Verify device memory. Risks: Low Priority: P1 Signed-off-by: [email protected]

Aniket0606 requested a review from a team as a code owner August 18, 2025 22:50

amarnathhullur requested changes Aug 20, 2025

View reviewed changes

Pavlo-Uvarov reviewed Aug 20, 2025

View reviewed changes

mateuszCieslak-GL suggested changes Aug 22, 2025

View reviewed changes

Aniket0606 force-pushed the he_bus_async_method_invoke_api branch from 0043130 to bfd2333 Compare August 22, 2025 12:52

mateuszCieslak-GL suggested changes Aug 25, 2025

View reviewed changes

amarnathhullur requested changes Aug 26, 2025

View reviewed changes

Aniket0606 force-pushed the he_bus_async_method_invoke_api branch from bfd2333 to de9c544 Compare August 26, 2025 17:50

amarnathhullur approved these changes Aug 27, 2025

View reviewed changes

Merge branch 'develop' into he_bus_async_method_invoke_api

9316c0f

	wifi_util_info_print(WIFI_BUS,"%s:%d memory:%p already have some references:%d\r\n",
	wifi_util_info_print(WIFI_BUS,"%s:%d memory:%p still have some references:%d\r\n",

-    while(p_data_prop) {
-        next = p_data_prop->next_data;
-        if (p_data_prop->ref_count <= 1) {
-            free_bus_raw_data(&p_data_prop->value);
-            free(p_data_prop);
-        } else {
-            wifi_util_info_print(WIFI_BUS,"%s:%d memory:%p already have some references:%d\r\n",
-                __func__, __LINE__, p_data_prop, p_data_prop->ref_count);
-            p_data_prop->ref_count--;
-        }
-        p_data_prop = next;
-    }
+    next = p_data_prop->next_data;
+    while(next) {
+        if (next->ref_count <= 1) {
+            bus_data_prop_t *temp_next = next;
+            next = next->next_data;
+            free_bus_raw_data(&temp_next->value);
+            free(temp_next);
+            if (prev) {
+                prev->next_data = NULL;
+            }
+        } else {
+            wifi_util_info_print(WIFI_BUS,"%s:%d memory:%p still have some references:%d\r\n",
+                __func__, __LINE__, next, next->ref_count);
+            next->ref_count--;
+            if (!first) {
+                first = next;
+            }
+            if (prev) {
+                prev->next_data = next;
+            }
+            prev = next;
+            next = next->next_data;
+            ++*num_prop;
+        }
+    }
+    return first;

-void bus_release_data_prop(bus_data_prop_t *p_data_prop)
-{
+bus_data_prop_t *bus_release_data_prop(bus_data_prop_t *p_data_prop, int *num_prop)
+{
+    *num_prop = 0;
+    bus_data_prop_t *first = NULL;
+    bus_data_prop_t *prev = NULL;

		bus_release_data_prop(&p_bus_obj->data_prop);
		p_bus_obj->num_prop = 0;

	bus_release_data_prop(&p_bus_obj->data_prop);
	p_bus_obj->num_prop = 0;
	p_bus_obj->data_prop = *bus_release_data_prop(&p_bus_obj->data_prop, &p_bus_obj->num_prop);

RDKB-61190:[OneWifi] Implemented HE bus async method invoke API #541

Are you sure you want to change the base?

RDKB-61190:[OneWifi] Implemented HE bus async method invoke API #541

Uh oh!

Conversation

Aniket0606 commented Aug 18, 2025

Uh oh!

CLAassistant commented Aug 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

amarnathhullur left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Pavlo-Uvarov Aug 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Pavlo-Uvarov Aug 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Pavlo-Uvarov Aug 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

CLAassistant commented Aug 18, 2025 •

edited

Loading

Pavlo-Uvarov Aug 20, 2025 •

edited

Loading

Pavlo-Uvarov Aug 20, 2025 •

edited

Loading

Pavlo-Uvarov Aug 20, 2025 •

edited

Loading

Aniket0606 Aug 26, 2025 •

edited

Loading