tohojo requested changes on this pull request.

This is a good start! There are some details to fix below. Also, you'll need to 
add a proper commit message. The first line should be a oneline description of 
the change, prefixed by the system you are touching (so something like 
"metadata: Add parsing of WiFi status information". Then add a description of 
what your change does (a shortish paragraph is probably enough for this 
change), with an example of the output you are parsing embedded in the commit 
message. You'll also need to add a Signed-off-by tag, as the bot says.

When revising, you can rewrite the commit in your local git tree, and 
force-push it to the same branch; that will update this pull request directly, 
while still keeping the history clean.

Feel free to ask clarifying questions, of course :)

> @@ -514,3 +514,27 @@ def get_module_versions():
             module_versions[m] = v
 
     return module_versions
+
+def get_wifi_data(iface):

You'll also need to add appropriate calls to this function in 
record_metadata(); I'd suggest putting them along with the calls to 
get_ip_addrs() and get_gateways()

> @@ -514,3 +514,27 @@ def get_module_versions():
             module_versions[m] = v
 
     return module_versions
+
+def get_wifi_data(iface):
+    wifi_data = {}
+    unwanted_keys = ["Interface", "ifindex", "wdev", "wiphy"]
+    output = get_command_output("iw dev %s info" % iface)
+    if output is not None:
+        for line in output.splitlines():
+            parts = line.split() 

This needs to discard empty lines first

> @@ -514,3 +514,27 @@ def get_module_versions():
             module_versions[m] = v
 
     return module_versions
+
+def get_wifi_data(iface):
+    wifi_data = {}
+    unwanted_keys = ["Interface", "ifindex", "wdev", "wiphy"]
+    output = get_command_output("iw dev %s info" % iface)
+    if output is not None:
+        for line in output.splitlines():
+            parts = line.split() 
+
+            if parts[0]  in unwanted_keys:
+                continue
+            k,v = parts[0], parts[1]

Since you're doing the split here, better to use the 'k' and 'v' variables 
further down instead of parts[0] and parts[1]

> @@ -514,3 +514,27 @@ def get_module_versions():
             module_versions[m] = v
 
     return module_versions
+
+def get_wifi_data(iface):
+    wifi_data = {}
+    unwanted_keys = ["Interface", "ifindex", "wdev", "wiphy"]
+    output = get_command_output("iw dev %s info" % iface)
+    if output is not None:
+        for line in output.splitlines():
+            parts = line.split() 
+
+            if parts[0]  in unwanted_keys:
+                continue
+            k,v = parts[0], parts[1]
+            if parts[0] == 'txpower':
+               v = float(parts[1])
+            if parts[0] == 'channel':

Please add a comment with an example of the line you are trying to parse with 
this

> +
+            if parts[0]  in unwanted_keys:
+                continue
+            k,v = parts[0], parts[1]
+            if parts[0] == 'txpower':
+               v = float(parts[1])
+            if parts[0] == 'channel':
+                v = {}
+                v['number'] =int(parts[1])
+                v['band'] = int(parts[2].strip("("));
+                v['width'] = int(parts[5])
+                v['center1'] = int(parts[8])
+                
+            wifi_data[k] = v
+
+    return wifi_data

Missing newline at the end of the file

> @@ -514,3 +514,27 @@ def get_module_versions():
             module_versions[m] = v
 
     return module_versions
+
+def get_wifi_data(iface):
+    wifi_data = {}
+    unwanted_keys = ["Interface", "ifindex", "wdev", "wiphy"]
+    output = get_command_output("iw dev %s info" % iface)
+    if output is not None:
+        for line in output.splitlines():
+            parts = line.split() 

Also, you'll need to handle unwanted lines. On my system, the output looks like 
this:

```
Interface wlan0
        ifindex 3
        wdev 0x1
        addr 98:de:ad:ca:fe:85
        ssid myssid
        type managed
        wiphy 0
        channel 36 (5180 MHz), width: 80 MHz, center1: 5210 MHz
        txpower 22.00 dBm
        multicast TXQ:
                qsz-byt qsz-pkt flows   drops   marks   overlmt hashcol 
tx-bytes        tx-packets
                0       0       0       0       0       0       0       0       
        0
```
Your parser will choke on the bit at the bottom; I think it's probably fine if 
you just abort parsing when you get to the "multicast TXQ" line...

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/tohojo/flent/pull/188#pullrequestreview-312762596
_______________________________________________
Flent-users mailing list
Flent-users@flent.org
http://flent.org/mailman/listinfo/flent-users_flent.org

Reply via email to