From 699a56a0ed49c691e262f3be559fbf196ecb7926 Mon Sep 17 00:00:00 2001 From: "Josef M. Gallmetzer" <64498081+galjos@users.noreply.github.com> Date: Wed, 2 Sep 2026 14:47:40 +0200 Subject: [PATCH] Require explicit fee=no for --free and explicit public access Unknown OSM fee tags were passing --free, so most "free" results near Bolzano could still bill. --public had the same unknown-tag leak. Refs #6. --- CHANGELOG.md | 10 ++++++++++ src/pitstop/chargers.py | 6 ++++-- src/pitstop/cli.py | 6 ++++-- tests/test_chargers.py | 33 +++++++++++++++++++++++++++++++++ 4 files changed, 51 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c8132a..d0b556b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,16 @@ All notable changes to this project are documented here. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project aims to follow [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Fixed + +- `--free` now returns only chargers with `fee=no`. Unknown fee (absent OSM + tag) used to pass the filter alongside true free stations, so most `--free` + hits near Bolzano were chargers that might bill. Refs #6. +- `--public` now requires an explicit `public` / `yes` / `permissive` access + tag; a missing access tag no longer counts as public. + ## [1.1.0] - 2026-08-02 Correctness release: closes gaps between what `pitstop` claimed and what it diff --git a/src/pitstop/chargers.py b/src/pitstop/chargers.py index c4aa683..b52e98a 100644 --- a/src/pitstop/chargers.py +++ b/src/pitstop/chargers.py @@ -220,9 +220,11 @@ def find_chargers( continue if min_power_kw > 0 and (st.max_power_kw is None or st.max_power_kw < min_power_kw): continue - if free_only and st.fee is True: + # Unknown fee/access must not pass affirmative filters: --free means + # fee=no, not "fee is not yes"; --public means an explicit public tag. + if free_only and st.fee is not False: continue - if public_only and st.access and st.access.lower() not in ("public", "yes", "permissive"): + if public_only and st.access.lower() not in ("public", "yes", "permissive"): continue st.distance_km = round(haversine_km(near[0], near[1], st.lat, st.lon), 2) st.tariff_info_url = cpo_tariffs.lookup(st.operator) diff --git a/src/pitstop/cli.py b/src/pitstop/cli.py index 8017ee9..a6034e8 100644 --- a/src/pitstop/cli.py +++ b/src/pitstop/cli.py @@ -105,8 +105,10 @@ def _build_parser() -> argparse.ArgumentParser: chargers.add_argument("--fast", action="store_true", help="shortcut for --min-power 50") chargers.add_argument("--ultra-fast", dest="ultra_fast", action="store_true", help="shortcut for --min-power 150") - chargers.add_argument("--free", action="store_true", help="only chargers explicitly free (fee=no)") - chargers.add_argument("--public", action="store_true", help="only public access") + chargers.add_argument("--free", action="store_true", + help="only chargers explicitly free (fee=no); unknown fee is excluded") + chargers.add_argument("--public", action="store_true", + help="only chargers with explicit public/yes/permissive access; unknown access is excluded") chargers.add_argument("--limit", type=int, default=20, help="max stations; 0 = no limit") chargers.add_argument("--json", dest="as_json", action="store_true") chargers.add_argument("--geojson", dest="as_geojson", action="store_true", help="emit GeoJSON FeatureCollection") diff --git a/tests/test_chargers.py b/tests/test_chargers.py index 8cdf604..a093e27 100644 --- a/tests/test_chargers.py +++ b/tests/test_chargers.py @@ -134,6 +134,39 @@ def test_find_chargers_filters_operator(monkeypatch): assert [s.osm_id for s in only] == [20] +def test_find_chargers_free_only_requires_fee_no(monkeypatch): + """--free means fee=no; unknown (absent tag) and fee=yes must not pass.""" + elements = [ + _node(40, 46.50, 11.35, {"amenity": "charging_station", "fee": "no", + "socket:type2": "1"}), + _node(41, 46.50, 11.35, {"amenity": "charging_station", "fee": "yes", + "socket:type2": "1"}), + _node(42, 46.50, 11.35, {"amenity": "charging_station", + "socket:type2": "1"}), # fee absent -> None + ] + monkeypatch.setattr(chargers.overpass, "fetch_elements", lambda *a, **k: (elements, None)) + only, error = chargers.find_chargers(near=(46.50, 11.35), radius_km=5, free_only=True) + assert error is None + assert [s.osm_id for s in only] == [40] + assert only[0].fee is False + + +def test_find_chargers_public_only_requires_explicit_public(monkeypatch): + """--public must not treat a missing access tag as public.""" + elements = [ + _node(50, 46.50, 11.35, {"amenity": "charging_station", "access": "public", + "socket:type2": "1"}), + _node(51, 46.50, 11.35, {"amenity": "charging_station", "access": "private", + "socket:type2": "1"}), + _node(52, 46.50, 11.35, {"amenity": "charging_station", + "socket:type2": "1"}), # access absent + ] + monkeypatch.setattr(chargers.overpass, "fetch_elements", lambda *a, **k: (elements, None)) + only, error = chargers.find_chargers(near=(46.50, 11.35), radius_km=5, public_only=True) + assert error is None + assert [s.osm_id for s in only] == [50] + + # ---- v0.9.0 additions: error envelope, GeoJSON, MCP bilingual normalize ----