Consolidate Wi‑Fi workflow: add --dry-run, tracing, and deeper diagnostics (v11.1.0)#15
Draft
ib-bsb-br wants to merge 1 commit into
Draft
Consolidate Wi‑Fi workflow: add --dry-run, tracing, and deeper diagnostics (v11.1.0)#15ib-bsb-br wants to merge 1 commit into
--dry-run, tracing, and deeper diagnostics (v11.1.0)#15ib-bsb-br wants to merge 1 commit into
Conversation
Reviewer's GuideIntroduces a consolidated v11.1.0 netconnect.sh network management script with structured logging, dry-run and tracing support, more robust Wi‑Fi workflows and diagnostics, and updates the README to document capabilities and new flags. Sequence diagram for WiFi connection workflow with dry_run and backend fallbackssequenceDiagram
actor User
participant Main as MainFlow_main
participant WifiFlow as connect_wifi_iface
participant Safety as WifiSafetyAndState
participant DHCP as configure_dhcp_iface
participant Trace as run_action_logged
participant NM as nmcli_backend
participant IWD as iwctl_backend
participant WPA as wpa_cli_backend
participant Mod as modprobe_driver_reload
User->>Main: invoke netconnect.sh -n -i wlan0 -t wifi --ssid SSID [--dry-run]
Main->>Main: parse_args()
Main->>Main: detect_os(), detect_network_stack()
Main->>WifiFlow: configure_wifi_flow(iface, ssid, pass, sec, hidden)
WifiFlow->>WifiFlow: connect_wifi_iface(iface, ssid, pass, sec, hidden)
WifiFlow->>Safety: ensure_wifi_unblocked(iface)
Safety->>Trace: run_cmd_logged(rfkill list, rfkill list)
alt hard block detected
Safety-->>WifiFlow: fail (HARD blocked)
WifiFlow-->>Main: error
Main-->>User: exit non_zero
else no hard block
WifiFlow->>Safety: wifi_state_snapshot(iface)
WifiFlow->>Safety: nm_disconnect_iface(iface)
loop attempts 1..MAX_BACKEND_RETRIES
alt NM_AVAILABLE
WifiFlow->>NM: connect_wifi_nmcli()
NM->>Trace: run_action_logged(nmcli wifi connect ...)
alt success
WifiFlow->>DHCP: configure_dhcp_iface(iface) if needed
DHCP->>Trace: run_action_logged(dhcp backend)
DHCP-->>WifiFlow: success
WifiFlow-->>Main: success
Main-->>User: exit 0
else nmcli failed
NM-->>WifiFlow: set_last_wifi_error(nmcli, reason)
end
end
alt IWD_AVAILABLE and not connected
WifiFlow->>IWD: connect_wifi_iwd()
IWD->>Trace: run_action_logged(iwctl station connect)
alt success
WifiFlow->>DHCP: configure_dhcp_iface(iface)
DHCP->>Trace: run_action_logged(dhcp backend)
DHCP-->>WifiFlow: success
WifiFlow-->>Main: success
Main-->>User: exit 0
else iwctl failed
IWD-->>WifiFlow: set_last_wifi_error(iwctl, reason)
end
end
alt WPA_CLI_AVAILABLE and not connected
WifiFlow->>WPA: connect_wifi_wpa_cli()
WPA->>Trace: run_action_logged(wpa_cli add_network, set_network, enable)
alt success
WifiFlow->>DHCP: configure_dhcp_iface(iface)
DHCP->>Trace: run_action_logged(dhcp backend)
DHCP-->>WifiFlow: success
WifiFlow-->>Main: success
Main-->>User: exit 0
else wpa_cli failed
WPA-->>WifiFlow: set_last_wifi_error(wpa_cli, reason)
end
end
end
alt all backends failed
opt DRY_RUN is false and iface_is_wifi and modprobe available
WifiFlow->>Mod: driver reload sequence
Mod->>Trace: run_action_logged(modprobe -r driver)
Mod->>Trace: run_action_logged(modprobe driver)
WifiFlow->>Safety: nm_disconnect_iface(iface)
WifiFlow->>NM: optional final nmcli/other retry
end
WifiFlow->>Safety: wifi_state_snapshot(iface)
WifiFlow->>Safety: network_failure_hints(iface)
WifiFlow-->>Main: failure (LAST_WIFI_BACKEND, LAST_WIFI_ERROR)
Main-->>User: exit non_zero
end
end
note over Trace: When DRY_RUN is true, run_action_logged logs
note over Trace: [DRY-RUN] tag :: command and skips execution
Class diagram for consolidated netconnect.sh v11_1_0 structureclassDiagram
class MainFlow {
+string SCRIPT_NAME
+string SCRIPT_VERSION
+bool NON_INTERACTIVE
+bool CHECK_ONLY
+bool SHOW_STATUS
+bool INSTALL_DEPS
+bool SHOW_VERSION
+bool DRY_RUN
+bool TRACE_TO_FILE
+string TARGET_IFACE
+string TARGET_TYPE
+string TARGET_METHOD
+string WIFI_SSID
+string WIFI_PASSWORD
+bool WIFI_PASSWORD_STDIN
+string WIFI_SECURITY
+bool WIFI_HIDDEN
+int DEBUG_LEVEL
+bool DBG
+int main(argc, argv)
+void parse_args(argc, argv)
+void interactive_menu()
+int auto_connect_flow()
}
class LoggingAndTracing {
+string LOG_DIR
+string LOG_FILE
+string TRACE_DIR
+string RUNTIME_DIR
+bool TRACE_TO_FILE
+bool DRY_RUN
+void log(level, message)
+void debug(message)
+void debugv(message)
+void debugvv(message)
+int run_cmd_logged(tag, command, args)
+int run_action_logged(tag, command, args)
+string _mktemp_runtime()
}
class EnvironmentAndLocking {
+string LOCK_FILE
+string CONFIG_DIR
+string PROFILE_DIR
+void require_root()
+void acquire_lock()
+void init_dirs()
+void cleanup()
}
class OsAndStackDetection {
+string OS_ID
+string OS_VERSION_ID
+string OS_PRETTY
+string PKG_MANAGER
+bool SYSTEMD_AVAILABLE
+bool RFKILL_AVAILABLE
+bool NM_AVAILABLE
+bool IWD_AVAILABLE
+bool WPA_CLI_AVAILABLE
+void detect_os()
+void detect_network_stack()
+void install_optional_deps()
+bool service_active(svc)
}
class DiagnosticsModule {
+int connectivity_check()
+int diagnostics_report()
+void show_status()
+void report_system()
+void report_interfaces()
+void report_routes()
+void report_dns()
+void report_dhcp_leases()
+void report_logs()
+void report_neighbors()
+void report_interface_stats()
+void report_ports()
+void report_firewall()
+void report_network_managers()
+void report_path_diagnostics()
}
class InterfaceAndIpHelpers {
+bool iface_exists(iface)
+bool iface_is_wifi(iface)
+bool iface_has_ipv4(iface)
+string wifi_connected_ssid(iface)
+int bring_iface_up(iface)
+string get_eth_ifaces()
+string get_wifi_ifaces()
+bool is_valid_ipv4(ip)
+bool is_valid_cidr(cidr)
}
class WifiSafetyAndState {
+bool RFKILL_AVAILABLE
+string LAST_WIFI_BACKEND
+string LAST_WIFI_ERROR
+void set_last_wifi_error(backend, reason)
+bool rfkill_wifi_hard_blocked()
+int ensure_wifi_unblocked(iface)
+void wifi_state_snapshot(iface)
+void network_failure_hints(iface)
+string iface_driver(iface)
+void nm_disconnect_iface(iface)
}
class WifiScanAndBackends {
+int scan_wifi_networks(iface)
+string scan_wifi_nmcli(iface)
+string scan_wifi_iw(iface)
+string scan_wifi_wpa_cli(iface)
+bool ensure_wpa_supplicant_socket(iface)
+int connect_wifi_nmcli(iface, ssid, pass, hidden)
+int connect_wifi_iwd(iface, ssid, pass)
+int connect_wifi_wpa_cli(iface, ssid, pass, sec, hidden)
+int connect_wifi_iface(iface, ssid, pass, sec, hidden)
}
class IpConfigModule {
+int configure_dhcp_iface(iface)
+int configure_static_iface(iface, cidr, gw, dns_csv)
+int configure_dns(dns_csv)
+int configure_ethernet_flow(iface, method)
+int configure_wifi_flow(iface, ssid, pass, sec, hidden)
}
class WifiProfiles {
+string PROFILE_DIR
+string profile_path_for(iface, ssid)
+int save_wifi_profile(iface, ssid, pass, sec, hidden)
+bool load_wifi_profile_safe(profile_file)
}
class InteractiveUiHelpers {
+string prompt(text, def)
+string prompt_secret(text)
+string choose_from_list(title, list)
+void print_header(text)
}
class Utils {
+bool have(cmd)
+string ts()
+string trim(value)
+string safe_iface(iface)
+string mask_secret(value)
}
MainFlow --> LoggingAndTracing
MainFlow --> EnvironmentAndLocking
MainFlow --> OsAndStackDetection
MainFlow --> DiagnosticsModule
MainFlow --> InterfaceAndIpHelpers
MainFlow --> WifiSafetyAndState
MainFlow --> WifiScanAndBackends
MainFlow --> IpConfigModule
MainFlow --> WifiProfiles
MainFlow --> InteractiveUiHelpers
MainFlow --> Utils
DiagnosticsModule --> LoggingAndTracing
IpConfigModule --> LoggingAndTracing
WifiScanAndBackends --> LoggingAndTracing
WifiSafetyAndState --> LoggingAndTracing
WifiProfiles --> LoggingAndTracing
OsAndStackDetection --> LoggingAndTracing
WifiScanAndBackends --> WifiSafetyAndState
WifiScanAndBackends --> IpConfigModule
IpConfigModule --> InterfaceAndIpHelpers
WifiSafetyAndState --> InterfaceAndIpHelpers
InteractiveUiHelpers --> IpConfigModule
InteractiveUiHelpers --> WifiScanAndBackends
InteractiveUiHelpers --> WifiProfiles
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly upgrades the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This is an impressive and comprehensive update to the netconnect.sh script. The addition of --dry-run, tracing, and enhanced diagnostics significantly improves its usability and debuggability. The code is well-structured, robust with extensive error handling and fallbacks, and shows great attention to security and portability concerns. I've made a few minor suggestions to further improve robustness and efficiency in a couple of areas. The updated README.md is also clear and provides excellent documentation for the new features.
Comment on lines
+698
to
+711
| for pat in "${patterns[@]}"; do | ||
| # shellcheck disable=SC2086 | ||
| for f in $pat; do | ||
| [[ -f "$f" ]] || continue | ||
| found=1 | ||
| echo "Lease file: $f" | ||
| if file "$f" 2>/dev/null | grep -qiE 'text|ascii|utf'; then | ||
| tail -n 50 "$f" || true | ||
| else | ||
| echo "(binary/non-text lease file skipped)" | ||
| fi | ||
| echo "----------------------------------------" | ||
| done | ||
| done |
There was a problem hiding this comment.
The inner loop for f in $pat is not safe for filenames containing spaces due to word splitting. While lease files with spaces are uncommon, it's more robust to handle this case.
You can fix this by temporarily setting IFS to a newline for the duration of the inner loop.
Suggested change
| for pat in "${patterns[@]}"; do | |
| # shellcheck disable=SC2086 | |
| for f in $pat; do | |
| [[ -f "$f" ]] || continue | |
| found=1 | |
| echo "Lease file: $f" | |
| if file "$f" 2>/dev/null | grep -qiE 'text|ascii|utf'; then | |
| tail -n 50 "$f" || true | |
| else | |
| echo "(binary/non-text lease file skipped)" | |
| fi | |
| echo "----------------------------------------" | |
| done | |
| done | |
| for pat in "${patterns[@]}"; do | |
| local old_ifs=$IFS | |
| IFS=$'\n' | |
| # shellcheck disable=SC2086 | |
| for f in $pat; do | |
| [[ -f "$f" ]] || continue | |
| found=1 | |
| echo "Lease file: $f" | |
| if file "$f" 2>/dev/null | grep -qiE 'text|ascii|utf'; then | |
| tail -n 50 "$f" || true | |
| else | |
| echo "(binary/non-text lease file skipped)" | |
| fi | |
| echo "----------------------------------------" | |
| done | |
| IFS=$old_ifs | |
| done |
|
|
||
| scan_wifi_iw() { | ||
| local iface="$1" | ||
| iw dev "$iface" scan 2>/dev/null | awk ' |
There was a problem hiding this comment.
The awk script for parsing iw scan output does not trim trailing whitespace from the SSID. This could lead to connection failures if the AP name has trailing spaces in the scan output but not in reality. Pre-processing the output with sed to trim trailing whitespace from all lines is a robust way to fix this.
Suggested change
| iw dev "$iface" scan 2>/dev/null | awk ' | |
| iw dev "$iface" scan 2>/dev/null | sed 's/[[:space:]]*$//' | awk ' |
Comment on lines
+1401
to
+1447
| auto_connect_flow() { | ||
| local rc=1 iface profile pass_decoded | ||
|
|
||
| mapfile -t eths < <(get_eth_ifaces) | ||
| for iface in "${eths[@]:-}"; do | ||
| [[ -z "$iface" ]] && continue | ||
| log INFO "Trying Ethernet DHCP on $iface" | ||
| if configure_dhcp_iface "$iface"; then | ||
| rc=0 | ||
| break | ||
| fi | ||
| done | ||
|
|
||
| if [[ $rc -ne 0 && -n "${WIFI_SSID:-}" ]]; then | ||
| mapfile -t wlans < <(get_wifi_ifaces) | ||
| for iface in "${wlans[@]:-}"; do | ||
| [[ -z "$iface" ]] && continue | ||
| log INFO "Trying Wi-Fi on $iface with provided SSID" | ||
| if connect_wifi_iface "$iface" "$WIFI_SSID" "${WIFI_PASSWORD:-}" "${WIFI_SECURITY:-WPA2-PSK}" "$WIFI_HIDDEN"; then | ||
| save_wifi_profile "$iface" "$WIFI_SSID" "${WIFI_PASSWORD:-}" "${WIFI_SECURITY:-WPA2-PSK}" "$WIFI_HIDDEN" | ||
| rc=0 | ||
| break | ||
| fi | ||
| done | ||
| fi | ||
|
|
||
| if [[ $rc -ne 0 ]]; then | ||
| mapfile -t wlans2 < <(get_wifi_ifaces) | ||
| for iface in "${wlans2[@]:-}"; do | ||
| [[ -z "$iface" ]] && continue | ||
| for profile in "$PROFILE_DIR/${iface}_"*.conf; do | ||
| [[ -f "$profile" ]] || continue | ||
| load_wifi_profile_safe "$profile" || continue | ||
| pass_decoded="" | ||
| [[ -n "${PASSWORD_B64:-}" ]] && pass_decoded=$(printf '%s' "$PASSWORD_B64" | base64 -d 2>/dev/null || true) | ||
|
|
||
| log INFO "Trying saved profile on $iface (SSID=${SSID:-unknown})" | ||
| if connect_wifi_iface "$iface" "${SSID:-}" "$pass_decoded" "${SECURITY:-WPA2-PSK}" "${HIDDEN:-false}"; then | ||
| rc=0 | ||
| break 2 | ||
| fi | ||
| done | ||
| done | ||
| fi | ||
|
|
||
| return "$rc" | ||
| } |
There was a problem hiding this comment.
The function auto_connect_flow calls get_wifi_ifaces twice (lines 1415 and 1428). This is slightly inefficient. You can call it once at the beginning of the function and store the result in an array to be reused.
auto_connect_flow() {
local rc=1 iface profile pass_decoded
mapfile -t eths < <(get_eth_ifaces)
for iface in "${eths[@]:-}"; do
[[ -z "$iface" ]] && continue
log INFO "Trying Ethernet DHCP on $iface"
if configure_dhcp_iface "$iface"; then
rc=0
break
fi
done
if [[ $rc -ne 0 ]]; then
mapfile -t wlans < <(get_wifi_ifaces)
if [[ -n "${WIFI_SSID:-}" ]]; then
for iface in "${wlans[@]:-}"; do
[[ -z "$iface" ]] && continue
log INFO "Trying Wi-Fi on $iface with provided SSID"
if connect_wifi_iface "$iface" "$WIFI_SSID" "${WIFI_PASSWORD:-}" "${WIFI_SECURITY:-WPA2-PSK}" "$WIFI_HIDDEN"; then
save_wifi_profile "$iface" "$WIFI_SSID" "${WIFI_PASSWORD:-}" "${WIFI_SECURITY:-WPA2-PSK}" "$WIFI_HIDDEN"
rc=0
break
fi
done
fi
if [[ $rc -ne 0 ]]; then
for iface in "${wlans[@]:-}"; do
[[ -z "$iface" ]] && continue
for profile in "$PROFILE_DIR/${iface}_"*.conf; do
[[ -f "$profile" ]] || continue
load_wifi_profile_safe "$profile" || continue
pass_decoded=""
[[ -n "${PASSWORD_B64:-}" ]] && pass_decoded=$(printf '%s' "$PASSWORD_B64" | base64 -d 2>/dev/null || true)
log INFO "Trying saved profile on $iface (SSID=${SSID:-unknown})"
if connect_wifi_iface "$iface" "${SSID:-}" "$pass_decoded" "${SECURITY:-WPA2-PSK}" "${HIDDEN:-false}"; then
rc=0
break 2
fi
done
done
fi
fi
return "$rc"
}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
--dry-runmode so operators can preview mutating actions.Description
v11.1.0flow innetconnect.shthat introducesrun_cmd_loggedandrun_action_logged,TRACE_TO_FILE, andDRY_RUNto capture and (optionally) persist backend stdout/stderr.wifi_state_snapshot,ensure_wifi_unblocked/rfkill hard/soft handling, backend error tracking viaLAST_WIFI_BACKEND/LAST_WIFI_ERROR, andnetwork_failure_hints.nmcli/iwctl/wpa_cliintegration, guarded driver reload as a last resort, and avoidance of blind reconnects when L3 is already present.load_wifi_profile_safe/save_wifi_profilewith base64 password storage,--password-stdinsupport, updated interactive menu semantics that trackdid_config/last_config_ok, and bumpedSCRIPT_VERSIONto11.1.0.README.mdto document--dry-run,--trace-to-file,--password-stdin, and the expanded Wi‑Fi troubleshooting guidance.Testing
bash -n netconnect.shwhich completed without syntax errors (success)../netconnect.sh --versionwhich printedv11.1.0(success)../netconnect.sh --helpto confirm new flags and usage text (success)../netconnect.sh --dry-run -n -i lo -t ethernet -m dhcpwhich produced dry‑run logging and diagnostic output but was limited by the container environment lacking full network toolchain (functional smoke; environment‑dependent results).Codex Task
Summary by Sourcery
Introduce a consolidated v11.1.0 netconnect.sh network diagnostics and connection manager with improved Wi‑Fi workflows, dry‑run support, and richer tracing/diagnostics, and document its capabilities and usage.
New Features:
Enhancements:
Documentation:
Summary by cubic
Consolidates the Wi‑Fi workflow and adds
--dry-run, command tracing, and deeper diagnostics to make connections more predictable and easier to debug across modern distros. Introduces unifiednmcli/iwctl/wpa_clibackends, safer profiles, and improved interactive behavior (v11.1.0).New Features
--dry-runto preview mutating actions without applying changes.--trace-to-fileto persist per-command stdout/stderr under/var/log/netconnect/trace/commands.trace.nmcli->iwctl->wpa_cli) with guarded driver reload as a last resort and no blind reconnect when L3 is present.--password-stdin.Refactors
netconnect.shv11.1.0 and addedrun_cmd_logged/run_action_loggedfor structured logging and tracing.nmclioriproute2) and DNS helpers, plus updatedREADME.mdwith new flags and Wi‑Fi troubleshooting.Written for commit b422355. Summary will update on new commits.