* 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