search

StanfordBDHG / ResearchKit

ResearchKit with Swift Package Manager (SPM), SwiftUI, C++ Interoperability, and visionOS support
https://swiftpackageindex.com/StanfordBDHG/ResearchKit/documentation/researchkitswiftui
Other
7 stars 3 forks source link

🐛 Bug report: App crashes on iPad due to missing SensorKit #17

Closed luppoalberto111 closed 8 months ago

luppoalberto111 commented 8 months ago

Description

When running the app with this framework, the app instantly crashes on iPad due to missing reference to SensorKit. This issue is also present in the demo app included in this repository. I am also attaching the crash-log:

dyld[1576]: Library not loaded: '/System/Library/Frameworks/SensorKit.framework/SensorKit'
  Referenced from: '/private/var/containers/Bundle/Application/.../Frameworks/ResearchKit.framework/ResearchKit'
  Reason: tried: '/System/Library/Frameworks/SensorKit.framework/SensorKit' (no such file)

Reproduction

Running the demo app on a physical iPad makes this issue easily reproducible. This issue is not present when running in the simulator. A physical device is required.

Expected behavior

This framework should work also in apps that support iPads and not crash instantly. Running without crashing is therefore the expected behaviour.

Additional context

No response

Code of Conduct

Supereg commented 8 months ago

Thanks for the bug report 🚀 Just a quick upfront question, what iPadOS version and iPad model are you running?

zinn-claus commented 8 months ago

I suffer from the same bug, for instance, on an iPad (9th generation) on iPadOS-Version 17.2

philippzagar commented 8 months ago

Just tried it locally on my iPad and can confirm that the crash only occurs on the physical iPad (iPad Pro 12.9, 4th Gen, iPadOS 17.3), not the simulator. No issues on the physical iPhone.

Steps I already tried:

Sadly, these steps didn't seem to make a difference. The app still crashes on startup with the error message outlined above.

@PSchmiedmayer @Supereg @vishnuravi Any input from your side? To be honest, I'm out of ideas. Might that be somehow related to Entitlement issues?

PSchmiedmayer commented 8 months ago

@luppoalberto111 @zinn-claus Thank you for sharing the issue with us and taking the time to investigate it together with us! It's great to see people use our framework, including ResearchKitOnFHIR!

@philippzagar Even though I can't find it in the platform availability, I think that I remember that SensorKit is not available on iPad. The tagline of the docs is "Retrieve data and derived metrics from sensors on an iPhone or paired Apple Watch." [SensorKit - Apple Documentation]; nevertheless the platform tags note support for iPadOS.

The key issue there is most likely stemming from our beloved friend ResearchKit: https://github.com/StanfordBDHG/ResearchKit/blob/66f2fca769dc103de5801bb491b1d8ceafcd281e/ResearchKit/Common/ORKSensorPermissionType.m#L34.

As none of our applications nor any functionality in ResearchKitOnFHIR is using SensorKit so, I would assume it might be worth a try to remove the SensorKit-related functionality from ResearchKit in our fork and see if that allows one to load RK on an iPad. @Supereg, this will also be important for our upcoming iPad project.

In the future: With ResearchKit 3.0, we will have a bit of an easier time maintaining this as the Active Tasks will move out to a separate target but for now it might just be worth manually removing the code within the RK fork?

zinn-claus commented 8 months ago

Thanks for the quick response! I also do not need SensorKit functionality in my apps. Therefore, I am happy with the idea of removing SensorKit in your fork.

PSchmiedmayer commented 8 months ago

Sounds great; IMO that would be a good path to go.

Would you feel comfortable creating a PR to our ResearchKit fork that removes the SensorKit import and related types from the codebase? Let us know if you need any help or support and/or if you don't feel that this is something you would like to address yourself 👍

zinn-claus commented 8 months ago

Hmm, I don't have much experience with the intricacies of the RK codebase, so I am feeling a little uncomfortable about it...

philippzagar commented 8 months ago

@PSchmiedmayer @zinn-claus From what I'm seeing, SensorKit should be available on iPadOS 14 and up (as you noted): https://developer.apple.com/documentation/sensorkit The note on the page explicitly states: This framework ignores calls from Mac apps that you build with Mac Catalyst, and from compatible iPad and iPhone apps running in visionOS. -> No mention of iPadOS incompatibility

Also, if querying the issues of the ResearchKit repo, it seems like ReserachKit is useable on iPad: https://github.com/ResearchKit/ResearchKit/issues?q=ipad In addition, the application is able to launch on iPad simulators and we're able to build it for the iPadOS destinations.

There might be some inconsistency here from Apple's side but I feel like SensorKit should be supported on the iPad as well.

Of course, as @PSchmiedmayer mentioned, one could just remove the SensorKit-related code from RK, but as far as I'm seeing, we should be able to use SensorKit on the iPad as well

luppoalberto111 commented 8 months ago

I totally agree with @philippzagar. ResearchKit also includes a demo app in it's repository and that one works perfectly on iPad. Hence, I'd presume that there's some issue in ResearchKitOnFHIR.

And if the ResearchKit app works fine on iPad, we should avoid ripping out the SensorKit from it.

At this point we might not be aware how deep the "rabbit hole" might be and if some types are shared, complete separation might be very difficult if not impossible and could produce even more nasty bugs. Also future update of ResearchKit can become even more complex.

@philippzagar As you've stated, this issue manifests differently on a physical device compared to running it on a mac. I think that the reason behind it is the fact, that we use simulators as oppose to android which uses emulation for its devices. And this small difference could be the reason, why the linking is executed differently in simulation.

luppoalberto111 commented 8 months ago

Guys, so I made a small observation. I took the original ResearchKit and the the fork. Both still have the demo application called ORKCatalog. As a result, this application does work in the original (Apple) repository but does not on the fork. That being said, the issue is in ResearchKit by Spezi rather than in ResearchKitOnFHIR.

So IMHO further discussion and solution can be handled there.

Of course it's up to @PSchmiedmayer to decide. Do you think that we should re-open this issue in ResearchKit and close this one?

Supereg commented 8 months ago

Moving this issue to the ResearchKit repository as this is where the underlying issue is present.

@luppoalberto111 is right that this issue is not present in the ResearchKit upstream and is something that slipped in with our PR.

I experimented a bit with our fork and could replicate the issue. However, the issue was not present in our fork previously. My current suspicion is that it is caused by the latest changes in #16 where we bump the minimum deployment target from iOS 13 to iOS 15. I think that is what causes the issue to show up. The underlying issue is probably related around SensorKit which is marked to be available on iPad from iOS 14 onwards. So makes sense that it didn't show up before as the due to the minimum deployment target of iOS 13, the library didn't try to load the SensorKit framework on iPad.

We will need to look what the actual issue is though. Assuming Apple documentation is correct that SensorKit is available on iPad, there shouldn't be an issue. Otherwise, if SensorKit is marked as available by mistake we need to reconsider what to do with our current minimum deployment target.

Supereg commented 8 months ago

Replicated it again: version 2.2.23 breaks while 2.2.22 works of our https://github.com/StanfordBDHG/ResearchKit package.

Supereg commented 8 months ago

tldr: Apple is wrong.

SensorKit specifies that it is available on iPadOS 14+. Also their documentation hint This framework ignores calls from Mac apps that you build with Mac Catalyst, and from compatible iPad and iPhone apps running in visionOS. is a bit misleading or weirdly phrased. Because you can have a iPadOS app running on visionOS interacting with SensorKit (doing nothing) but the same app cannot run on iPadOS as the framework is not present.

I checked ISPW files for iPadOS, iOS, visionOS and my local Mac installation.

The reason why ResearchKit didn't previously crash was because of its minimum deployment target being lower than when SensorKit was introduced.

PSchmiedmayer commented 8 months ago

Thank you @Supereg for the investigation and helping us to resolve this issue! 🚀