Skip to content

Add android14 support in lrs android wrapper example RS Camera - #13910

Merged
Nir-Az merged 4 commits into
realsenseai:developmentfrom
nanotuxi:android_14_lrs_wrapper
Apr 19, 2025
Merged

Add android14 support in lrs android wrapper example RS Camera#13910
Nir-Az merged 4 commits into
realsenseai:developmentfrom
nanotuxi:android_14_lrs_wrapper

Conversation

@nanotuxi

@nanotuxi nanotuxi commented Apr 4, 2025

Copy link
Copy Markdown
Contributor

Adding support for android 14 to example tools/camera of the librealsense wrapper for android

@sysrsbuild

Copy link
Copy Markdown
Contributor

Can one of the admins verify this patch?

@nanotuxi

nanotuxi commented Apr 5, 2025

Copy link
Copy Markdown
Contributor Author

By ading #13718 changes the build.gradle(LRS) is now

defaultCompileSdkVersion = 35
defaultTargetSdkVersion = 34
//defaultMinSdkVersion = 19
defaultMinSdkVersion = 23 

Due lack of time during the weekend I will aprove the patch early next week. Thanks.

@Nir-Az

Nir-Az commented Apr 6, 2025

Copy link
Copy Markdown
Collaborator

By ading #13718 changes the build.gradle(LRS) is now

defaultCompileSdkVersion = 35
defaultTargetSdkVersion = 34
//defaultMinSdkVersion = 19
defaultMinSdkVersion = 23 

Due lack of time during the weekend I will aprove the patch early next week. Thanks.

Hi @nanotuxi
Thanks you for your contribution.
I remind you about #13718 that for it to get merged we will need to ask you to rebase the code and base it (origin+ target) on development branch.
Thanks

@nanotuxi

nanotuxi commented Apr 6, 2025

Copy link
Copy Markdown
Contributor Author

@Nir-Az Thanks for the quick response. The patch also works without using pr 13718. The (dirty) workaround would be to set defaultTargetSdkVersion = 33. So, should we wait for pr 13718 to be rebased by the user having created it and being merged into the development branch and use it later to adapt the TargetSdk versions to the most recent stable versions? My own PR will be rebased by me on dev branch next week. Thank you.

@Nir-Az

Nir-Az commented Apr 6, 2025

Copy link
Copy Markdown
Collaborator

Sorry I thought that PR is also yours.
If it indeed needed for your PR you are welcome to add it to your PR as the previous user did not responsive.

@nanotuxi
nanotuxi force-pushed the android_14_lrs_wrapper branch from 44a2569 to 79b1d0a Compare April 7, 2025 10:02
@nanotuxi

nanotuxi commented Apr 7, 2025

Copy link
Copy Markdown
Contributor Author

@Nir-Az Hopefully it works for android 15. The only device which I can test is based on android 14. The update should also resolve PR 13718 as this PR is based on dev branch.

@Nir-Az

Nir-Az commented Apr 10, 2025

Copy link
Copy Markdown
Collaborator

Thanks, let's pend @mengyui review and then we can proceed

@nanotuxi

nanotuxi commented Apr 10, 2025

Copy link
Copy Markdown
Contributor Author

Oh, I'm not in a hurry 😌

@mengyui

mengyui commented Apr 12, 2025

Copy link
Copy Markdown
Contributor

@nanotuxi I'm sorry for late.
I have some question about this PR.

  1. May I know why you upgrade AGP version to 8.9.1 from 8.1.4? The AGP 8.1.4 should work with current Android Studio (2024.3.1). Have you found the gradle version in your environment?
  2. Would you modify the value of defaultCompileSdkVersion and defaultTargetSdkVersion to the same number, in this PR?
  3. May I know the reason of drop Android 6 support? (change the value of defaultMinSdkVersion to 24)

@nanotuxi

Copy link
Copy Markdown
Contributor Author

@nanotuxi I'm sorry for late. I have some question about this PR.

Oh, that's quite o.k.

1. May I know why you upgrade AGP version to 8.9.1 from 8.1.4?  The AGP 8.1.4 should work with current Android Studio (2024.3.1). Have you found the gradle version in your environment?

Using Android Studio "Meerkat" (2024.3.1) the automatic update mechanism of this recent version suggested to update to AGP 8.9.1 That is the only reason for the update and you are totally right to ask for the reason. In my personal experience it is sometimes useful to update to a more recent version of a software package or framework to make it more future proof and profit from a lot of improved methods and bug-cleaned code. If you do not wish to change to proceed to AGP 8.9.1 we can step back to 8.1.4.

2. Would you modify the value of defaultCompileSdkVersion and defaultTargetSdkVersion to the same number, in this PR?

Yes. That makes sense. As I did not have physical access to android 15 devices at the moment of writing the PR the versions are different. I will make the change to identical versions of compile/targetSDKs.

3. May I know the reason of drop Android 6 support? (change the value of defaultMinSdkVersion to 24)

Android Studio showed an error in one of the wrapper project files that could only be solved updating to defaultMinSdkVersion from 23 to 24. That can be reverted if you want to revert to older software versions. Iam aware of the fact that the range of Android versions should be as wide as possible. But is it really neccessary to provide support for Android 6 in 2025? But if you decide to step back, we will do that.

@mengyui

mengyui commented Apr 14, 2025

Copy link
Copy Markdown
Contributor
  1. both of the AGP version are ok for me.
  2. please update the version number.
  3. @Nir-Az , do you agree to the upgrade? the minsdkversion is ok for me.

@Nir-Az

Nir-Az commented Apr 14, 2025

Copy link
Copy Markdown
Collaborator

@mengyui when you approve the PR after the changes I will merge.

@mengyui mengyui left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you for your contribution.

@nanotuxi

Copy link
Copy Markdown
Contributor Author

Thank you for your contribution.

It was a pleasure for me. Thanks.
@Nir-Az

@mengyui when you approve the PR after the changes I will merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants