location: add missing subscribe fan-out for location events#2914
Open
FormalSnake wants to merge 1 commit into
Open
location: add missing subscribe fan-out for location events#2914FormalSnake wants to merge 1 commit into
FormalSnake wants to merge 1 commit into
Conversation
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.
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.
The generic
subscribehandler incore/internal/server/server.gofans out every service except location. There is noshouldSubscribe("location")block, so a subscribe-all connection never receives location events, and an explicitlocationsubscription is silently ignored (unknown service names don't error). Theservice === "location"branch in DMSService.qml's event dispatch is dead code today, and theensureSubscription()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.getStatepull right after connecting, roughly 3s after core start. The core can't have a GeoClue2 fix that early.NewClientseeds 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:
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 listunsticks 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 vetandgo 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.