Make Hyundai Europe location parsing robust#33
Open
dz0ny wants to merge 1 commit into
Open
Conversation
parseLocation chose the /location/park branch purely by date, so when the park endpoint returns no coordinates (no recent park event) and the status response carries no embedded vehicleLocation, the location collapsed to (0, 0) and disappeared from the UI. - parseLocation now reads both candidate sources (park endpoint and the status-embedded location) and falls back to whichever actually carries coordinates, only preferring park when it is non-empty and at least as recent as the status location. - Add VehicleStatus.Location.hasCoordinates, treating (0, 0) null island as "no fix". - Fix a force-unwrap in getDoubleFromJson (Double(value)!) that would crash the entire status parse on any non-numeric coordinate string.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR improves how vehicle coordinates are represented and selected when parsing Hyundai Europe API responses, reducing missing/stale location results and avoiding a potential crash when parsing numeric strings.
Changes:
- Added
VehicleStatus.Location.hasCoordinatesto detect the API’s(0,0)“no fix” sentinel. - Updated location parsing to choose between park endpoint vs. status-embedded location based on recency and coordinate presence.
- Made string-to-double parsing non-throwing to prevent runtime crashes.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| Sources/BetterBlueKit/Models/VehicleStatus.swift | Adds a helper for determining whether a location contains real coordinates vs. (0,0) sentinel. |
| Sources/BetterBlueKit/API/HyundaiEurope/HyundaiEuropeAPIClient+Parsing.swift | Refines location source selection logic and makes numeric parsing from strings safer. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+281
to
+290
| let parkIsNewer = (parkDate != nil && locationDate != nil) | ||
| ? locationDate!.compare(parkDate!).rawValue <= 0 | ||
| : parkLocation.hasCoordinates | ||
| if parkIsNewer, parkLocation.hasCoordinates { | ||
| return parkLocation | ||
| } | ||
| if statusLocation.hasCoordinates { | ||
| return statusLocation | ||
| } | ||
| return parkLocation.hasCoordinates ? parkLocation : statusLocation |
| // whichever source actually carries coordinates so a missing park | ||
| // event no longer drops the location entirely. | ||
| let parkIsNewer = (parkDate != nil && locationDate != nil) | ||
| ? locationDate!.compare(parkDate!).rawValue <= 0 |
schmidtwmark
requested changes
May 26, 2026
| // as recent as the (often stale) status location. Fall back to | ||
| // whichever source actually carries coordinates so a missing park | ||
| // event no longer drops the location entirely. | ||
| let parkIsNewer = (parkDate != nil && locationDate != nil) |
Owner
There was a problem hiding this comment.
I'd much rather use guard statements instead of != nil and then a force unwrapping.
Owner
|
This looks reasonable enough to me -- I had a comment on some of the parsing logic. I'd also like confirmation that this was run through |
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.
Problem
On Hyundai Europe vehicles, status syncs return everything (battery, lock, climate, odometer, tyres) except the car location, which shows as missing.
Root cause
parseLocationchose between the two location sources purely by timestamp:…/location/park(a cached park snapshot)vehicleLocation)It returned the
/location/parkbranch whenever its date was newer — even when that endpoint returned no coordinates (the car has had no recent park event). Combined with a status body that carries no embeddedvehicleLocation, the location collapsed to(0, 0)and disappeared. There was no fallback to the other source.For reference,
hyundai_kia_connect_apionly uses/location/parkwhen it actually contains coordinates (if … get_child_value(location, "coord.lat")) and otherwise keeps the live location.Changes
parseLocationreads both candidate sources and falls back to whichever actually carries coordinates, only preferring/location/parkwhen it is non-empty and at least as recent as the status location.VehicleStatus.Location.hasCoordinates— new helper treating(0, 0)(null island) as "no fix".getDoubleFromJson— fixed a force-unwrap (Double(value)!) that would crash the entire status parse on any non-numeric coordinate string.Testing
swift buildpasses.hyundai_kia_connect_api.Note
This addresses the common "no recent park event" case where one source is empty. If a vehicle returns coordinates from neither source, a dedicated
GET …/locationcall (as the reference makes) would be the follow-up — happy to add it if logs show that shape.