Create story 1.4: location resolution ICAO and address
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,141 @@
|
|||||||
|
# Story 1.4: Location Resolution (ICAO & Address)
|
||||||
|
|
||||||
|
Status: ready-for-dev
|
||||||
|
|
||||||
|
## Story
|
||||||
|
|
||||||
|
As a user setting up the device,
|
||||||
|
I want to type my home airfield ICAO code or my home address/postcode and have the device resolve it to coordinates and show the result for confirmation,
|
||||||
|
So that I can verify the device is centred on the correct location before committing.
|
||||||
|
|
||||||
|
## Acceptance Criteria
|
||||||
|
|
||||||
|
1. **Given** the user enters a valid ICAO code (e.g. `EGLL`) **When** "Find location" is pressed **Then** the bundled `airports.csv` is queried via `importlib.resources` and the matching lat/lon is returned **And** the resolved location name and coordinates are displayed on the portal for confirmation
|
||||||
|
|
||||||
|
2. **Given** the user enters an address or postcode (e.g. `OX1 1AA`) **When** "Find location" is pressed **Then** the Nominatim API is called once with the input and the resolved lat/lon is displayed for confirmation
|
||||||
|
|
||||||
|
3. **Given** the user enters an ICAO code not present in `airports.csv` **When** "Find location" is pressed **Then** the portal displays: "ICAO code not found — try an address instead"
|
||||||
|
|
||||||
|
4. **Given** Nominatim returns no results **When** "Find location" is pressed **Then** the portal displays: "Location not found — try a different search term"
|
||||||
|
|
||||||
|
5. **Given** tests run in CI **When** location tests execute **Then** Nominatim calls are mocked — no real network calls required in the test suite
|
||||||
|
|
||||||
|
## Tasks / Subtasks
|
||||||
|
|
||||||
|
- [ ] Task 1: Implement `location.resolve(query)` in `src/planemapper/provisioning/location.py` (AC: #1, #2, #3, #4)
|
||||||
|
- [ ] 1.1 Normalise the input: `query = query.strip().upper()`
|
||||||
|
- [ ] 1.2 Detect ICAO heuristic: `len(query) == 4 and query.isalpha()` — if true, attempt ICAO lookup first
|
||||||
|
- [ ] 1.3 ICAO lookup: open `airports.csv` via `importlib.resources.files("planemapper.data").joinpath("airports.csv").open("r", encoding="utf-8")`; parse with `csv.DictReader`; search for row where `row["ident"] == query`; return `(float(row["latitude_deg"]), float(row["longitude_deg"]), row["name"])`
|
||||||
|
- [ ] 1.4 If ICAO lookup finds no match, raise `ValueError("ICAO code not found — try an address instead")`
|
||||||
|
- [ ] 1.5 Non-ICAO path: call `requests.get("https://nominatim.openstreetmap.org/search", params={"q": query, "format": "json", "limit": 1}, headers={"User-Agent": "planemapper/0.1 (https://github.com/football2801/planeMapper)"}, timeout=10)`
|
||||||
|
- [ ] 1.6 Parse Nominatim response: if `results` list is non-empty, return `(float(results[0]["lat"]), float(results[0]["lon"]), results[0]["display_name"])`
|
||||||
|
- [ ] 1.7 If Nominatim returns an empty list, raise `ValueError("Location not found — try a different search term")`
|
||||||
|
- [ ] 1.8 Annotate the function signature: `def resolve(query: str) -> tuple[float, float, str]`
|
||||||
|
|
||||||
|
- [ ] Task 2: Add `POST /find-location` route to `src/planemapper/provisioning/portal.py` (AC: #1, #2, #3, #4)
|
||||||
|
- [ ] 2.1 Import `location` from `planemapper.provisioning` at the top of `portal.py`
|
||||||
|
- [ ] 2.2 Implement `POST /find-location` — read `request.form["location"]` field
|
||||||
|
- [ ] 2.3 Call `location.resolve(query)` inside a `try/except ValueError`
|
||||||
|
- [ ] 2.4 On success: return updated form HTML showing the resolved name and coordinates (e.g. a confirmation section with `lat`, `lon`, `name` values visible) and hidden fields pre-populated with `lat`/`lon` for subsequent form submit
|
||||||
|
- [ ] 2.5 On `ValueError`: return updated form HTML with the error message displayed inline (no 4xx status — keep the form usable)
|
||||||
|
- [ ] 2.6 Annotate the route function with return type `str | Response`
|
||||||
|
|
||||||
|
- [ ] Task 3: Write tests in `tests/provisioning/test_location.py` (AC: #1, #2, #3, #4, #5)
|
||||||
|
- [ ] 3.1 Test ICAO lookup hit: call `resolve("EGLL")` against the real `airports.csv`; assert returned `(lat, lon, name)` is a `tuple[float, float, str]` with plausible UK coordinates
|
||||||
|
- [ ] 3.2 Test ICAO lookup miss: call `resolve("ZZZZ")`; assert `ValueError` is raised with message `"ICAO code not found — try an address instead"`
|
||||||
|
- [ ] 3.3 Test Nominatim success: patch `planemapper.provisioning.location.requests.get` with `unittest.mock.patch`; mock return value `.json()` returns `[{"lat": "51.5", "lon": "-0.1", "display_name": "London"}]`; call `resolve("OX1 1AA")`; assert `(51.5, -0.1, "London")` returned
|
||||||
|
- [ ] 3.4 Test Nominatim empty response: patch `requests.get` as above but `.json()` returns `[]`; call `resolve("nonsense query")`; assert `ValueError` is raised with message `"Location not found — try a different search term"`
|
||||||
|
- [ ] 3.5 Assert the mock was called exactly once with the expected URL and `User-Agent` header (confirms no real HTTP in CI)
|
||||||
|
|
||||||
|
- [ ] Task 4: Update portal tests in `tests/provisioning/test_portal.py` (AC: #1, #2, #3, #4)
|
||||||
|
- [ ] 4.1 Add test for `POST /find-location` with a successful resolve (mock `location.resolve` to return `(51.5, -0.1, "London")`); assert 200 and that the response body contains the resolved name and coordinates
|
||||||
|
- [ ] 4.2 Add test for `POST /find-location` with a `ValueError` from `location.resolve`; assert 200 and that the response body contains the error message text
|
||||||
|
|
||||||
|
- [ ] Task 5: Run quality gates
|
||||||
|
- [ ] 5.1 `pytest tests/` — all tests pass, 0 failures
|
||||||
|
- [ ] 5.2 `ruff check .` — zero violations
|
||||||
|
- [ ] 5.3 `ruff format --check .` — no formatting issues
|
||||||
|
|
||||||
|
## Dev Notes
|
||||||
|
|
||||||
|
### CSV parsing via `importlib.resources`
|
||||||
|
|
||||||
|
Use `csv.DictReader` and access the bundled file through `importlib.resources`:
|
||||||
|
|
||||||
|
```python
|
||||||
|
import csv
|
||||||
|
import importlib.resources
|
||||||
|
|
||||||
|
def _lookup_icao(code: str) -> tuple[float, float, str] | None:
|
||||||
|
traversable = importlib.resources.files("planemapper.data").joinpath("airports.csv")
|
||||||
|
with traversable.open("r", encoding="utf-8") as fh:
|
||||||
|
reader = csv.DictReader(fh)
|
||||||
|
for row in reader:
|
||||||
|
if row["ident"] == code:
|
||||||
|
return float(row["latitude_deg"]), float(row["longitude_deg"]), row["name"]
|
||||||
|
return None
|
||||||
|
```
|
||||||
|
|
||||||
|
OurAirports CSV columns used: `ident` (4-letter ICAO code), `name`, `latitude_deg`, `longitude_deg`.
|
||||||
|
|
||||||
|
### ICAO detection heuristic
|
||||||
|
|
||||||
|
```python
|
||||||
|
query = query.strip().upper()
|
||||||
|
if len(query) == 4 and query.isalpha():
|
||||||
|
# try ICAO first
|
||||||
|
```
|
||||||
|
|
||||||
|
This is not a perfect ICAO validator but is sufficient for MVP. A 4-letter all-alpha string is almost certainly an ICAO code in this context.
|
||||||
|
|
||||||
|
### Nominatim call
|
||||||
|
|
||||||
|
```python
|
||||||
|
import requests
|
||||||
|
|
||||||
|
NOMINATIM_URL = "https://nominatim.openstreetmap.org/search"
|
||||||
|
USER_AGENT = "planemapper/0.1 (https://github.com/football2801/planeMapper)"
|
||||||
|
|
||||||
|
def _geocode(query: str) -> tuple[float, float, str] | None:
|
||||||
|
resp = requests.get(
|
||||||
|
NOMINATIM_URL,
|
||||||
|
params={"q": query, "format": "json", "limit": 1},
|
||||||
|
headers={"User-Agent": USER_AGENT},
|
||||||
|
timeout=10,
|
||||||
|
)
|
||||||
|
resp.raise_for_status()
|
||||||
|
results = resp.json()
|
||||||
|
if not results:
|
||||||
|
return None
|
||||||
|
r = results[0]
|
||||||
|
return float(r["lat"]), float(r["lon"]), r["display_name"]
|
||||||
|
```
|
||||||
|
|
||||||
|
The `User-Agent` header is required by Nominatim's usage policy.
|
||||||
|
|
||||||
|
### Return type and coordinate convention
|
||||||
|
|
||||||
|
`resolve(query: str) -> tuple[float, float, str]` — always `(lat, lon, name)`, never `(lon, lat)`. This convention is used throughout the codebase.
|
||||||
|
|
||||||
|
### Test mock path
|
||||||
|
|
||||||
|
```python
|
||||||
|
from unittest.mock import patch, MagicMock
|
||||||
|
|
||||||
|
@patch("planemapper.provisioning.location.requests.get")
|
||||||
|
def test_nominatim_success(mock_get):
|
||||||
|
mock_resp = MagicMock()
|
||||||
|
mock_resp.json.return_value = [{"lat": "51.5", "lon": "-0.1", "display_name": "London"}]
|
||||||
|
mock_get.return_value = mock_resp
|
||||||
|
lat, lon, name = resolve("OX1 1AA")
|
||||||
|
assert lat == 51.5
|
||||||
|
assert lon == -0.1
|
||||||
|
assert name == "London"
|
||||||
|
mock_get.assert_called_once()
|
||||||
|
```
|
||||||
|
|
||||||
|
Patching at `planemapper.provisioning.location.requests.get` (the module where `requests` is imported, not the `requests` package directly) ensures no real HTTP calls reach Nominatim in CI.
|
||||||
|
|
||||||
|
### `airports.csv` file path
|
||||||
|
|
||||||
|
The CSV must be declared as package data in `pyproject.toml` (or `setup.cfg`) so that `importlib.resources` can find it at runtime and during tests. Confirm `[tool.setuptools.package-data]` includes `"planemapper.data" = ["*.csv"]` (this should already be in place from Story 1.1 scaffold).
|
||||||
@@ -47,7 +47,7 @@ development_status:
|
|||||||
1-1-project-scaffold-and-verified-entry-points: done
|
1-1-project-scaffold-and-verified-entry-points: done
|
||||||
1-2-configuration-read-write-wipe: done
|
1-2-configuration-read-write-wipe: done
|
||||||
1-3-wifi-hotspot-and-captive-portal-form: done
|
1-3-wifi-hotspot-and-captive-portal-form: done
|
||||||
1-4-location-resolution-icao-and-address: backlog
|
1-4-location-resolution-icao-and-address: ready-for-dev
|
||||||
1-5-provisioning-execution-tile-download-cache-validation-and-wifi-kill: backlog
|
1-5-provisioning-execution-tile-download-cache-validation-and-wifi-kill: backlog
|
||||||
epic-1-retrospective: optional
|
epic-1-retrospective: optional
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user