search

soundscape-community / soundscape

An iOS application/service that aids navigation through spatialized audio
https://soundscape.services
MIT License
21 stars 22 forks source link

Replace `iOS-GPX-Framework` with `CoreGPX` #65

Closed 2kai2kai2 closed 1 year ago

2kai2kai2 commented 1 year ago

Resolves #64

Changes

Essentially, this eliminates the need for cocoapods and cocoapods-patch We do the following:

  1. Replace iOS-GPX-Framework (Obj-C, cocoapods) with CoreGPX (Swift, Swift Package Manager)
  2. Replace custom extensions implementation (Obj-C) in apps/ios/patches/iOS-GPX-Framework+0.0.2.diff with implementation (Swift) in apps/ios/GuideDogs/Code/App/Framework Extensions/Geo Extensions/GPXExtensions.swift (the previous version used a patch on the Obj-C extension, which also made it a more complex dependency system)
  3. Remove all remaining references to cocoapods and the Pods-soundscape build

In the new implementation of our custom GPX extensions, I've implemented them as views rather than intercepting and adding custom parsing for GPX elements. This system may be slightly less efficient due to accessing the extension's GPX tree, but this should not be a major issue since our main use case (loading AuthoredActivityContent) can discard the GPX object after loading. Additionally, it has the advantage of simplifying and reducing the surface area with the dependency.

Testing

~This pull request excludes the automated testing which can be seen here: https://github.com/2kai2kai2/soundscape Said testing (more so the build systems) does not seem to be stable with the previous legacy dependencies.~

~I intend to open a second, separate pull request for the full testing system after this, but can combine them if that is preferred.~

EDIT: XCTest is now included in this pull request.

2kai2kai2 commented 1 year ago

Just overwrote with the full changes including tests. There are a copy of these tests running here: https://github.com/2kai2kai2/soundscape/actions/runs/6632231863/job/18017464213

It looks like the tests did not pass due to the AudioEngine test. Not sure why, but overall the fact they are running is good news.

steinbro commented 1 year ago

Once the tests are passing, your expertise would be helpful in getting Fastlane working again (#58). Probably for a separate pull request.

2kai2kai2 commented 1 year ago

Test issue seems to be due to the lack of installed TTS voices which are used in testing. Nevertheless, it has been partially resolved by the inclusion of updated service URLs in main.

I believe this pull request is ready to merge.

steinbro commented 1 year ago

Thanks! Amazing work. Do you have any idea what's going on with the TTS voices in the simulator? I noticed this even before your PR -- I also don't seem to have any iOS voices installed in my simulator, and the Settings app is crashing immediately after launch. Is this a bug with the new iOS 17 simulator?