Skip to content

location: add missing subscribe fan-out for location events#2914

Open
FormalSnake wants to merge 1 commit into
AvengeMedia:masterfrom
FormalSnake:fix/subscribe-location-events
Open

location: add missing subscribe fan-out for location events#2914
FormalSnake wants to merge 1 commit into
AvengeMedia:masterfrom
FormalSnake:fix/subscribe-location-events

Conversation

@FormalSnake

@FormalSnake FormalSnake commented Jul 23, 2026

Copy link
Copy Markdown

The generic subscribe handler in core/internal/server/server.go fans out every service except location. There is no shouldSubscribe("location") block, so a subscribe-all connection never receives location events, and an explicit location subscription is silently ignored (unknown service names don't error). The service === "location" branch in DMSService.qml's event dispatch is dead code today, and the ensureSubscription() added on the QML side in fe561f2 subscribes to a stream the server never produces.

That leaves the shell with exactly one shot at coordinates: the location.getState pull right after connecting, roughly 3s after core start. The core can't have a GeoClue2 fix that early. NewClient seeds an IP-derived location, and the real fix only lands after the geoclue client's 10s TimeThreshold, so the pull loses the race every time and there is no second chance afterwards.

For me the failure is very visible: I'm on Lanzarote in the Canary Islands, and my ISP's egress geolocates to Granada on the mainland, over 1000km away. The weather widget shows Granada for the whole session, every session, even though the core itself converges to the correct WiFi-based position as soon as GeoClue delivers a fix — about 15s in when fresh scan data is available (easy to verify: echo '{"id":1,"method":"location.getState"}' | nc -U $XDG_RUNTIME_DIR/danklinux-*.sock).

Repro:

  1. run on a machine where IP geolocation and GeoClue2 disagree (any laptop behind a CGNAT ISP, basically)
  2. restart dms.service and watch the journal print "GeoClue2 has no fix yet, seeding with IP location"
  3. query the socket as above: correct coordinates within ~15s
  4. the weather widget never leaves the IP city

Fix: wire location into the fan-out exactly like loginctl/sysupdate, initial state on subscribe, then pushes from the location manager. No QML changes needed, the subscription logic from fe561f2 starts working as written.

One caveat found while testing, so nobody mistakes it for this PR not working: the core itself can stall on the IP seed when GeoClue has nothing to work with. If the machine has been associated to the same AP for a while, wpa_supplicant's BSS cache can be empty (NetworkManager doesn't background-scan while comfortably associated), so GeoClue fails its one query with "Failed to query location: No WiFi networks found" and doesn't retry until something else triggers a scan (even a manual nmcli dev wifi list unsticks it). I've watched the core keep the IP location for over an hour that way. That's a location-manager/GeoClue2 interaction, independent of the subscribe fan-out — probably worth a follow-up (nudge a rescan or re-create the geoclue client while still sitting on the IP seed), but out of scope here. Once GeoClue does deliver, this PR is what makes the shell actually follow it.

go build, go vet and go test ./internal/server/ pass. Tested on two NixOS machines that both showed Granada: with the patched core the widget corrects itself as soon as the geoclue fix lands, and it now follows location changes mid-session too.

handleSubscribe forwards every service except location, so neither
subscribe-all connections nor explicit location subscriptions ever
receive location events, and the shell's only source of coordinates
is a single getState pull that races the geoclue TimeThreshold and
loses. Wire location into the fan-out like loginctl/sysupdate:
initial state on subscribe, then manager pushes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant