dji-sdk / Mobile-SDK-Android-V5

MSDK V5 Sample
Other
247 stars 126 forks source link

Proper way to download photo from drone #377

Open IakovlevAA opened 1 week ago

IakovlevAA commented 1 week ago

Hi Dji, I want to download the last photo I've taken. As for now I have code that pulls media from drone, and downloads photo, but it have some issues.

Device: Mini 3 pro

  1. mediaManager.enable always fails, but fetch is successful
  2. pullMediaFileListFromCamera takes approximately 3-5 seconds, although pulling in sample app is very quick (I am not using any ui)
  3. Download speed is very low, I wait 10-15 seconds for 500kb photo, also in sample app

fun downloadFile()
    {
        enable()
        sleep(3000)
        val mediaSource = MediaFileListDataSource.Builder().setLocation(location).build()
        MediaDataCenter.getInstance().mediaManager.setMediaFileDataSource(CameraStorageLocation.SDCARD)
        sleep(1000)
        pullMedia()

        if (!mediafiles.isEmpty())
        {
            mediaFile = mediafiles.first()
        }

        val dirs: File = File(DiskUtil.getExternalCacheDirPath(ContextUtil.getContext(), mediaFileDir))
        if (!dirs.exists()) {
            dirs.mkdirs()
        }

        val filepath = DiskUtil.getExternalCacheDirPath(ContextUtil.getContext(), mediaFileDir + "/" + mediaFile?.fileName)
        val file: File = File(filepath)
        if (file.exists()) {
            file.delete()
        }

        //Open up the byte output stream.

        var beginTime = System.currentTimeMillis()
        val outputStream = FileOutputStream(file, true)
        val bos = BufferedOutputStream(outputStream)

        mediaFile?.pullPreviewFromCamera(object :
            CommonCallbacks.CompletionCallbackWithParam<Bitmap> {
            override fun onSuccess(t: Bitmap?) {
                val stream = ByteArrayOutputStream()
                t?.compress(Bitmap.CompressFormat.PNG, 50, stream)
                val image = stream.toByteArray()
                bytes = image
                bos.write(bytes, 0, bytes.size)
                bos.flush()
                try
                {
                    outputStream.close()
                    bos.close()
                }
                catch (error: IOException) {
                    LogUtils.e("MediaFile" , "close error$error" )
                }
                ToastUtils.showToast("bytes: ${bytes.size} spend time: ${(System.currentTimeMillis() - beginTime) / 1000 } s"  )
            }

            override fun onFailure(error: IDJIError) {
                LogUtils.e("MediaFile", "fetch preview failed$error")
            }

        })
    var mediaFile: MediaFile? = null
    var mediaFileDir = "/media"
    var mediafiles : List<MediaFile> = listOf()
    var bytes = ByteArray(0)

    fun enable() {
        MediaDataCenter.getInstance().mediaManager.enable(object :
            CommonCallbacks.CompletionCallback {
            override fun onSuccess() {
                ToastUtils.showToast("enable playback success")
            }

            override fun onFailure(error: IDJIError) {
                ToastUtils.showToast("error is ${error.description()}")
            }
        })
    }

    fun pullMedia()
    {
        var currentTime = System.currentTimeMillis()
        MediaDataCenter.getInstance().mediaManager.pullMediaFileListFromCamera(
            PullMediaFileListParam.Builder().mediaFileIndex(-1).count(-1).filter(MediaFileFilter.PHOTO).build(),
            object :
                CommonCallbacks.CompletionCallback {
                override fun onSuccess() {
                    ToastUtils.showToast("Spend time:${(System.currentTimeMillis() - currentTime) / 1000}s")
                    println("fetch success")
                    mediafiles = MediaDataCenter.getInstance().mediaManager.mediaFileListData.data
                }

                override fun onFailure(error: IDJIError) {
                    ToastUtils.showToast("fetch failed$error")
                }
            })
    }
antonymarion commented 1 week ago

Why are you putting sleeping timer and not using (nesting your code) into success callback instead ?

For instance enable is followed by sleep, why don't you put the code after the sleep into the success callback of enable instead ?

IakovlevAA commented 1 week ago

I think I'll try your method, but theoretically I don't need to nest it because for example:

MediaDataCenter.getInstance().mediaManager.enable(object :
            CommonCallbacks.CompletionCallback {
            override fun onSuccess() {
                var currentTime = System.currentTimeMillis()
                MediaDataCenter.getInstance().mediaManager.pullMediaFileListFromCamera(
                    PullMediaFileListParam.Builder().mediaFileIndex(-1).count(-1).filter(MediaFileFilter.PHOTO).build(),
                    object :
                        CommonCallbacks.CompletionCallback {
                        override fun onSuccess() {
                            ToastUtils.showToast("Spend time:${(System.currentTimeMillis() - currentTime) / 1000}s")
                            println("fetch success")
                            mediafiles = MediaDataCenter.getInstance().mediaManager.mediaFileListData.data
                        }

                        override fun onFailure(error: IDJIError) {
                            ToastUtils.showToast("fetch failed$error")
                        }
                    })
            }

            override fun onFailure(error: IDJIError) {
                ToastUtils.showToast("error is ${error.description()}")
            }
        })

This code, if I understood you, doesn't look correct to me...

antonymarion commented 1 week ago

This code is correct, this is kind of callback hell (will lead to nested callback inside nested callback and so on), but it is the correct way to handle asynchronous execution.

Promise Like with await/async is much better to read: https://kotlinlang.org/docs/composing-suspending-functions.html#concurrent-using-async

https://kotlinlang.org/docs/async-programming.html

IakovlevAA commented 1 week ago

Thanks for tips! Interesting what others think about it. I'm just 100% confident, that there is another way, but not nesting callbacks

antonymarion commented 1 week ago

Yes async await as explained in the docs above.

Callback is error prone and not maintanable.

Promise with then is better for maintenance.

Async await is the best option

antonymarion commented 1 week ago

By the way, I tested right now (removing all the timers) I have no issue of slow down like you have, I believe that your timer does not help..

IakovlevAA commented 1 week ago

Do you mean slow down of fetching mediafiles or pulling image? Without timers I couldn't get code to work at all

IakovlevAA commented 1 week ago

As Antony mentioned, I tried nesting callbacks. Also I found, that if you call CameraKey.KeyStartShootPhoto.create().action() and after that trying to do something with MediaDataCenter or MediaManager it will process comands a very long time(enabling will always fail btw). So, that was the reason why fetching and downloading was too slow

antonymarion commented 1 week ago

Call enabling with MediaManager instead.

It will work then

IakovlevAA commented 1 week ago

No, still failing with this code

fun takePhoto(callback: CommonCallbacks.CompletionCallback) {
        RxUtil.setValue(
            KeyTools.createKey<CameraMode>(
                CameraKey.KeyCameraMode
            ), CameraMode.PHOTO_NORMAL)
            .andThen(RxUtil.performActionWithOutResult(KeyTools.createKey(CameraKey.KeyStartShootPhoto)))
            .subscribe({ CallbackUtils.onSuccess(callback) }
            ) { throwable: Throwable ->
                CallbackUtils.onFailure(
                    callback,
                    (throwable as RxError).djiError
                )
            }
    }

    fun shootPhoto()
    {

        takePhoto(object : CommonCallbacks.CompletionCallback {
            override fun onSuccess() {
                ToastUtils.showToast("take photo success")
                var currentTime = System.currentTimeMillis()
                MediaManager.getInstance().enable(object :
                    CommonCallbacks.CompletionCallback {
                    override fun onSuccess() {
                        ToastUtils.showToast("enable playback success")
                        val mediaSource = MediaFileListDataSource.Builder().setLocation(CameraStorageLocation.SDCARD).build()
                        MediaDataCenter.getInstance().mediaManager.setMediaFileDataSource(mediaSource)
                        MediaDataCenter.getInstance().mediaManager.pullMediaFileListFromCamera(
                            PullMediaFileListParam.Builder().mediaFileIndex(-1).count(-1).filter(MediaFileFilter.PHOTO).build(),
                            object :
                                CommonCallbacks.CompletionCallback {
                                override fun onSuccess() {
                                    ToastUtils.showToast("Spend time:${(System.currentTimeMillis() - currentTime) / 1000}s")
                                    println("fetch success")
                                    mediafiles = MediaDataCenter.getInstance().mediaManager.mediaFileListData.data
                                    downloadFile()
                                }

                                override fun onFailure(error: IDJIError) {
                                    ToastUtils.showToast("fetch failed$error")
                                }
                            })
                    }

                    override fun onFailure(error: IDJIError) {
                        ToastUtils.showToast("error is ${error.description()}")
                    }
                })
                    }

                    override fun onFailure(error: IDJIError) {
                        ToastUtils.showToast("take photo failed")
                    }
                })
    }

Noticed that there is an error with this code

java.lang.NoClassDefFoundError: Failed resolution of: Ldji/v5/common/callback/CommonCallbacks$CompletionCallback;

But if I insert this code in begining of function shootPhoto

if(CameraKey.KeyIsShootingPhoto.create().get() == true)
{
          CameraKey.KeyStopShootPhoto.create().action()     
}

MediaManager won't be enabled

antonymarion commented 1 week ago

Weird.

Working on my side, I use 300 RTK for testing

If ever you do not find with the official DJI support then go to my profile page and take the $50 sponsor I will give you support.

IakovlevAA commented 1 week ago

Tried another approach:

fun pullMedia()
    {
        var currentTime = System.currentTimeMillis()
        MediaManager.getInstance().enable(object :
            CommonCallbacks.CompletionCallback {
            override fun onSuccess() {
                ToastUtils.showToast("enable playback success")
                val mediaSource = MediaFileListDataSource.Builder().setLocation(CameraStorageLocation.SDCARD).build()
                MediaDataCenter.getInstance().mediaManager.setMediaFileDataSource(mediaSource)
                MediaDataCenter.getInstance().mediaManager.pullMediaFileListFromCamera(
                    PullMediaFileListParam.Builder().mediaFileIndex(-1).count(-1).filter(MediaFileFilter.PHOTO).build(),
                    object :
                        CommonCallbacks.CompletionCallback {
                        override fun onSuccess() {
                            ToastUtils.showToast("Spend time:${(System.currentTimeMillis() - currentTime) / 1000}s")
                            println("fetch success")
                            mediafiles = MediaDataCenter.getInstance().mediaManager.mediaFileListData.data
                        }

                        override fun onFailure(error: IDJIError) {
                            ToastUtils.showToast("fetch failed$error")
                        }
                    })
            }

            override fun onFailure(error: IDJIError) {
                ToastUtils.showToast("error is ${error.description()}")
            }
        })

    }

fun shootPhoto(): ByteArray {
        rotateCamera()

        KeyManager.getInstance().performAction(KeyTools.createKey(CameraKey.KeyResetCameraSetting), object: CommonCallbacks.CompletionCallbackWithParam<EmptyMsg>{
            override fun onSuccess(t: EmptyMsg?) {
                ToastUtils.showToast("reset photo success")
                KeyManager.getInstance().performAction(KeyTools.createKey(CameraKey.KeyStartShootPhoto), object: CommonCallbacks.CompletionCallbackWithParam<EmptyMsg>{
                    override fun onSuccess(t: EmptyMsg?) {
                        ToastUtils.showToast("take photo success")
                        pullMedia()
                    }

                    override fun onFailure(error: IDJIError) {
                        ToastUtils.showToast("take photo failed $error")
                    }
                })
    }

            override fun onFailure(error: IDJIError) {
                ToastUtils.showToast("reset photo failed $error")
            }
        })

First shoot is ok, but after that CameraKey.KeyStartShootPhoto returns CANNOT_START_TASK_VLOTAGE_ALARM error. I've read that I must set camera mode, but it doesn't help

dji-dev commented 1 week ago

Agent comment from yating.liao in Zendesk ticket #110093:

I have reviewed your code. After switching the camera mode, you directly take a photo and then use mediaManager to retrieve the information. The sequence is correct, but we need to consider the time required for the camera to switch modes and generate the photo.

I would recommend adjusting your code according to this logic:

  1. Listen for changes in KeyCameraMode to determine when the camera has completed the mode switch. The success of setValue only indicates that the camera has successfully received the command.
  2. Add a MediaFileListStateListener to monitor the status of media files. After taking a photo, the camera will generate the corresponding file, and the file list status can indicate whether the camera has completed writing the file. UP_TO_DATE indicates that the file list is up to date, and you can directly retrieve the latest file type using getMediaFileListData. Typically, the latest photo is placed first. It is worth to noting that you need to call pullMediaFileListFromCamera at least once to initialize the listener and file list.
  3. Before downloading the file, use MediaManager.getInstance().enable to put the camera into playback mode. You can use CameraKey.KeyIsPlayingBack to determine if it is in playback mode.

    °°°

IakovlevAA commented 1 week ago

Thanks, I'll try this. I have some questions:

  1. Do I need to disable MediaManager if I want to take photos again?
  2. Do I need to call pullMediaFileListFromCamera again, if I disabled MediaManager?
  3. Does MediaDataCenter.getInstance().mediaManager.mediaFileListData updates by itself or I need to call pullMedia again
antonymarion commented 1 week ago

Agent comment from yating.liao in Zendesk ticket #110093:I have reviewed your code. After switching the camera mode, you directly take a photo and then use mediaManager to retrieve the information. The sequence is correct, but we need to consider the time required for the camera to switch modes and generate the photo.

I would recommend adjusting your code according to this logic:

  1. Listen for changes in KeyCameraMode to determine when the camera has completed the mode switch. The success of setValue only indicates that the camera has successfully received the command.
  2. Add a MediaFileListStateListener to monitor the status of media files. After taking a photo, the camera will generate the corresponding file, and the file list status can indicate whether the camera has completed writing the file. UP_TO_DATE indicates that the file list is up to date, and you can directly retrieve the latest file type using getMediaFileListData. Typically, the latest photo is placed first. It is worth to noting that you need to call pullMediaFileListFromCamera at least once to initialize the listener and file list.
  3. Before downloading the file, use MediaManager.getInstance().enable to put the camera into playback mode. You can use CameraKey.KeyIsPlayingBack to determine if it is in playback mode. °°°

Just to make sure, you said "Listen for changes in KeyCameraMode", but, is it necessary to listen if we can simply use RxUtil.setValue to cameraMode PHOTO_NORMAL and chain the promise using andThen instead ?

Normally, if the Promise is returned after the CameraMode has resolved, it should already be in the right state to take the Photo. So, no need to create a dedicated listener for this, right?

dji-dev commented 4 days ago

Agent comment from yating.liao in Zendesk ticket #110093:

1. Do I need to disable MediaManager if I want to take photos again? -- Yes. After exiting the playback mode, you can take photos.

  1. Do I need to call pullMediaFileListFromCamera again, if I disabled MediaManager? -- No.The file list will update automatically, even if you disable MediaManager.
  2. Does MediaDataCenter.getInstance().mediaManager.mediaFileListData updates by itself or I need to call pullMedia again. -- Yes, it will update automatically. The file list status UP_TO_DATE indicates that the update is complete.

    °°°

IakovlevAA commented 4 days ago

Thanks, it helped a lot! Yet, one another question: Is it necessary to enter playaback mode to download photos? As I can see, I can enable MediaManager, start pulling photos and as it updates without MediaManager, I can just look for photo by its index. So, in short - does pullOriginalMediaFileFromCamera need playback mode enabled?

antonymarion commented 4 days ago

And please do not forget my question about chained promises using RxUtil setValue.

It is quite critical to know if we can rely on promises in DJI's SDK or if we have to set listeners and trigger some logic on events received instead, as you proposed...

dji-dev commented 3 days ago

Agent comment from yating.liao in Zendesk ticket #110093:

Do you mean if the callback result of the SDK interface can be used as the result for the next step in RxUtil? I would recommend using the listeners provided by the SDK as the callback result of the SDK interface does not represent the execution result of the drone. This is also the approach that Mobile SDK has always taken.

°°°

antonymarion commented 3 days ago

Well, it answers to my question, but the fact that "this is also the approach that Mobile SDK has always taken." is not a justification if you place yourself at the users' perspective.

You said "the callback result of the SDK interface does not represent the execution result of the drone". So it confirms what I noticed and did not expect from an SDK. But I prefered to be sure and to have your confirmation.

Do you believe the DJI's SDK will handle callback/promises as expected in a near future?

Promise or Callback are designed to be chained, and if it is not possible with DJI's SDK then it is a design flaw inside the SDK.

dji-lyt commented 2 days ago

@antonymarion We can discuss under the issue you created, which will continue to track the problem with @IakovlevAA .