mobile_scanner icon indicating copy to clipboard operation
mobile_scanner copied to clipboard

Hot restart - camera no longer usable

Open EArminjon opened this issue 1 year ago • 22 comments

If i do a hot restart while camera is displayed, I no longer have the possibility to use it until app is closed.

MobileScannerException: code genericError, message: Called start() while already started

mobile_scanner: ^5.0.1

EArminjon avatar Apr 23 '24 17:04 EArminjon

Here too, I need to reinstall the app. If I close and open it again, the same error persists.

mobile_scanner: ^5.0.0

dcarv01 avatar Apr 23 '24 19:04 dcarv01

@dcarv01 can you tell me your device + os, that's weird to uninstall as a unique solution.

EArminjon avatar Apr 23 '24 20:04 EArminjon

same https://github.com/juliansteenbakker/mobile_scanner/issues/773#issuecomment-2072972048

laterdayi avatar Apr 24 '24 01:04 laterdayi

Personally I didn't have an issue with the hot reload, only the hot restart.

EArminjon avatar Apr 24 '24 04:04 EArminjon

@EArminjon We indeed need to provide better support for hot reload. I filed #773 for that in the past.

We might be able to use the reassemble hook from the framework, but I'm not sure.

I'm going to close this issue in favor of #773 so that we have only one tracking issue.

navaronbracke avatar Apr 24 '24 07:04 navaronbracke

Hello @navaronbracke,

I didn't talk about hot reload but hot restart, can you confirm that your answer fit for hot restart ?

EArminjon avatar Apr 25 '24 13:04 EArminjon

Hot restart is a full restart of the app state. Perhaps if we fix hot reload, we might also fix it for hot restart?

navaronbracke avatar Apr 25 '24 13:04 navaronbracke

Personally i didn't have any issue around hot reload, only hot restart for both Android and iOS.

EArminjon avatar Apr 25 '24 13:04 EArminjon

To be more precise : on hot restart camera work but not the preview on flutter side (black screen) : i can scan some document despite my black screen.

EArminjon avatar Apr 25 '24 14:04 EArminjon

I too have problem only with Hot restart, hot reload is ok.

@dcarv01 can you tell me your device + os, that's weird to uninstall as a unique solution.

I tried again, and there was no need to uninstall, perhaps I was mistaken.

dcarv01 avatar Apr 25 '24 14:04 dcarv01

A hint :

I've removed some protections to force call the stop() method from MobileScanner.swift before instantiate the MobileScannerController() on my dart side. I've added some protection inside this stop() method to only process some action if the variable is not nil.

Result :

On hot restart camera works.

Limit :

I can see the green camera OS icon after the hot restart, so camera is still used by something. Not normal.

Conclusion :

We need to find a way on hot restart to remove used resources or on start() to do so.

(maybe this topic can help once fixed : https://github.com/flutter/flutter/issues/10437)

EArminjon avatar Apr 25 '24 15:04 EArminjon

On our app, we always dispose the MobileScannerController when we need to do 'pause' or to open a new page which potentially will contain an other MobileScannerController().

We use visibility detector to start() the camera and we delay it a bit to let other MobileScannerController() instance the time to dispose. That's an other issue which i will open later because it can be fixed maybe by this issue.

EArminjon avatar Apr 25 '24 15:04 EArminjon

Based on these observations and as it's not hot reload directly linked, I think we can reopen :/ ?

EArminjon avatar Apr 25 '24 15:04 EArminjon

I will keep this issue open for tracking. Perhaps if we fix the hot reload support, the other issue goes away, although I am not certain.

navaronbracke avatar Apr 25 '24 17:04 navaronbracke

@EArminjon I just now published a fix on the master channel that addresses the problems with hot reload in #1086

I plan on releasing a fix with this patch shortly. Could you verify on master (using a git override) if this issue is also fixed by that patch? I specifically targeted hot reload and not hot restart, hence the question.

navaronbracke avatar Sep 30 '24 07:09 navaronbracke

Hello @navaronbracke, still black on hot restart :'( !

  mobile_scanner:
    git:
      url: https://github.com/juliansteenbakker/mobile_scanner
  

EArminjon avatar Sep 30 '24 14:09 EArminjon

Interesting. Does hot reload work for you? (using lowercase r in the terminal when running a Flutter app)

In the case of hot restart (upper case R), do you see any errors in logcat / the terminal? Does the MobileScannerController throw any errors? Does the MobileScannerController.value.error provide you with an error?

navaronbracke avatar Sep 30 '24 14:09 navaronbracke

Hot reload always worked for me. Today i use on my project 5.2.3and not issue with hot reload.

Actually no error despite the black screen. When i put some breakpoints on your package i saw "MobileScannerErrorCode.controllerAlreadyInitialized" (see code after). Maybe the previous instance can't kill the camera and so after app restart it can't create an other one.

 if (error.errorCode ==
          MobileScannerErrorCode.controllerAlreadyInitialized) {
        return;
      }

EArminjon avatar Sep 30 '24 14:09 EArminjon

Hmm, yes this seems like an issue with hot restart like you mentioned earlier in this thread.

I added the check on error.errorCode == MobileScannerErrorCode.controllerAlreadyInitialized to prevent issues with hot reload, as it might call start() on a new instance of the controller there.

I think you might be right that in this case, we should still do the above check, but we should actually stop the old controller and then just continue.

So we have two situations:

Hot Reload (r): The Dart snapshot is reloaded, but the app still runs. In this case, prevent the controller from starting again, using the check above, as the native code has not changed and the app is still running. Only the Dart snapshot has changed in memory. This is what the patch that I finished today does. Hot Restart (R): The entire app reloads from scratch. The native side should be disposed, so that the app can create a new instance (like you indicated)

To fix the second case, we will probably need to know if the app restarted due to hot reload or hot restart. Unless if we change the check above to always dispose the old instance before continuing as usual?

When I implemented the check above I wasn't sure if we should "ignore the additional calls to start and abort" or "detect it, dispose of the old instance and continue with starting a new instance"

So I think I made the wrong choice here? Although, now that we have the new check in place, adjusting it to fit the new situation should be easy (in the if statement, we await stop() and remove the return;)

What do you think about this option?

navaronbracke avatar Sep 30 '24 14:09 navaronbracke

Sorry, for the delay.

The following code didn't fix the issue (even if i dispose my page and recreate it, camera is not longer available), it's more deep.

if (error.errorCode ==
          MobileScannerErrorCode.controllerAlreadyInitialized) {
        await stop();
        return;
      }

As i remember, Flutter didn't allow package to detect Hot Restart (only Hot Reload) : https://github.com/flutter/flutter/issues/10437. I don't know so if it's possible to today to find a way for disposing the camera instance on hot restart.

EArminjon avatar Oct 01 '24 06:10 EArminjon

Shouldn't we do do this instead? (in mobile_scanner, not in your code)

if (error.errorCode == MobileScannerErrorCode.controllerAlreadyInitialized) {
  await stop();
}

// Continue with the rest of the start method, as if the scanner never started before.

But if this also does not fix the issue, then I agree that this can only be fixed when https://github.com/flutter/flutter/issues/10437 is fixed, so that we can detect this case in the plugin, using an API.

navaronbracke avatar Oct 01 '24 07:10 navaronbracke

Shouldn't we do do this instead? (in mobile_scanner, not in your code)

if (error.errorCode == MobileScannerErrorCode.controllerAlreadyInitialized) {
  await stop();
}

// Continue with the rest of the start method, as if the scanner never started before.

But if this also does not fix the issue, then I agree that this can only be fixed when flutter/flutter#10437 is fixed, so that we can detect this case in the plugin, using an API.

I've tested that, it sadly didn't work.

EArminjon avatar Oct 01 '24 07:10 EArminjon

Will be fixed in v6.0.8

juliansteenbakker avatar Apr 18 '25 21:04 juliansteenbakker