Flent-users discussion archives
 help / color / mirror / Atom feed
* Re: [Flent-users] [tohojo/flent] metadata: Add parsing of WiFi status information (#192)
       [not found] <tohojo/flent/pull/192@github.com>
@ 2019-11-26 10:29 ` Toke Høiland-Jørgensen
  2019-12-02 12:19 ` Toke Høiland-Jørgensen
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 5+ messages in thread
From: Toke Høiland-Jørgensen @ 2019-11-26 10:29 UTC (permalink / raw)
  To: tohojo/flent; +Cc: Subscribed


[-- 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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Flent-users] [tohojo/flent] metadata: Add parsing of WiFi status information (#192)
       [not found] <tohojo/flent/pull/192@github.com>
  2019-11-26 10:29 ` [Flent-users] [tohojo/flent] metadata: Add parsing of WiFi status information (#192) Toke Høiland-Jørgensen
@ 2019-12-02 12:19 ` Toke Høiland-Jørgensen
  2019-12-02 12:29 ` Toke Høiland-Jørgensen
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 5+ messages in thread
From: Toke Høiland-Jørgensen @ 2019-12-02 12:19 UTC (permalink / raw)
  To: tohojo/flent; +Cc: Subscribed


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

tohojo approved this pull request.

The changes look good now, great work! :)

It seems there are two commits in the pull request; I guess you created a new one instead of amending the original one? I can fix this and squash them together when merging, but just something to be aware of next time :)



-- 
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-325199679

[-- Attachment #1.2: Type: text/html, Size: 1923 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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Flent-users] [tohojo/flent] metadata: Add parsing of WiFi status information (#192)
       [not found] <tohojo/flent/pull/192@github.com>
  2019-11-26 10:29 ` [Flent-users] [tohojo/flent] metadata: Add parsing of WiFi status information (#192) Toke Høiland-Jørgensen
  2019-12-02 12:19 ` Toke Høiland-Jørgensen
@ 2019-12-02 12:29 ` Toke Høiland-Jørgensen
  2019-12-02 12:29 ` Toke Høiland-Jørgensen
  2019-12-02 12:35 ` Toke Høiland-Jørgensen
  4 siblings, 0 replies; 5+ messages in thread
From: Toke Høiland-Jørgensen @ 2019-12-02 12:29 UTC (permalink / raw)
  To: tohojo/flent; +Cc: Subscribed


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

git --amend will also add any new changes to the existing commit, not just change the commit message. It's a way to keep the history "clean" (having one logical commit) while making changes.

-- 
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#issuecomment-560375705

[-- Attachment #1.2: Type: text/html, Size: 1746 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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Flent-users] [tohojo/flent] metadata: Add parsing of WiFi status information (#192)
       [not found] <tohojo/flent/pull/192@github.com>
                   ` (2 preceding siblings ...)
  2019-12-02 12:29 ` Toke Høiland-Jørgensen
@ 2019-12-02 12:29 ` Toke Høiland-Jørgensen
  2019-12-02 12:35 ` Toke Høiland-Jørgensen
  4 siblings, 0 replies; 5+ messages in thread
From: Toke Høiland-Jørgensen @ 2019-12-02 12:29 UTC (permalink / raw)
  To: tohojo/flent; +Cc: Subscribed


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

Closed #192 via 55597375caa819a43c3719171b82921d7dc6ad66.

-- 
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#event-2845484024

[-- Attachment #1.2: Type: text/html, Size: 2231 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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Flent-users] [tohojo/flent] metadata: Add parsing of WiFi status information (#192)
       [not found] <tohojo/flent/pull/192@github.com>
                   ` (3 preceding siblings ...)
  2019-12-02 12:29 ` Toke Høiland-Jørgensen
@ 2019-12-02 12:35 ` Toke Høiland-Jørgensen
  4 siblings, 0 replies; 5+ messages in thread
From: Toke Høiland-Jørgensen @ 2019-12-02 12:35 UTC (permalink / raw)
  To: tohojo/flent; +Cc: Subscribed


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

Huh, I guess Github isn't smart enough to figure out that that commit was a manual merge of the PR, so it shows up as "closed" instead of "merged". But don't worry, the actual changes are merged :)

Also, I fixed up the author name on your commit to use your full name; that is good practice, so you may want to update your git config to do this by default. See https://help.github.com/en/github/using-git/setting-your-username-in-git

-- 
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#issuecomment-560377810

[-- Attachment #1.2: Type: text/html, Size: 2083 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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2019-12-02 12:36 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <tohojo/flent/pull/192@github.com>
2019-11-26 10:29 ` [Flent-users] [tohojo/flent] metadata: Add parsing of WiFi status information (#192) Toke Høiland-Jørgensen
2019-12-02 12:19 ` Toke Høiland-Jørgensen
2019-12-02 12:29 ` Toke Høiland-Jørgensen
2019-12-02 12:29 ` Toke Høiland-Jørgensen
2019-12-02 12:35 ` Toke Høiland-Jørgensen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox