* 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