Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v2.0.0-beta.17 #3156

Merged
merged 5 commits into from Jul 9, 2021
Merged

v2.0.0-beta.17 #3156

merged 5 commits into from Jul 9, 2021

Conversation

1ec5
Copy link
Contributor

@1ec5 1ec5 commented Jul 8, 2021

Updated the Xcode project, CocoaPods podspec, and readmes for v2.0.0-beta.17.

In addition to the usual updates, this PR upgrades to MapboxNavigationNative v56.0.0, which includes the following changes versus previous v2.0.0 betas:

Additionally, I took the opportunity to refactor OpenLRStandard, with a view toward future enhancements to the road objects/incidents API:

  • Replaced the OpenLRStandard enumeration with an OpenLRIdentifier enumeration that includes a RoadObjectIdentifier as an associated value. RoadObjectMatcher.matchOpenLR(location:standard:identifier:) has been renamed to RoadObjectMatcher.matchOpenLR(location:identifier:). (v2.0.0-beta.17 #3156)

And a typo in the jazzy table of contents was corrected.

/cc @mapbox/navigation-ios

@1ec5 1ec5 added build Issues related to builds and dependency management. op-ex Refactoring, Tech Debt or any other operational excellence work. labels Jul 8, 2021
@1ec5 1ec5 added this to the v2.0.0 (GA) milestone Jul 8, 2021
@1ec5 1ec5 requested a review from a team July 8, 2021 22:23
@1ec5 1ec5 self-assigned this Jul 8, 2021
if let path = path {
completionHandler(URL(fileURLWithPath: path))
} else {
completionHandler(nil)
}
self?.historyRecorder?.startRecording()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the HistoryRecorderHandle.stopRecording(result:) documentation, we have to explicitly restart history recording after it’s done writing out history, a change from before when dumpHistory(result:) would keep history recording going.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, this change seems to be causing various test failures:

MapboxCoreNavigationTests.PassiveLocationManagerTests
testHistoryRecording, XCTAssertNotNil failed
/Users/distiller/project/Tests/MapboxCoreNavigationTests/PassiveLocationManagerTests.swift:124

      PassiveLocationManager.writeHistory { url in
          XCTAssertNotNil(url)
          historyExpectation.fulfill()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#3157 moves this startRecording() call outside the callback to happen synchronously with stopRecording(result:), as recommended by @DmitryAk.

@@ -142,6 +143,7 @@ class Navigator {
navigator.setElectronicHorizonObserverFor(self)
navigator.addObserver(for: self)
navigator.setFallbackVersionsObserverFor(self)
historyRecorder?.startRecording()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The relevant change in MapboxNavigationNative clearly expects the history recorder to be started explicitly at some point before dumping (or “stopping”) history, so I’ve added a bit here to start recording basically as soon as possible. In principle, we could start recording more lazily, perhaps based on some application-defined behavior, but we don’t have a convenient place to do that, so for now I’m just preserving the previous behavior.

/cc @mskurydin @DmitryAk

@1ec5 1ec5 merged commit 2584e6b into main Jul 9, 2021
@1ec5 1ec5 deleted the 1ec5-v2.0.0-beta.17 branch July 9, 2021 00:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues related to builds and dependency management. op-ex Refactoring, Tech Debt or any other operational excellence work.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants