Update Mapbox Maps SDK dependency to v10.0.0-beta.19.1.
#2983
Conversation
MaximAlien
changed the title
MaximAlien
changed the title
v10.0.0-beta.19.
# Conflicts: # Sources/MapboxNavigation/ArrivalController.swift # Sources/MapboxNavigation/CameraController.swift # Sources/MapboxNavigation/NavigationMapView.swift
# Conflicts: # Sources/MapboxNavigation/OrnamentsController.swift # Tests/CocoaPodsTest/PodInstall/Podfile.lock
MaximAlien
changed the title
v10.0.0-beta.19.v10.0.0-beta.19.1.
# Conflicts: # Sources/MapboxNavigation/NavigationMapView.swift
There was a problem hiding this comment.
My only concern is how we’re dealing with runtime styling errors. There wouldn’t be any harm from landing the PR as is, since previous map SDK releases would’ve given us no opportunity to react to these failures anyways. But log statements in general are a pretty weak error handling mechanism, given all the console spew that the SDK and its dependencies already put out.
| navigationMapView.mapView.update { | ||
| $0.location.puckType = .puck2D() | ||
| } |
There was a problem hiding this comment.
| layer.paint?.lineColor = .constant(.init(color: styledFeature.color)) | ||
| try navigationMapView.mapView.style.addLayer(layer) | ||
| } catch { | ||
| NSLog("Failed to perform operation with error: \(error.localizedDescription).") |
There was a problem hiding this comment.
There are log statements in response to runtime styling errors throughout this PR. Is this is a best practice, or should we do something more aggressive like asserting or rethrowing? How likely are these errors?
There was a problem hiding this comment.
In the past I've been frustrated by how fatal the Maps SDK treats things like adding a duplicate style identifier, etc. so I would have liked to see this kind of non-fatal error logging.
That said, it might be useful to make these errors more fatal at least for this period when the Maps SDK v10 is still in beta. it might help us to catch issues in the SDK that might otherwise slip by unnoticed.
There was a problem hiding this comment.
I was thinking about keeping those log statements to see if there are any issues during development process in runtime styling, when our styling logic is more stable we can probably remove it. Also as @avi-c found we currently have some errors already, so it'd be good to create a ticket to tackle those.
| if let expressionData = expressionString.data(using: .utf8), | ||
| let expJSONObject = try? JSONSerialization.jsonObject(with: expressionData, options: []) { | ||
| mapView.mapboxMap.__map.setStyleLayerPropertyForLayerId(routeDurationAnnotationsLayerIdentifier, |
There was a problem hiding this comment.
I verified that the Route Duration feature still works, so from that perspective it looks good, however, I am seeing a bunch of errors/warnings logged to the console if I run the Example app and long press to go into route preview mode. They have this form:
2021-05-12 10:17:04.942581-0700 Example[84268:5959937] Failed to perform operation on MapboxNavigation-MapboxNavigation-resourcesbuildingExtrusionLayer with error: Layer MapboxNavigation-MapboxNavigation-resourcesbuildingExtrusionLayerdoes not exist.
2021-05-12 10:17:05.098531-0700 Example[84268:5959937] Failed to remove layer MapboxNavigation-MapboxNavigation-resources_waypointCircleLayer with error: Layer MapboxNavigation-MapboxNavigation-resources_waypointCircleLayerdoes not exist.
2021-05-12 10:17:05.098779-0700 Example[84268:5959937] Failed to remove layer MapboxNavigation-MapboxNavigation-resources_waypointSymbolLayer with error: Layer MapboxNavigation-MapboxNavigation-resources_waypointSymbolLayerdoes not exist.
2021-05-12 10:17:05.098981-0700 Example[84268:5959937] Failed to remove source MapboxNavigation-MapboxNavigation-resources_waypointSource with error: Source MapboxNavigation-MapboxNavigation-resources_waypointSource is in use, cannot remove.
2021-05-12 10:17:05.111996-0700 Example[84268:5959937] Failed to add route casing source 0x000060000388fa00.main.source_casing with error: Source 0x000060000388fa00.main.source_casing already exists.
2021-05-12 10:17:05.128682-0700 Example[84268:5959937] Failed to remove layer MapboxNavigation-MapboxNavigation-resources_routeDurationAnnotationsLayer with error: Layer MapboxNavigation-MapboxNavigation-resources_routeDurationAnnotationsLayerdoes not exist.
2021-05-12 10:17:05.128877-0700 Example[84268:5959937] Failed to remove source MapboxNavigation-MapboxNavigation-resources_routeDurationAnnotationsSource with error: Source MapboxNavigation-MapboxNavigation-resources_routeDurationAnnotationsSource is in use, cannot remove.
Thanks Avi. Warnings/errors will be tackled in separate PR. |
I agree that logging errors doesn't give end user a chance to react to possible errors. At the same time it's a bit better than silently hiding thrown exceptions. Not sure if re-throwing will help either, functionality like labelling current road feature, adding route line etc is pretty self-contained. |
Description
Updated Mapbox Maps SDK dependency to
v10.0.0-beta.19.1.Also fixes:
logoandattributionButtonon CarPlay. #2992