Re: [Flent-users] [tohojo/flent] metadata: Add parsing of WiFi status information (#192)

["Toke Høiland-Jørgensen" <notifications@github.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3202 bytes --]

tohojo requested changes on this pull request.

Looking good! Only a few minor details to fix.

Also, please add some text to the commit message explaining the change; only needs to be a line or two for this (put that before the signed-off-by).

> +
+        for line in output.splitlines():
+
+            if len(line.strip()) >= 1:
+                parts = line.split() 
+                k,v = parts[0], parts[1]
+
+                if k  in unwanted_keys:
+                  continue
+
+                if k == 'txpower':
+                   v = float(parts[1])
+
+                if k == 'channel':
+                    # This condition will return a dict with all the values of the channel
+		            # Output will be {'addr':..., channel': {'band': 2462, 'center1': 2462, 'number': 11, 'width': 20}, 'ssid':...}

The indentation is off on the second comment line. Also, please add an example of the input (i.e., the line you're trying to parse) along with the output.

> +                if k  in unwanted_keys:
+                  continue
+
+                if k == 'txpower':
+                   v = float(parts[1])
+
+                if k == 'channel':
+                    # This condition will return a dict with all the values of the channel
+		            # Output will be {'addr':..., channel': {'band': 2462, 'center1': 2462, 'number': 11, 'width': 20}, 'ssid':...}
+                    v = {}
+                    v['number'] =int(parts[1])
+                    v['band'] = int(parts[2].strip("("));
+                    v['width'] = int(parts[5])
+                    v['center1'] = int(parts[8])
+
+                if k == "multicast TXQ":

Please add a short comment explaining this (could just be, e.g., "No interesting output after this")

> +                if k == 'txpower':
+                   v = float(parts[1])
+
+                if k == 'channel':
+                    # This condition will return a dict with all the values of the channel
+		            # Output will be {'addr':..., channel': {'band': 2462, 'center1': 2462, 'number': 11, 'width': 20}, 'ssid':...}
+                    v = {}
+                    v['number'] =int(parts[1])
+                    v['band'] = int(parts[2].strip("("));
+                    v['width'] = int(parts[5])
+                    v['center1'] = int(parts[8])
+
+                if k == "multicast TXQ":
+                    break
+
+            wifi_data[k] = v

This should be inside the if statement.

Alternatively, you could save one level of indentation by reversing the if statement above and turning it into a "continue". I.e., replace

```
if len(line.strip()) >= 1:
   # lots of code here
```
with
```
if not line.strip():
  continue
# code here (unindented)
```

Up to you which you prefer :)

> @@ -138,6 +138,7 @@ def record_metadata(results, extended, hostnames):
     if extended:
         m['IP_ADDRS'] = get_ip_addrs()
         m['GATEWAYS'] = get_gateways()
+        m['WIFI_DATA'] =get_wifi_data()

Missing space after =

-- 
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/192#pullrequestreview-322868381

[-- Attachment #1.2: Type: text/html, Size: 5631 bytes --]

[-- Attachment #2: Type: text/plain, Size: 151 bytes --]

_______________________________________________
Flent-users mailing list
Flent-users@flent.org
http://flent.org/mailman/listinfo/flent-users_flent.org