RDKB-60777: OneWifi sends ACK immediately instead of sending it after enabling it. #492

Srijeyarankesh · 2025-07-29T09:48:26Z

Make the WebConfig Amenity thread wait and signal it only after appropriately enabling LnF VAPs.

CLAassistant · 2025-07-29T09:48:33Z

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.
0 out of 2 committers have signed the CLA.

❌ sjs889
❌ Srijeyarankesh

sjs889 seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

source/core/wifi_ctrl.c

bogdanbogush2 · 2025-08-04T07:13:59Z

source/core/wifi_ctrl.c

+    wifi_ctrl_t *ctrl = get_wifictrl_obj();
+
+    if (subdoc_type != webconfig_subdoc_type_lnf) {
+        wifi_util_info_print(WIFI_CTRL,"%s:%d - subdoc_type:%d not supported\n", __func__, __LINE__, subdoc_type);


code style: more than 100 chars in line, missing space after WIFI_CTRL,

wifi_util_info_print(WIFI_CTRL, "%s:%d - subdoc_type:%d not supported\n", __func__, __LINE__, subdoc_type);

I have made the change

bogdanbogush2 · 2025-08-04T07:15:04Z

source/core/wifi_ctrl.c

+    wifi_sem_param_signal(&ctrl->managed_wifi_sem_param, status);
+}
+
+bool managed_wifi_cfg_sem_wait_duration(uint32_t time_in_sec, webconfig_subdoc_type_t subdoc_type)


add function prototype in wifi_ctrl.h if exported

I have exported it to wifi_ctr.h file where I have added a prototype.

bogdanbogush2 · 2025-08-04T07:21:26Z

source/core/wifi_ctrl_webconfig.c

                    webconfig_analytic_event_data_to_hal_apply(data);
                    ret = webconfig_hal_lnf_vap_apply(ctrl, &data->u.decoded);
+                    bool status = ((ret == RETURN_OK) ? true : false);
+                    wifi_util_info_print(WIFI_CTRL,":%s:%d lnf blob cfg status:%d\n", __func__, __LINE__, ret);


code style: more than 100 chars in line, missing space after WIFI_CTRL,
redundant ":" at the beggining of log message, ":%s:%d lnf blob cfg status:%d\n" -> "%s:%d lnf blob cfg status:%d\n"

Have made the change

source/core/wifi_multidoc_webconfig.c

Srijeyarankesh

I have made the changes. Can you please have a look at them again?

amarnathhullur · 2025-08-05T22:12:33Z

source/core/wifi_ctrl.c

-    wifi_ctrl_t *ctrl = NULL;
-
-    ctrl = (wifi_ctrl_t *)get_wifictrl_obj();
+    if (!sem_param->is_init) {


check for sem_param not NULL before dereferencing it

amarnathhullur · 2025-08-05T22:17:44Z

source/core/wifi_ctrl.c

-        pthread_mutex_unlock(&ctrl->hotspot_sem_param.lock);
+static void wifi_sem_param_destroy(wifi_sem_param_t *sem_param)
+{
+    if (sem_param->is_init) {


Same comment as above to check the set_param being Not NULL

amarnathhullur · 2025-08-05T22:22:55Z

source/core/wifi_ctrl.c

+{
+    wifi_ctrl_t *ctrl = get_wifictrl_obj();
+
+    if (subdoc_type != webconfig_subdoc_type_lnf) {


What is the reason for checking of subdoc_type in sem_wait_duration?

bogdanbogush2 · 2025-08-06T11:51:52Z

source/core/wifi_multidoc_webconfig.c

                              __func__, (subdoc_type == webconfig_subdoc_type_lnf) ? "lnf_psk" : "xfinity");
        goto done;
    }
+    else if(ret == RETURN_TIMEDOUT) {


please fix code style
} else if (ret == RETURN_TIMEDOUT) {

Srijeyarankesh · 2025-08-18T12:12:06Z

@amarnathhullur @bogdanbogush2 @narendradandu Can you please review now? Please ignore some debug prints as of now. Can you please check if code syntax and semantics are ok?

bogdanbogush2 · 2025-08-18T12:19:34Z

source/core/wifi_multidoc_webconfig.c

-    if (ret_value == false) {
-        wifi_util_error_print(WIFI_CTRL, "%s:%d WebConfig blob apply is failed\n", __func__,
-            __LINE__);
+    wifi_util_dbg_print(WIFI_CTRL, "%s:%d:SREESH Encoded blob:\n%s\n", __func__, __LINE__, str);


please remove SREESH

bogdanbogush2 · 2025-08-18T12:20:11Z

source/core/wifi_multidoc_webconfig.c

+
+    if (subdoc_type == webconfig_subdoc_type_lnf) {
+        wifi_util_info_print(WIFI_CTRL,
+            "%s:%d:SREESH Doing a thread wait for the managed wifi blob for %d seconds\n", __func__,


please remove SREESH

source/core/wifi_multidoc_webconfig.c

bogdanbogush2 · 2025-08-18T12:34:36Z

source/core/wifi_multidoc_webconfig.c

+            webconfig_data_free(data);
+            return RETURN_TIMEDOUT;
+        }
+    } else if(subdoc_type == webconfig_subdoc_type_xfinity) {


please fix code style

} else if (subdoc_type == webconfig_subdoc_type_xfinity) {

amarnathhullur · 2025-08-19T00:37:32Z

@Srijeyarankesh, no comments from my side apart from the comments provided by @bogdanbogush2 .

gsathish86 · 2025-08-19T01:44:14Z

Am good with the latest changes.

bogdanbogush2 · 2025-08-19T06:45:04Z

source/core/wifi_multidoc_webconfig.c

        wifi_util_error_print(WIFI_CTRL, "%s: failed to encode %s subdoc\n", \
                              __func__, (subdoc_type == webconfig_subdoc_type_lnf) ? "lnf_psk" : "xfinity");
        goto done;
+    } else if(ret == RETURN_TIMEDOUT) {


code style

} else if (ret == RETURN_TIMEDOUT) {

Pavlo-Uvarov · 2025-08-20T15:52:47Z

source/core/wifi_ctrl.h

+bool managed_wifi_sem_wait_duration(uint32_t time_in_sec);
+void managed_wifi_sem_signal(bool status);


Where are definitions for these?

lines 2032 + 2039 in wifi_ctrl.c

You should name them the same way: managed_wifi_cfg_sem_wait_duration and managed_wifi_cfg_sem_signal

Pavlo-Uvarov · 2025-08-20T15:57:55Z

source/core/wifi_ctrl.c

+    wifi_sem_param_signal(&ctrl->managed_wifi_sem_param, status);
+}
+
+bool managed_wifi_cfg_sem_wait_duration(uint32_t time_in_sec)


Add declaration for this and other function used in other translation nodes to wifi_ctrl.h

mateuszCieslak-GL · 2025-08-22T18:17:38Z

source/core/wifi_ctrl.c

    bool status = false;

-    wifi_ctrl_t *ctrl = NULL;
+    wifi_sem_param_init(sem_param);


Suggested change

wifi_sem_param_init(sem_param);

if (!sem_param->is_init) wifi_sem_param_init(sem_param);

(together with removal of the if condition from wifi_sem_param_init. )
That way, one unnecessary function call will be spared making the code a bit faster.

mateuszCieslak-GL · 2025-08-22T18:23:42Z

source/core/wifi_ctrl.c

-    pthread_mutex_lock(&ctrl->hotspot_sem_param.lock);
-    ret = pthread_cond_timedwait(&ctrl->hotspot_sem_param.cond, &ctrl->hotspot_sem_param.lock, &ts);
+    pthread_mutex_lock(&sem_param->lock);
+    ret = pthread_cond_timedwait(&sem_param->cond, &sem_param->lock, &ts);


this can fail. Will extra info why it help will not be helpful in this case? or is it too rare?

mateuszCieslak-GL · 2025-08-22T18:33:56Z

source/core/wifi_ctrl.c

-    wifi_ctrl_t *ctrl = NULL;
-
-    ctrl = (wifi_ctrl_t *)get_wifictrl_obj();
+    if (sem_param && !sem_param->is_init) {


Suggested change

if (sem_param && !sem_param->is_init) {

move checking if (!sem_param -> is_init) to call place, in 2005 line.
checking whether sem_param exists doesnt make sense, since this is struct being part of object - it will always exist.

mateuszCieslak-GL · 2025-08-22T18:36:01Z

source/core/wifi_ctrl.c

-        pthread_mutex_unlock(&ctrl->hotspot_sem_param.lock);
+static void wifi_sem_param_destroy(wifi_sem_param_t *sem_param)
+{
+    if (sem_param && sem_param->is_init) {


as above with sem_param_init please.

mateuszCieslak-GL · 2025-08-22T18:36:48Z

source/core/wifi_ctrl.c

-    ctrl->hotspot_sem_param.is_init = false;
-    pthread_mutex_destroy(&ctrl->hotspot_sem_param.lock);
-    pthread_cond_destroy(&ctrl->hotspot_sem_param.cond);
+    wifi_sem_param_destroy(sem_param);


As in line 2005:

Suggested change

wifi_sem_param_destroy(sem_param);

if (sem_param->is_init) wifi_sem_param_destroy(sem_param);

mateuszCieslak-GL · 2025-08-22T18:37:40Z

source/core/wifi_ctrl.c


+static void wifi_sem_param_signal(wifi_sem_param_t *sem_param, bool status)
+{
+    if (sem_param && sem_param->is_init) {


as with _init and _destroy methods, at least remove first part of condition

mateuszCieslak-GL · 2025-08-22T18:49:05Z

source/core/wifi_ctrl.h

+bool managed_wifi_sem_wait_duration(uint32_t time_in_sec);
+void managed_wifi_sem_signal(bool status);


lines 2032 + 2039 in wifi_ctrl.c

mateuszCieslak-GL · 2025-08-22T18:58:11Z

source/core/wifi_multidoc_webconfig.c

+        } else {
+            wifi_util_error_print(WIFI_CTRL, "%s:%d Managed wifi blob failed, exiting\n", __func__,
+                __LINE__);
+            webconfig_data_free(data);


as there are no special circumstances, please use one exit point from function.
setting int ret_value = return_XX at the end of the block improves readibility significantly.

sjs889 added 4 commits July 28, 2025 13:01

Adding initial changes

bcb3b93

correct the errors

da35c08

Correcting build error

884c09c

Amned changes

cb62a9b

Srijeyarankesh requested a review from a team as a code owner July 29, 2025 09:48

Srijeyarankesh and others added 5 commits July 29, 2025 15:50

Merge branch 'develop' into managed_wifi_thread

9a2f411

changes

1bd49a2

Refactor the lock code

c0d8be7

Correct the error

d4b7af3

kk

1bd0bb8

bogdanbogush2 suggested changes Aug 4, 2025

View reviewed changes

Srijeyarankesh commented Aug 4, 2025

View reviewed changes

Commiting required Review coment changes

a4e9d12

amarnathhullur reviewed Aug 5, 2025

View reviewed changes

bogdanbogush2 reviewed Aug 6, 2025

View reviewed changes

sjs889 added 5 commits August 18, 2025 11:26

Resolved conflicts

833d249

Managed WiFi thread wait

0ea9c57

ll

d11d7c8

oo

8a574fb

Final changes

df44add

bogdanbogush2 suggested changes Aug 18, 2025

View reviewed changes

sjs889 and others added 2 commits August 19, 2025 06:24

Addressed comments

b8f9071

Merge branch 'develop' into managed_wifi_thread

5fac4c9

bogdanbogush2 approved these changes Aug 19, 2025

View reviewed changes

Pavlo-Uvarov reviewed Aug 20, 2025

View reviewed changes

mateuszCieslak-GL suggested changes Aug 22, 2025

View reviewed changes

		bool managed_wifi_sem_wait_duration(uint32_t time_in_sec);
		void managed_wifi_sem_signal(bool status);

	wifi_sem_param_init(sem_param);
	if (!sem_param->is_init) wifi_sem_param_init(sem_param);

	wifi_sem_param_destroy(sem_param);
	if (sem_param->is_init) wifi_sem_param_destroy(sem_param);

RDKB-60777: OneWifi sends ACK immediately instead of sending it after enabling it. #492

Are you sure you want to change the base?

RDKB-60777: OneWifi sends ACK immediately instead of sending it after enabling it. #492

Uh oh!

Conversation

Srijeyarankesh commented Jul 29, 2025

Uh oh!

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

Uh oh!

Uh oh!

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Srijeyarankesh 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!

Srijeyarankesh commented Aug 18, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bogdanbogush2 Aug 18, 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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amarnathhullur commented Aug 19, 2025

Uh oh!

gsathish86 commented Aug 19, 2025

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!

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

CLAassistant commented Jul 29, 2025 •

edited

Loading

bogdanbogush2 Aug 18, 2025 •

edited

Loading