fix(places): correct the shipped default station id (#641) - #642
Merged
Conversation
DEFAULT_CONFIG's [[place]] named "Schöneweide" shipped station id 900180001, which is a different, real, nearby station on the live BVG API: "S Köpenick/Parrisiusstr. (Berlin)", served only by S3 — none of which passes through Köpenick. The fetch always succeeded and always filtered to zero rows, rendering as the plain "no matching S-Bahn departures right now" text rather than an error, so the widget looked like a quiet evening instead of telling anyone it was misconfigured. The correct id, verified against the live API, is 900192001 = "S Schöneweide Bhf (Berlin)" — the station the place is actually named after. A comment naming the station in words now sits next to the id so the two can't silently drift apart again. Also reconsidered the lines/directions defaults: a populated filter that's wrong (as here) fails invisibly, while an absent filter shows everything through the station and fails visibly/usefully. Shipped with both axes empty by default, keeping the previous example values as commented-out template lines with a note on what they do. Test fallout: the three places.rs tests that pinned the literal "900180001" (self-consistently, with nothing to check it against) are updated, and a new test ties the default station id to the place name it must match, with the real-world station names spelled out in its failure message. This only changes what write_default_config writes for NEW installs (it only ever writes when places.toml is absent) — existing configs are untouched and need a manual edit. Closes #641
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
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.
Closes #641
The bug, confirmed against the live BVG API
crates/hytte-services/src/places.rs'sDEFAULT_CONFIGshipped a[[place]]named"Schöneweide"withstation = "900180001". Resolving both ids against the live API:900180001→ "S Köpenick/Parrisiusstr. (Berlin)" — a real, different, nearby station. The only S-Bahn line calling there is S3.900192001→ "S Schöneweide Bhf (Berlin)" — the station the place is actually named after.So the default fetched Köpenick and then filtered for
S8/S85/S9, none of which ever call there — a structural, permanent zero-match. The fetch succeeds and the JSON decodes cleanly, so this renders as the plain"no matching S-Bahn departures right now"text (hytte-plugin-departures/src/main.rs'sOk/Staleempty-items arm), not an error — it looks like a quiet evening rather than "I'm misconfigured". That's the entire bug per the issue's triage comment.Fix:
station = "900192001", with a comment naming the station in words right next to the id so the opaque numeric id and the human place name can't silently drift apart again.lines/directionsdefaults — decisionI went with Annika's lean from the issue: ship the default with both axes empty/commented out, not populated.
Reasoning: a populated filter that's wrong (as here) fails invisibly — the widget just shows nothing, indistinguishable from an actually quiet station, exactly how #641 shipped for however long. An empty filter (per
feed.rs:94-97, empty = allow all) fails visibly and usefully instead — a new user sees every suburban line through the station immediately, confirms the widget works, and only then opts into narrowing it. The previous["S8", "S85", "S9"]/["Spandau", "Birkenwerder", "Hohen Neuendorf", "Waidmannslust"]values are kept as commented-out template lines with a note on what each axis does, so the shape is still discoverable.Tests
default_config_parses(was pinning the wrongstationliteral, and the oldlinesvalue) now expects900192001and emptylines/directions.place_transition_silent_on_first_resolution/place_transition_fires_on_genuine_name_changeused"900180001"as an arbitrary fixture id for an unrelated"Home"test place (nothing to do with Schöneweide) — swapped for an obviously-synthetic id (900000001) so no test in the file keeps the buggy literal lying around to confuse a future grep.default_station_id_matches_its_place_name: ties the default place'sname("Schöneweide") to itsstationid (900192001), with an assertion failure message that spells out the real-world station names (900192001= S Schöneweide Bhf,900180001= S Köpenick/Parrisiusstr., Departures broken #641) so a future editor who changes this literal sees exactly what they're changing and why the old value was a trap. A bareassert_eq!against a self-chosen literal — which is exactly what let this ship — can't catch this class of bug; pinning the id/name relationship can.Falsification (per the issue's ask)
Reverted
stationback to"900180001"(only that literal, inDEFAULT_CONFIG) and rerancargo test -p hytte-services places::tests:Then restored the fix and confirmed the file is byte-identical to before the revert (
diffclean), and reran the full suite green:cargo test -p hytte-services→ 672 passed; 0 failed.Gates run
unset RUSTC_WRAPPER && cargo clippy -p hytte-services --all-targets --features system-tests -- -D warnings— clean.cargo test -p hytte-services— 672 passed, 0 failed.nix fmttwice — 0 files changed both times.What this does NOT fix
Existing installs are unaffected.
write_default_config(places.rs:329) only writesplaces.tomlwhen the file is absent, so this corrects the default for new installs only. If you already have a~/.config/trollshell/places.tomlwith the old900180001/S8,S85,S9values (as Annika does), this PR does nothing for it — you need to hand-edit that file yourself (changestationto900192001, and/or drop thelines/directionslines to see everything). This repo intentionally was not touched outsidecrates/hytte-services/src/places.rs; nothing undercrates/hytte-plugin-departures/changed (its fetch/filter logic was already correct — this was purely a wrong-constant bug in the shipped config), and no config files outside the repo were touched.