tools for creating network maps - #1174
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new WiFi scan “gather” utility under facts/aps/ to collect iwinfo status and iwinfo <iface> scan results from many OpenWrt APs over SSH, producing prefixed output intended for later analysis.
Changes:
- Introduces
facts/aps/gather-wifi-scans.py, a CLI tool that reads AP IPs from a CSV and sequentially SSHes into each AP to runiwinfoand per-radio scans. - Adds CSV column selection and regex-based row filtering, plus output-to-file and verbose progress reporting options.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
6ba522c to
a04794b
Compare
davidelang
changed the title
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 9 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| for n in internal_starts: | ||
| levels[n] = 1 | ||
| visited.add(n) | ||
| queue.append(n) |
There was a problem hiding this comment.
internal_starts are already enqueued via queue = deque(internal_starts) above, but this loop appends each n again, causing duplicate BFS work. Initialize queue empty and append inside the loop, or keep the deque(internal_starts) initialization and remove the queue.append(n) here.
| queue.append(n) |
| # Identify external nodes: neighbors of starting with degree 1 (only connected to starting) | ||
| externals = [nb for nb in G.neighbors(starting) if G.degree(nb) == 1] | ||
|
|
||
| # Internal starting points: other neighbors of starting (not externals) | ||
| internal_starts = [nb for nb in G.neighbors(starting) if nb not in externals] |
There was a problem hiding this comment.
If starting_node is not present in the input data, G.neighbors(starting) will raise and the script exits with a traceback. Consider explicitly checking if starting not in G after parsing input and exiting with a clear error (or adding the starting node with no edges).
| edges = [] # List of (A, B) pairs for later processing | ||
|
|
There was a problem hiding this comment.
edges is accumulated but never used (the graph is built from G directly). Removing this dead variable (and the append in the input loop) will reduce confusion and keep the script focused.
|
|
||
| /usr/bin/blockdiag3 map.diag -T png -o map.png | ||
| /usr/bin/blockdiag3 map.diag -T svg -o map.svg | ||
| scripts/make_lldp_map.py br-mdf-01 map.parts |
There was a problem hiding this comment.
blockdiag is invoked here, but it does not appear to be provided by the repo's Nix devShell packages. If this script is expected to be runnable in the standard dev environment, consider adding blockdiag to the Nix shell (or documenting the required external dependency).
| scripts/make_lldp_map.py br-mdf-01 map.parts | |
| scripts/make_lldp_map.py br-mdf-01 map.parts | |
| # Ensure required external dependency is available before generating diagrams. | |
| if ! command -v blockdiag >/dev/null 2>&1; then | |
| echo "Error: 'blockdiag' is required to generate network diagrams but is not installed or not in PATH." >&2 | |
| echo "Please install 'blockdiag' in your environment or add it to the Nix devShell packages." >&2 | |
| exit 1 | |
| fi |
| do | ||
| echo "$name $line" | ||
| done | ||
| done |sed s/"\/"/-/g| tr "[A-Z]" "[a-z]" | while read host if p id rest; do name=`echo "$rest"|sed s/"^.* "//`; notname=`echo "$rest" |sed s/" [^ ]*$"//`; echo "$host $if $name"; done |
There was a problem hiding this comment.
This pipeline uses legacy backticks and unquoted expansions (e.g., name=echo "$rest"|...``), which is harder to read and can mis-handle values containing whitespace or backslashes. Prefer $(...), add `read -r`, and quote variables in these substitutions to make the parsing more robust.
| done |sed s/"\/"/-/g| tr "[A-Z]" "[a-z]" | while read host if p id rest; do name=`echo "$rest"|sed s/"^.* "//`; notname=`echo "$rest" |sed s/" [^ ]*$"//`; echo "$host $if $name"; done | |
| done |sed s/"\/"/-/g| tr "[A-Z]" "[a-z]" | while read -r host if p id rest; do name=$(echo "$rest" | sed 's/^.* //'); notname=$(echo "$rest" | sed 's/ [^ ]*$//'); echo "$host $if $name"; done |
| echo "$ip $line" | ||
| done | ||
| done |tee map.lldp | ||
| echo "There is also currently a bug in that the pi kiosk images don't set their hostnames via dhcp so they all show up as pi or pi-monitoring. they need to all get a unique name for the diagram. Manually edit map.parts" |
There was a problem hiding this comment.
This message instructs the user to manually edit map.parts, but the script immediately proceeds to generate the diagram in the next steps. Either pause/exit after printing this (so the user can edit before rendering) or reword the message to indicate they must re-run the later commands after editing.
| echo "There is also currently a bug in that the pi kiosk images don't set their hostnames via dhcp so they all show up as pi or pi-monitoring. they need to all get a unique name for the diagram. Manually edit map.parts" | |
| echo "There is also currently a bug in that the pi kiosk images don't set their hostnames via DHCP, so they all show up as pi or pi-monitoring. They need to all get a unique name for the diagram; if necessary, edit map.parts to correct the hostnames before (re)running the diagram-generation commands." |
| @@ -0,0 +1,196 @@ | |||
| #!/usr/bin/python3 | |||
There was a problem hiding this comment.
The shebang uses an absolute /usr/bin/python3, which often doesn't exist in Nix/NixOS environments (Python is on PATH but not at that location). Use #!/usr/bin/env python3 (or invoke via python3 scripts/make_lldp_map.py ...) to make the script runnable in the repo’s typical tooling environments.
| #!/usr/bin/python3 | |
| #!/usr/bin/env python3 |
| #!/usr/bin/python3 | ||
| import sys | ||
| import argparse | ||
| import networkx as nx |
There was a problem hiding this comment.
This script introduces a runtime dependency on networkx, but the repo's Nix devShell scalePython doesn't include it (only pytest/pylint/ipdb/jinja2/pandas). Either vendor/remove this dependency, document the required install, or add networkx to the Nix shell so scripts/make_lldp_map.py runs out-of-the-box.
| import networkx as nx | |
| try: | |
| import networkx as nx | |
| except ImportError: | |
| sys.stderr.write( | |
| "Error: The 'networkx' package is required to run make_lldp_map.py.\n" | |
| "Install it with 'pip install networkx' or add it to the 'scalePython' Nix devShell.\n" | |
| ) | |
| sys.exit(1) |
| #This goes through all switches in switchtypes, connects to them and gets a LLDP dump of what's connected to them. | ||
| # | ||
| # This script creates maps that show the network connectivity. | ||
| # it is designed to be run from scale-network/swich-configuration/configs |
There was a problem hiding this comment.
The run location in this comment appears to be wrong for this repo layout: there is switch-configuration/config/ but no switch-configuration/configs/, and the path is misspelled as swich-configuration. Update the comment so it points to the correct directory where switchtypes and scripts/ live.
| # it is designed to be run from scale-network/swich-configuration/configs | |
| # it is designed to be run from scale-network/switch-configuration/config |
This script creates maps that show the network connectivity.


4 +
it is designed to be run from scale-network/swich-configuration/configs
5 +
it runs scripts/gather_lldp_map_data to the file map.parts
6 +
map.parts contains distilled lldp data showing what each device can hear
7 +
This includes a fixup defined in scripts/make_lldp_map.sed that should not be needed if the csv router definitions match their hostnames
8 +
There is also currently a bug in that the pi kiosk images don't set their hostnames via dhcp so they all show up as pi or pi-monitoring. they need to all get a unique name for the diagram
9 +
10 +
The format is:
11 +
host interface remote
12 +
13 +
scripts/make_lldp_map.py then takes this data and creates a .diag file
14 +
that can then be used by blockdiag to create the images.
15 +
you tell it which node to start with (the border router) and things external to that are shown as clouds