-
Notifications
You must be signed in to change notification settings - Fork 314
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
Global app state manipulation #1474
Comments
Hello @simonseyer and thank you very much for opening this issue. Your feedback is greatly appreciated! Regarding the issues you raised, we are aware and in agreement regarding both the issue of manipulating global shared state as well as the location manager coupling. Note that the latter we have also been discussing in our Maps SDK (issue) (PR) -- your feedback on this approach would be most welcome as to whether something similar in our Navigation SDK would meet your needs. Regarding the manipulation of |
Note that battery monitoring is currently used not only for setting CLLocationManager options, but also for throttling NavigationMapView’s frame rate when the device isn’t plugged in. |
Thanks for the immediate reply. I’m happy that you are sharing my opinion about this topic. I can only look into the mentioned ticket/pr on Monday but will be happy to give feedback then. |
I reviewed the PR and it goes in the right direction from my point of view. However, I would just suggest getting rid of the |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Stale. |
Mapbox Navigation SDK version: Master branch
In the
RouteController.init
the battery monitoring is enabled (UIDevice.current.isBatteryMonitoringEnabled = true
) and in thedeinit
it is disabled again (UIDevice.current.isBatteryMonitoringEnabled = false
).This manipulation of a global app state is breaking features of us that rely on battery monitoring. This at least has to be configurable. Even better would be if the configuration would be delegated to the integrating app (
delegate.routeController(_ controller, requiresBatteryMonitoring: true/false)
).I only read a subset of your code yet, but I also recognized a hard dependency to the
CLLocationManger
and its types. This also causes some inconveniences for us because our location is coming from different sources. My suggestion would be to have a protocol which defines just the requirements you have (a location with lat/long & accuracy and true heading I guess) and provide a default implementation with aCLLocationManager
.In general it would be best (from my point of view) to not rely on any system library and to delegate all the required informations/actions to the app code while still providing default implemenations. This would provide a lot of flexibility (that we more or less need to integrate your SDK into our rather complex app) while still keeping the "works out of the box" experience.
The text was updated successfully, but these errors were encountered: