[wifi][gonk-kk] Porting WifiUtils

RESOLVED FIXED

Status

Firefox OS
Wifi
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: kli, Assigned: kli)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

4 years ago
WifiUtils is compiled failed in gonk-kk.
(Assignee)

Updated

4 years ago
Assignee: nobody → kli
Blocks: 943278
(Assignee)

Comment 1

4 years ago
Created attachment 8345156 [details] [diff] [review]
Update NetUtils and WifiUtils to support Kitkat.

Hi, Vincent, some APIs are changed in kitkat. Could you help me to review this patch? Thank!
Attachment #8345156 - Flags: review?(vchang)
(Assignee)

Updated

4 years ago
Status: NEW → ASSIGNED
Comment on attachment 8345156 [details] [diff] [review]
Update NetUtils and WifiUtils to support Kitkat.

Review of attachment 8345156 [details] [diff] [review]:
-----------------------------------------------------------------

Hi kli, thank you for jumping to this. I am fine with the patch. 
But we need Fabrice to review this, too.

::: dom/wifi/WifiUtils.h
@@ +109,3 @@
>  
>    virtual int32_t
> +  do_wifi_connect_to_supplicant(const char* iface) = 0; // KK == ICS != JB 

Nit: Remove trailing whitespace.

@@ +112,3 @@
>  
>    virtual void
> +  do_wifi_close_supplicant_connection(const char* iface) = 0; // KK == ICS != JB 

Nit: Remove trailing whitespace.
Attachment #8345156 - Flags: review?(vchang)
Attachment #8345156 - Flags: review?(fabrice)
Attachment #8345156 - Flags: review+
Comment on attachment 8345156 [details] [diff] [review]
Update NetUtils and WifiUtils to support Kitkat.

Review of attachment 8345156 [details] [diff] [review]:
-----------------------------------------------------------------

I'd really like to see systemlibs.js being cleaned up!

::: dom/system/gonk/systemlibs.js
@@ +237,5 @@
> +                        ctypes.int.ptr,   // lease
> +                        ctypes.char.ptr,  // vendorinfo
> +                        ctypes.char.ptr,  // domain
> +                        ctypes.char.ptr); // mtu
> +    } else if (sdkVersion >= 18) { // 18 == JB 4.3

I think we have no JS callers for dhcp_do_request and dhcp_do_request_renew anymore. We should just remove this code.

::: dom/wifi/NetUtils.cpp
@@ +132,5 @@
>      char domains[PROPERTY_VALUE_MAX];
>      ret = dhcp_do_request(ifname, ipaddr, gateway, prefixLength, dns,
>                            server, lease, vendorinfo, domains);
> +  } else if (sdkVersion == 19) {
> +    // JB 4.4

can you add a link to androidxref like for the other versions?
Attachment #8345156 - Flags: review?(fabrice) → feedback+
(Assignee)

Comment 4

4 years ago
I think loading few javascript source in b2g could save memory usage. I will revert systemlibs.js and file a new bug to remove the unused API in systemlibs.js. And the new bug will block 1.3+ to benefit low memory device such Tarako.
(In reply to Kai-Zhen Li from comment #4)
> I think loading few javascript source in b2g could save memory usage. I will
> revert systemlibs.js and file a new bug to remove the unused API in
> systemlibs.js. And the new bug will block 1.3+ to benefit low memory device
> such Tarako.

No need to block 1.3 on that. We'll cherry pick for tarako though.

Comment 6

4 years ago
This needs to be landed. Kai-Zhen?
Flags: needinfo?(kli)
(Assignee)

Comment 7

4 years ago
(In reply to Andreas Gal :gal from comment #6)
> This needs to be landed. Kai-Zhen?

Yes, this needs to be landed. But I notice that some driver command is not available in kk, such as MACADDR. So I want to got it fixed before landed.
Flags: needinfo?(kli)
(Assignee)

Comment 8

4 years ago
Created attachment 8359224 [details] [diff] [review]
Update NetUtils and WifiUtils for gonk-kk

Hi Fabrice,Can we land this first? Because it block the gonk-kk build. Let's file a new bug to follow the inconsistent APIs/Commands of kk.
Attachment #8345156 - Attachment is obsolete: true
Attachment #8359224 - Flags: review?(fabrice)
Comment on attachment 8359224 [details] [diff] [review]
Update NetUtils and WifiUtils for gonk-kk

Review of attachment 8359224 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/system/gonk/systemlibs.js
@@ +186,5 @@
>      RESET_IPV6_ADDRESSES: 0x02,
>    };
>  
>    iface.RESET_ALL_ADDRESSES = iface.RESET_IPV4_ADDRESSES |
>                                iface.RESET_IPV6_ADDRESSES

let's add the missing semicolon at the end of the line!

::: dom/wifi/WifiUtils.h
@@ +109,3 @@
>  
>    virtual int32_t
> +  do_wifi_connect_to_supplicant(const char* iface) = 0; // KK == ICS != JB 

nit: trailing whitespace

@@ +112,3 @@
>  
>    virtual void
> +  do_wifi_close_supplicant_connection(const char* iface) = 0; // KK == ICS != JB 

nit: trailing whitespace
Attachment #8359224 - Flags: review?(fabrice) → review+
(Assignee)

Comment 10

4 years ago
Created attachment 8359578 [details] [diff] [review]
Update NetUtils and WifiUtils for gonk-kk

nit got fixed.
Attachment #8359224 - Attachment is obsolete: true
(Assignee)

Updated

4 years ago
Keywords: checkin-needed

Comment 11

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/a4d44de1d3a0
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/a4d44de1d3a0
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.