From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on mail.toke.dk X-Spam-Level: X-Spam-ASN: X-Spam-Status: No, score=-101.9 required=5.0 tests=BAYES_00,SHORTCIRCUIT shortcircuit=ham autolearn=disabled version=3.4.2 Received: from mail.toke.dk by mail.toke.dk with LMTP id YLPJIqr+3F1U6QwAOr1fkg (envelope-from ) for ; Tue, 26 Nov 2019 11:30:02 +0100 Authentication-Results: mail.toke.dk; none (SPF check N/A for local connections - client-ip=77.235.54.103; helo=web6.sd.eurovps.com; envelope-from=flent-users-bounces@flent.org; receiver=) Authentication-Results: mail.toke.dk; dkim=fail reason="signature verification failed" (1024-bit key) header.d=github.com header.i=@github.com header.b=XM4vdnig Received: from web6.sd.eurovps.com (web6.sd.eurovps.com [77.235.54.103]) by mail.toke.dk (Postfix) with ESMTPS id 69FD46B69A8 for ; Tue, 26 Nov 2019 11:29:58 +0100 (CET) Received: from [::1] (port=34400 helo=web6.sd.eurovps.com) by web6.sd.eurovps.com with esmtp (Exim 4.92) (envelope-from ) id 1iZY6W-000OrS-78; Tue, 26 Nov 2019 12:29:56 +0200 Received: from out-24.smtp.github.com ([192.30.252.207]:43255) by web6.sd.eurovps.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.92) (envelope-from ) id 1iZY6M-000Olz-5x for flent-users@flent.org; Tue, 26 Nov 2019 12:29:54 +0200 Received: from github-lowworker-2ef7ba1.ac4-iad.github.net (github-lowworker-2ef7ba1.ac4-iad.github.net [10.52.16.66]) by smtp.github.com (Postfix) with ESMTP id 270216A1CE0 for ; Tue, 26 Nov 2019 02:29:06 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=github.com; s=pf2014; t=1574764146; bh=AULX34KthXjZFhrnUsxSAWBli3M2Z8j66l7FQ1C23jw=; h=Date:From:Reply-To:To:Cc:In-Reply-To:References:Subject:List-ID: List-Archive:List-Post:List-Unsubscribe:From; b=XM4vdnigPWqkBfPONfzBIp4JnUfMicm7ZX4SOGKONAZO+q6B3IcVpa8unWIjYPvX2 3Wf0B1I3xO8w4K6fa+YWCXgZ1l4sPHx06KIQVIIR6Dkbsy9HEZ+ki9iz7unfK3SdfN n7KeP6oj/bLsqdIUA/EkI+58GrjqhI0l9YWtyYb0= Date: Tue, 26 Nov 2019 02:29:06 -0800 From: =?UTF-8?B?VG9rZSBIw7hpbGFuZC1Kw7hyZ2Vuc2Vu?= To: tohojo/flent Message-ID: In-Reply-To: References: Mime-Version: 1.0 Precedence: list X-GitHub-Sender: tohojo X-GitHub-Recipient: flent-users X-GitHub-Reason: subscribed X-Auto-Response-Suppress: All X-GitHub-Recipient-Address: flent-users@flent.org Subject: Re: [Flent-users] [tohojo/flent] metadata: Add parsing of WiFi status information (#192) X-BeenThere: flent-users@flent.org X-Mailman-Version: 2.1.29 List-Id: Flent discussion list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: tohojo/flent Cc: Subscribed Content-Type: multipart/mixed; boundary="===============1104569950071391449==" Errors-To: flent-users-bounces@flent.org Sender: "Flent-users" X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - web6.sd.eurovps.com X-AntiAbuse: Original Domain - toke.dk X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - flent.org X-Get-Message-Sender-Via: web6.sd.eurovps.com: acl_c_authenticated_local_user: mailman/mailman X-Authenticated-Sender: web6.sd.eurovps.com: mailman@flent.org --===============1104569950071391449== Content-Type: multipart/alternative; boundary="--==_mimepart_5ddcfe72144b5_4b823fd5eeccd9642449d0"; charset=UTF-8 Content-Transfer-Encoding: 7bit ----==_mimepart_5ddcfe72144b5_4b823fd5eeccd9642449d0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit 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 ----==_mimepart_5ddcfe72144b5_4b823fd5eeccd9642449d0 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 7bit

@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).

In flent/metadata.py:

> +
+        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.

In flent/metadata.py:

> +                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")

In flent/metadata.py:

> +                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 :)

In flent/metadata.py:

> @@ -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, view it on GitHub, or unsubscribe.

----==_mimepart_5ddcfe72144b5_4b823fd5eeccd9642449d0-- --===============1104569950071391449== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Flent-users mailing list Flent-users@flent.org http://flent.org/mailman/listinfo/flent-users_flent.org --===============1104569950071391449==--