tftp: say what to do when the server cannot take its address

[openipc-ai]

Follow-up from OpenIPC/firmware#2299.

The report

A user running defib install hit this partway through Phase 2:

It gets into Phase 2 then returns an error "OS Address already in use"
If I do an "ip address" I don't see 192.168.1.10 or the 192.168.1.0/24 network defined or in use

They went hunting for a missing IP, which was the wrong trail — EADDRINUSE means something else already holds UDP/69. The kernel's text names neither the address, nor the port, nor the fix, and this fails in the middle of a flash recovery on a camera that is already half-written.

The change

start_tftp_server let the bare OSError out of create_datagram_endpoint. It now raises TFTPBindError with something actionable, distinguishing the three failures that actually occur here:

errno	message
EADDRINUSE	names the port, points at sudo ss -ulpn 'sport = :69' to find the holder (system tftpd, dnsmasq, a previous defib), and mentions --tftp-via pod
EADDRNOTAVAIL	the IP is not on any interface — gives the ip addr add line and ip -brief address to check
EACCES/EPERM	port below 1024 needs root

Anything else still names the address and includes the original text. The OSError stays attached as __cause__.

Fixed inside start_tftp_server rather than at the call sites, so all three (recover, install, flash) get it.

Tests

5 new, including an end-to-end one that binds an ephemeral UDP port and confirms the real bind path raises TFTPBindError rather than OSError.

739 passed, 2 skipped

Message text only — no behaviour change to a successful transfer.

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Add actionable TFTP bind failure diagnostics

🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

AI Description

• Replace raw TFTP bind failures with actionable, typed diagnostics.
• Tailor remediation for occupied ports, missing addresses, and insufficient privileges.
• Cover formatted messages and the real UDP bind failure path.

Diagram

graph TD
  CLI["Recovery commands"] --> Start["Start TFTP"] --> Bind["Bind UDP socket"] --> Result{"Bind succeeds?"}
  Result -->|Yes| Server["Running server"]
  Result -->|No| Diagnose["Map bind errno"] --> Error["TFTPBindError"]

Loading

High-Level Assessment

Centralizing translation in start_tftp_server is the best approach because every local TFTP caller receives consistent diagnostics and preserved exception context. Handling errors separately in recover, install, and flash would duplicate errno mapping and risk divergent guidance.

Files changed (2) +113 / -4

Bug fix (1) +46 / -4

tftp_server.pyTranslate UDP bind failures into actionable TFTP errors +46/-4

Translate UDP bind failures into actionable TFTP errors

• Adds TFTPBindError and errno-specific guidance for occupied ports, unavailable addresses, and permission failures, with a contextual fallback for unknown errors. Wraps datagram endpoint creation so every caller receives the typed exception while retaining the original OSError as __cause__.

src/defib/network/tftp_server.py

Tests (1) +67 / -0

test_tftp_bind_errors.pyVerify bind diagnostics and typed exception propagation +67/-0

Verify bind diagnostics and typed exception propagation

• Adds focused coverage for known and unknown errno messages. An end-to-end socket test occupies an ephemeral UDP port and confirms the real startup path raises TFTPBindError with the original OSError chained.

tests/test_tftp_bind_errors.py

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Port-holder command is Linux-only ✓ Resolved 🐞 Bug ≡ Correctness

Description

The EADDRINUSE branch always tells users to run Linux's sudo ss, so macOS and Windows users
receive a command that is unavailable instead of an actionable way to identify the process holding
UDP/69. This contradicts the repository's explicit cross-platform support.

Code

src/defib/network/tftp_server.py[R281-282]

+            f"exited. Find it with `sudo ss -ulpn 'sport = :{port}'` and stop "
+            f"it, or use --tftp-via pod if you have a rack pod."

Evidence

The changed branch hard-codes sudo ss, while the repository documents and implements Linux, macOS,
and Windows support and runs its tests on all three operating systems.

src/defib/network/tftp_server.py[276-283]
src/defib/network/ip_manager.py[1-10]
.github/workflows/ci.yml[10-17]
pyproject.toml[9-17]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The EADDRINUSE diagnostic unconditionally recommends the Linux-only `sudo ss` command, although defib supports Linux, macOS, and Windows.
## Issue Context
Select remediation using `sys.platform`, giving each supported OS a valid command for finding the process bound to the UDP port. Keep the rack-pod alternative where applicable.
## Fix Focus Areas
- src/defib/network/tftp_server.py[276-283]
- tests/test_tftp_bind_errors.py[21-26]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Address fix is Linux-only ✓ Resolved 🐞 Bug ≡ Correctness

Description

The EADDRNOTAVAIL branch unconditionally recommends ip addr add and ip -brief, but defib's
supported macOS and Windows paths require ifconfig and netsh respectively. Users on those
platforms cannot apply the printed recovery instructions.

Code

src/defib/network/tftp_server.py[R287-289]

+            f"The host needs {bind_addr} configured on the NIC the camera is "
+            f"plugged into, e.g. `sudo ip addr add {bind_addr}/24 dev <nic>` "
+            f"-- check `ip -brief address` to see what is actually set."

Evidence

The changed message always emits Linux commands, whereas add_ip explicitly dispatches to ip on
Linux, ifconfig on macOS, and netsh on Windows.

src/defib/network/tftp_server.py[284-290]
src/defib/network/ip_manager.py[43-63]
src/defib/network/ip_manager.py[7-10]
.github/workflows/ci.yml[10-17]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The EADDRNOTAVAIL diagnostic prints Linux `ip` commands on every supported operating system.
## Issue Context
Reuse the platform distinctions already implemented by `ip_manager`, or provide platform-neutral guidance that directs users to the existing interface configuration flow.
## Fix Focus Areas
- src/defib/network/tftp_server.py[284-290]
- src/defib/network/ip_manager.py[43-63]
- tests/test_tftp_bind_errors.py[29-32]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Privilege advice targets wrong platform ✓ Resolved 🐞 Bug ≡ Correctness

Description

The permission branch says ports below 1024 need root and recommends sudo or
CAP_NET_BIND_SERVICE unconditionally, although these are Linux-specific mechanisms and defib
supports Windows and macOS. It also calls defib a binary even though the package installs it as a
Python console-script entry point, making the capability instruction directly inapplicable as
written.

Code

src/defib/network/tftp_server.py[R293-295]

+            f"TFTP server cannot bind {where}: permission denied. Ports below "
+            f"1024 need root -- run defib with sudo, or grant the binary "
+            f"CAP_NET_BIND_SERVICE."

Evidence

The changed text unconditionally prescribes root, sudo, and a Linux capability; the server's own
existing docstring scopes the privileged-port statement to Linux, cross-platform networking supports
three OS families, and packaging defines defib as a Python entry point rather than a native
binary.

src/defib/network/tftp_server.py[291-296]
src/defib/network/tftp_server.py[310-316]
src/defib/network/ip_manager.py[7-10]
pyproject.toml[40-41]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The EACCES/EPERM diagnostic presents Linux privilege rules and capabilities as universal and tells users to grant a capability to a nonexistent defib binary.
## Issue Context
Emit platform-appropriate elevation guidance. On Linux, accurately identify what executable can receive `CAP_NET_BIND_SERVICE`, or omit that risky shorthand and recommend a supported invocation.
## Fix Focus Areas
- src/defib/network/tftp_server.py[291-296]
- tests/test_tftp_bind_errors.py[35-38]
- pyproject.toml[40-41]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Tip of the day

💡 Did you know, you can hide the parts of a finding you never read, like the evidence or the agent prompt

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗