search

clariusdev / mobileapi

Android API Allowing IPC with the Clarius App
https://www.clarius.com
BSD 3-Clause "New" or "Revised" License
12 stars 4 forks source link

MobileAPI breaks when upgrading from 9.3.3 to 10 #28

Closed mischkew closed 1 year ago

mischkew commented 1 year ago

Dear all, please find our problem description regarding backwards compatibility below:

Problem

The core problem is that some users will be on v9.3.3 while some will have already upgraded to v9.4.0 or v10. This means we as ThinkSono need to create a single app that supports both v9.3.3 and v10, as we cannot control which version the user has installed.

With MobileAPI 9.4.0, the mobileapi service and its definitions were moved from me.clarius.mobileapi to me.clarius.sdk.mobileapi.

I tried using the old MobileAPI definitions with the new service, by binding to me.clarius.sdk.mobileapi.MobileApiService instead of me.clarius.mobileapi.MobileApiService. That of course means I am still using the old class definitions, as our code needs to be compatible with the old service.

The service binds successfully, but when I send messages to the service, the clarius app crashes with this exception:

2022-11-10 16:01:10.803 26150-26150/? E/AndroidRuntime: FATAL EXCEPTION: main
    Process: me.clarius.clarius, PID: 26150
    android.os.BadParcelableException: ClassNotFoundException when unmarshalling: me.clarius.mobileapi.SettingsInfo
        at android.os.Parcel.readParcelableCreator(Parcel.java:3428)
        -- snip--

This is no surprise, as the Clarius app uses me.clarius.sdk.mobileapi.SettingsInfo while our app uses me.clarius.mobileapi.SettingsInfo.

Possible Solutions

  1. Clarius v10 stays backwards compatible with 9.3.3.
    • Requires a mapping of the class names from old to new package location
  2. ThinkSono builds a hacky way to support both versions
    • We detect which version of clarius is installed
    • We switch between implementations based on the version
    • Not 100% sure if this is technically possbile
    • Could lead to unexpected bugs and crashes
  3. Clarius updates first, then ThinkSono app
    • We update our code to work with v10 internally, and hold back the changes
    • Once Clarius v10 is rolled out, we push an update ourselves
    • If our app detects clarius 9.3.3, we prompt the user to update via the playstore
    • This requires a full rollout on Clarius' side, a partial rollout will leave some users unable to use our app

I would like to avoid option 3 as this make it might lead to a similar situation in the future where an app update on the Clarius side breaks for all existing ThinkSono users.

I would appreciate any input from your side. Happy to jump on a video call as well if that makes discussing design decisions easier.

Many thanks! Sven & Team

julien-l commented 1 year ago

Hi my apologies for the late answer. The SettingsInfo class was removed in Clarius App version 10.1.0 hence the exception. The motivation was to reduce the scope of Mobile API and encourage users to move to Cast or Solum SDKs.

After testing, the package name change does not cause any issue--the example application using me.clarius.sdk.mobileapi works properly with older Clarius App 9.3.3 that uses me.clarius.mobileapi.

I will update the library packages soon with an updated change log file.

mischkew commented 1 year ago

Hi Julien,

I apologise upfront for this lengthly wall of text. I am trying to comment on all your points and also share my perspective of the scope of the problem.

After testing, the package name change does not cause any issue--the example application using me.clarius.sdk.mobileapi works properly with older Clarius App 9.3.3 that uses me.clarius.mobileapi.

We can't reproduce this on our side.

Version 10.1 is not available on the gradle repository. We used the implementation version given in this PR: https://github.com/clariusdev/mobileapi/pull/26/files

Execution failed for task ':app:mergeDemoDebugNativeLibs'.
> Could not resolve all files for configuration ':app:demoDebugRuntimeClasspath'.
   > Could not find me.clarius.sdk:mobileapi:10.1.0.
     Searched in the following locations:
       - https://dl.google.com/dl/android/maven2/me/clarius/sdk/mobileapi/10.1.0/mobileapi-10.1.0.pom
       - https://repo.maven.apache.org/maven2/me/clarius/sdk/mobileapi/10.1.0/mobileapi-10.1.0.pom
       - https://jitpack.io/me/clarius/sdk/mobileapi/10.1.0/mobileapi-10.1.0.pom
       - https://maven.pkg.github.com/clariusdev/mobileapi/me/clarius/sdk/mobileapi/10.1.0/mobileapi-10.1.0.pom
     Required by:
         project :app

Thus, we had to use v9.4.0. First, we have to change the service name from me.clarius.mobileapi.MobileApiService to me.clarius.sdk.mobileapi.MobileApiService, otherwise binding the service fails completely.

When using the old binding (would require effort on our part to detect which one is available, see option 2 in my previous post), the mobile service throws this exception after binding the service (see snippet)

2022-12-02 10:47:07.544 31256-31256/? E/Parcel: Class not found when unmarshalling: me.clarius.sdk.mobileapi.SettingsInfo
    java.lang.ClassNotFoundException: me.clarius.sdk.mobileapi.SettingsInfo
        at java.lang.Class.classForName(Native Method)
        at java.lang.Class.forName(Class.java:454)
        at android.os.Parcel.readParcelableCreator(Parcel.java:3402)
        at android.os.Parcel.readParcelable(Parcel.java:3336)
        at android.os.Parcel.readValue(Parcel.java:3238)
        at android.os.Parcel.readArrayMapInternal(Parcel.java:3635)
        at android.os.BaseBundle.initializeFromParcelLocked(BaseBundle.java:292)
        at android.os.BaseBundle.unparcel(BaseBundle.java:236)
        at android.os.Bundle.getParcelable(Bundle.java:1002)
        at me.clarius.mobileapi.MobileApiService.getCustomParcelable(MobileApiService.java:723)
        at me.clarius.mobileapi.MobileApiService.access$100(MobileApiService.java:26)
        at me.clarius.mobileapi.MobileApiService$IncomingHandler.handleMessage(MobileApiService.java:304)
        at android.os.Handler.dispatchMessage(Handler.java:102)
        at android.os.Looper.loopOnce(Looper.java:226)
        at android.os.Looper.loop(Looper.java:313)
        at android.app.ActivityThread.main(ActivityThread.java:8582)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:563)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1133)
     Caused by: java.lang.ClassNotFoundException: me.clarius.sdk.mobileapi.SettingsInfo
        at java.lang.Class.classForName(Native Method) 
        at java.lang.Class.forName(Class.java:454) 
        at android.os.Parcel.readParcelableCreator(Parcel.java:3402) 
        at android.os.Parcel.readParcelable(Parcel.java:3336) 
        at android.os.Parcel.readValue(Parcel.java:3238) 
        at android.os.Parcel.readArrayMapInternal(Parcel.java:3635) 
        at android.os.BaseBundle.initializeFromParcelLocked(BaseBundle.java:292) 
        at android.os.BaseBundle.unparcel(BaseBundle.java:236) 
        at android.os.Bundle.getParcelable(Bundle.java:1002) 
        at me.clarius.mobileapi.MobileApiService.getCustomParcelable(MobileApiService.java:723) 
        at me.clarius.mobileapi.MobileApiService.access$100(MobileApiService.java:26) 
        at me.clarius.mobileapi.MobileApiService$IncomingHandler.handleMessage(MobileApiService.java:304) 
        at android.os.Handler.dispatchMessage(Handler.java:102) 
        at android.os.Looper.loopOnce(Looper.java:226) 
        at android.os.Looper.loop(Looper.java:313) 
        at android.app.ActivityThread.main(ActivityThread.java:8582) 
        at java.lang.reflect.Method.invoke(Native Method) 
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:563) 
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1133)

The SettingsInfo class was removed in Clarius App version 10.1.0 hence the exception.

API version v10.1.0 does not exist in this repository. I only found an unmerged branch. The only version available (as well as the API reference linked in the readme) points to v9.4.0. The SettingsInfo is still present there.

In the past, you had the SDK code here in the repository. That at least allowed us to inspect the code and figure out ourselves which functionality is present / did change. This process is now less transparent as I can only rely on the API reference.

The motivation was to reduce the scope of Mobile API and encourage users to move to Cast or Solum SDKs.

I understand your standpoint that you want to encourage usage of the new SDKs. But removing functionality makes it difficult for us to deliver a consistent user experience and in the worst case leads to the app not functioning with existing customers (as it happened yesterday / still happening today).

This time we have been able to ship a hotfix to our clinical trial sites. Removed functionality could make it impossible in the future to even ship a hotfix. I expected that you don't add new functionality due to the move to the new SDKs but I did not expect that functionality is removed which worked perfectly fine from our end before.

I will update the library packages soon with an updated change log file.

Thanks!

The main problem that we face though, is even if we get to reproduce compatibility from API v10 to App v9.3.3, that we need the other way round to run robustly. I did expect that with API v9.3.3 I can still do basic imaging etc when the App updates to v10.1 (like it was the case from v8 to v9). Because if not, that means whenever a new Clarius app update is available, potentially, any customer of ours might not be able to use the app anymore and it leads to serious disruptions on our side.

What would have helped tremendously is, if the exact v10.1 release date would have been communicated with the third party developer group, so we can prepare for the event beforehand. We did try the pilot but I was not aware that the 1st Dec would be the release date.

I hope these comments help to elaborate on the backwards compatibility problem. Many thanks for your support! Again, happy to discuss this further via a video call if that helps.

julien-l commented 1 year ago

Hello Mobile API v10.1.1 was released: https://github.com/clariusdev/mobileapi/releases/tag/v10.1.1 and the reference documentation updated https://clariusdev.github.io/mobileapi/reference/10.1.1.

In the future, we will improve our release workflow and ensure proper communication with SDK customers, @clariusk will contact you with detailed info. Thanks for your patience an inputs!

julien-l commented 1 year ago

Hello @mischkew the deprecated features will be reinstated with the next Clarius App release v10.3.0, development cycle ends this week so a pilot version should be available mid-April. Changes are demonstrated in the updated example app in the release branch https://github.com/clariusdev/mobileapi/tree/release.

Next change log should be:

Added:

  • MSG_SET_PATIENT_INFO: re-added after removal in previous release.
  • MSG_SET_SETTINGS_INFO: re-added after removal in previous release.
  • MSG_COMPLETE_EXAM: re-added after removal in previous release.
  • SettingsInfo: re-added after removal in previous release.
  • KEY_SETTINGS_INFO: re-added after removal in previous release.
  • KEY_COMPLETE_EXAM: re-added after removal in previous release.

Feature MSG_3P_PACKAGE to start the partner app from the clarius app will be left out.