-
-
Notifications
You must be signed in to change notification settings - Fork 605
fix: Clean up error codes #1180
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
9f6f5ae
c9df271
b73aa67
0b8b532
ece227d
93e38b4
1ab7f5a
41382ee
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -120,12 +120,7 @@ class MobileScanner( | |
} | ||
|
||
if (!returnImage) { | ||
mobileScannerCallback( | ||
barcodeMap, | ||
null, | ||
null, | ||
null | ||
) | ||
mobileScannerCallback(barcodeMap, null, null, null) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. formatting |
||
return@addOnSuccessListener | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
package dev.steenbakker.mobile_scanner.objects | ||
|
||
class MobileScannerErrorCodes { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consolidating the error codes in one place. MacOS and iOS also have an equivalent. Will be used by the new fix. |
||
companion object { | ||
const val ALREADY_STARTED_ERROR = "MOBILE_SCANNER_ALREADY_STARTED_ERROR" | ||
const val ALREADY_STARTED_ERROR_MESSAGE = "The scanner was already started." | ||
// The error code 'BARCODE_ERROR' does not have an error message, | ||
// because it uses the error message from the underlying error. | ||
const val BARCODE_ERROR = "MOBILE_SCANNER_BARCODE_ERROR" | ||
// The error code 'CAMERA_ACCESS_DENIED' does not have an error message, | ||
// because it is used for a boolean result. | ||
const val CAMERA_ACCESS_DENIED = "MOBILE_SCANNER_CAMERA_PERMISSION_DENIED" | ||
const val CAMERA_ERROR = "MOBILE_SCANNER_CAMERA_ERROR" | ||
const val CAMERA_ERROR_MESSAGE = "An error occurred when opening the camera." | ||
const val CAMERA_PERMISSIONS_REQUEST_ONGOING = "MOBILE_SCANNER_CAMERA_PERMISSION_REQUEST_PENDING" | ||
const val CAMERA_PERMISSIONS_REQUEST_ONGOING_MESSAGE = "Another request is ongoing and multiple requests cannot be handled at once." | ||
const val GENERIC_ERROR = "MOBILE_SCANNER_GENERIC_ERROR" | ||
const val GENERIC_ERROR_MESSAGE = "An unknown error occurred." | ||
const val INVALID_ZOOM_SCALE_ERROR_MESSAGE = "The zoom scale should be between 0 and 1 (both inclusive)" | ||
const val NO_CAMERA_ERROR = "MOBILE_SCANNER_NO_CAMERA_ERROR" | ||
const val NO_CAMERA_ERROR_MESSAGE = "No cameras available." | ||
const val SET_SCALE_WHEN_STOPPED_ERROR = "MOBILE_SCANNER_SET_SCALE_WHEN_STOPPED_ERROR" | ||
const val SET_SCALE_WHEN_STOPPED_ERROR_MESSAGE = "The zoom scale cannot be changed when the camera is stopped." | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -60,7 +60,7 @@ class _BarcodeScannerWithControllerState | |
|
||
@override | ||
void didChangeAppLifecycleState(AppLifecycleState state) { | ||
if (!controller.value.isInitialized) { | ||
if (!controller.value.hasCameraPermission) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the scanner has no permission, prevent app lifecycle changes from changing the controller state. |
||
return; | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
// | ||
// MobileScannerErrorCodes.swift | ||
// mobile_scanner | ||
// | ||
// Created by Navaron Bracke on 28/05/2024. | ||
// | ||
|
||
import Foundation | ||
|
||
/// This struct defines the error codes and error messages for MobileScanner errors. | ||
/// | ||
/// These are used by `FlutterError` as error code and error message. | ||
/// | ||
/// This struct should not be confused with `MobileScannerError`, | ||
/// which is an implementation detail for the iOS implementation. | ||
struct MobileScannerErrorCodes { | ||
static let ALREADY_STARTED_ERROR = "MOBILE_SCANNER_ALREADY_STARTED_ERROR" | ||
static let ALREADY_STARTED_ERROR_MESSAGE = "The scanner was already started." | ||
// The error code 'BARCODE_ERROR' does not have an error message, | ||
// because it uses the error message from the undelying error. | ||
static let BARCODE_ERROR = "MOBILE_SCANNER_BARCODE_ERROR" | ||
// The error code 'CAMERA_ERROR' does not have an error message, | ||
// because it uses the error message from the underlying error. | ||
static let CAMERA_ERROR = "MOBILE_SCANNER_CAMERA_ERROR" | ||
static let GENERIC_ERROR = "MOBILE_SCANNER_GENERIC_ERROR" | ||
static let GENERIC_ERROR_MESSAGE = "An unknown error occurred." | ||
// This message is used with the 'GENERIC_ERROR' error code. | ||
static let INVALID_ZOOM_SCALE_ERROR_MESSAGE = "The zoom scale should be between 0 and 1 (both inclusive)" | ||
static let NO_CAMERA_ERROR = "MOBILE_SCANNER_NO_CAMERA_ERROR" | ||
static let NO_CAMERA_ERROR_MESSAGE = "No cameras available." | ||
static let SET_SCALE_WHEN_STOPPED_ERROR = "MOBILE_SCANNER_SET_SCALE_WHEN_STOPPED_ERROR" | ||
static let SET_SCALE_WHEN_STOPPED_ERROR_MESSAGE = "The zoom scale cannot be changed when the camera is stopped." | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -258,9 +258,7 @@ public class MobileScannerPlugin: NSObject, FlutterPlugin { | |
let uiImage = UIImage(contentsOfFile: (call.arguments as! Dictionary<String, Any?>)["filePath"] as? String ?? "") | ||
|
||
if (uiImage == nil) { | ||
result(FlutterError(code: "MobileScanner", | ||
message: "No image found in analyzeImage!", | ||
details: nil)) | ||
result(nil) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of returning a non-standard error message, just use nil |
||
return | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
import 'package:flutter/services.dart'; | ||
import 'package:mobile_scanner/src/mobile_scanner_controller.dart'; | ||
|
||
/// This enum defines the different error codes for the mobile scanner. | ||
|
@@ -24,5 +25,25 @@ enum MobileScannerErrorCode { | |
permissionDenied, | ||
|
||
/// Scanning is unsupported on the current device. | ||
unsupported, | ||
unsupported; | ||
|
||
/// Convert the given [PlatformException.code] to a [MobileScannerErrorCode]. | ||
factory MobileScannerErrorCode.fromPlatformException( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To avoid magic strings, I added a factory constructor for this. |
||
PlatformException exception, | ||
) { | ||
// The following error code mapping should be kept in sync with their native counterparts. | ||
// These are located in `MobileScannerErrorCodes.kt` and `MobileScannerErrorCodes.swift`. | ||
return switch (exception.code) { | ||
// In case the scanner was already started, report the right error code. | ||
// If the scanner is already starting, | ||
// this error code is a signal to the controller to just ignore the attempt. | ||
'MOBILE_SCANNER_ALREADY_STARTED_ERROR' => | ||
MobileScannerErrorCode.controllerAlreadyInitialized, | ||
// In case no cameras are available, using the scanner is not supported. | ||
'MOBILE_SCANNER_NO_CAMERA_ERROR' => MobileScannerErrorCode.unsupported, | ||
'MOBILE_SCANNER_CAMERA_PERMISSION_DENIED' => | ||
MobileScannerErrorCode.permissionDenied, | ||
_ => MobileScannerErrorCode.genericError, | ||
}; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -187,8 +187,7 @@ class MethodChannelMobileScanner extends MobileScannerPlatform { | |
throw const MobileScannerException( | ||
errorCode: MobileScannerErrorCode.controllerAlreadyInitialized, | ||
errorDetails: MobileScannerErrorDetails( | ||
message: | ||
'The scanner was already started. Call stop() before calling start() again.', | ||
message: 'The scanner was already started.', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using a shorter message makes the formatting a bit nicer. We're going to swallow this type of error in the upcoming fix anyway. |
||
), | ||
); | ||
} | ||
|
@@ -204,7 +203,7 @@ class MethodChannelMobileScanner extends MobileScannerPlatform { | |
); | ||
} on PlatformException catch (error) { | ||
throw MobileScannerException( | ||
errorCode: MobileScannerErrorCode.genericError, | ||
errorCode: MobileScannerErrorCode.fromPlatformException(error), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Convert the incoming error into the right code. The Android/iOS/macOS implementations will be reporting different error codes, depending on the situation. |
||
errorDetails: MobileScannerErrorDetails( | ||
code: error.code, | ||
details: error.details as Object?, | ||
|
@@ -240,17 +239,13 @@ class MethodChannelMobileScanner extends MobileScannerPlatform { | |
startResult['currentTorchState'] as int? ?? -1, | ||
); | ||
|
||
final Map<Object?, Object?>? sizeInfo = | ||
startResult['size'] as Map<Object?, Object?>?; | ||
final double? width = sizeInfo?['width'] as double?; | ||
final double? height = sizeInfo?['height'] as double?; | ||
|
||
final Size size; | ||
|
||
if (width == null || height == null) { | ||
size = Size.zero; | ||
} else { | ||
if (startResult['size'] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If-case to extract the size |
||
case {'width': final double width, 'height': final double height}) { | ||
size = Size(width, height); | ||
} else { | ||
size = Size.zero; | ||
} | ||
|
||
return MobileScannerViewAttributes( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -281,8 +281,7 @@ class _MobileScannerState extends State<MobileScanner> | |
|
||
@override | ||
void didChangeAppLifecycleState(AppLifecycleState state) { | ||
if (widget.controller != null) return; | ||
if (!controller.value.isInitialized) { | ||
if (widget.controller != null || !controller.value.hasCameraPermission) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using the new getter to fix a bug in the sample. |
||
return; | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
import 'package:mobile_scanner/src/enums/mobile_scanner_error_code.dart'; | ||
|
||
/// This class represents an exception thrown by the mobile scanner. | ||
/// This class represents an exception thrown by the [MobileScannerController]. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I cannot yet use |
||
class MobileScannerException implements Exception { | ||
const MobileScannerException({ | ||
required this.errorCode, | ||
|
@@ -16,9 +16,9 @@ class MobileScannerException implements Exception { | |
@override | ||
String toString() { | ||
if (errorDetails != null && errorDetails?.message != null) { | ||
return "MobileScannerException: code ${errorCode.name}, message: ${errorDetails?.message}"; | ||
return 'MobileScannerException(${errorCode.name}, ${errorDetails?.message})'; | ||
} | ||
return "MobileScannerException: ${errorCode.name}"; | ||
return 'MobileScannerException(${errorCode.name})'; | ||
} | ||
} | ||
|
||
|
@@ -46,3 +46,22 @@ class MobileScannerErrorDetails { | |
/// This exception type is only used internally, | ||
/// and is not part of the public API. | ||
class PermissionRequestPendingException implements Exception {} | ||
|
||
/// This class represents an exception thrown by the [MobileScannerController] | ||
/// when a barcode scanning error occurs when processing an input frame. | ||
class MobileScannerBarcodeException implements Exception { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This error will be used by That will be added in the actual fix pull request. |
||
/// Creates a new [MobileScannerBarcodeException] with the given error message. | ||
const MobileScannerBarcodeException(this.message); | ||
|
||
/// The error message of the exception. | ||
final String? message; | ||
|
||
@override | ||
String toString() { | ||
if (message?.isNotEmpty ?? false) { | ||
return 'MobileScannerBarcodeException($message)'; | ||
} | ||
|
||
return 'MobileScannerBarcodeException(Could not detect a barcode in the input image.)'; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
import 'dart:ui'; | ||
|
||
import 'package:mobile_scanner/src/enums/camera_facing.dart'; | ||
import 'package:mobile_scanner/src/enums/mobile_scanner_error_code.dart'; | ||
import 'package:mobile_scanner/src/enums/torch_state.dart'; | ||
import 'package:mobile_scanner/src/mobile_scanner_exception.dart'; | ||
|
||
|
@@ -43,7 +44,8 @@ class MobileScannerState { | |
|
||
/// Whether the mobile scanner has initialized successfully. | ||
/// | ||
/// This is `true` if the camera is ready to be used. | ||
/// This does not indicate that the camera permission was granted. | ||
/// To check if the camera permission was granted, use [hasCameraPermission]. | ||
final bool isInitialized; | ||
|
||
/// Whether the mobile scanner is currently running. | ||
|
@@ -60,6 +62,12 @@ class MobileScannerState { | |
/// The current zoom scale of the camera. | ||
final double zoomScale; | ||
|
||
/// Whether permission to access the camera was granted. | ||
bool get hasCameraPermission { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This new getter is used to fix a bug in our lifecycle handling. (in the samples) |
||
return isInitialized && | ||
error?.errorCode != MobileScannerErrorCode.permissionDenied; | ||
} | ||
|
||
/// Create a copy of this state with the given parameters. | ||
MobileScannerState copyWith({ | ||
int? availableCameras, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
// | ||
// MobileScannerErrorCodes.swift | ||
// mobile_scanner | ||
// | ||
// Created by Navaron Bracke on 27/05/2024. | ||
// | ||
|
||
import Foundation | ||
|
||
struct MobileScannerErrorCodes { | ||
static let ALREADY_STARTED_ERROR = "MOBILE_SCANNER_ALREADY_STARTED_ERROR" | ||
static let ALREADY_STARTED_ERROR_MESSAGE = "The scanner was already started." | ||
// The error code 'BARCODE_ERROR' does not have an error message, | ||
// because it uses the error message from the undelying error. | ||
static let BARCODE_ERROR = "MOBILE_SCANNER_BARCODE_ERROR" | ||
// The error code 'CAMERA_ERROR' does not have an error message, | ||
// because it uses the error message from the underlying error. | ||
static let CAMERA_ERROR = "MOBILE_SCANNER_CAMERA_ERROR" | ||
static let NO_CAMERA_ERROR = "MOBILE_SCANNER_NO_CAMERA_ERROR" | ||
static let NO_CAMERA_ERROR_MESSAGE = "No cameras available." | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -273,7 +273,6 @@ public class MobileScannerPlugin: NSObject, FlutterPlugin, FlutterStreamHandler, | |
|
||
let argReader = MapArgumentReader(call.arguments as? [String: Any]) | ||
|
||
// let ratio: Int = argReader.int(key: "ratio") | ||
let torch:Bool = argReader.bool(key: "torch") ?? false | ||
let facing:Int = argReader.int(key: "facing") ?? 1 | ||
let speed:Int = argReader.int(key: "speed") ?? 0 | ||
|
@@ -327,7 +326,6 @@ public class MobileScannerPlugin: NSObject, FlutterPlugin, FlutterStreamHandler, | |
videoOutput.setSampleBufferDelegate(self, queue: DispatchQueue.main) | ||
captureSession!.addOutput(videoOutput) | ||
for connection in videoOutput.connections { | ||
// connection.videoOrientation = .portrait | ||
if position == .front && connection.isVideoMirroringSupported { | ||
connection.isVideoMirrored = true | ||
} | ||
|
@@ -459,20 +457,14 @@ public class MobileScannerPlugin: NSObject, FlutterPlugin, FlutterStreamHandler, | |
let symbologies:[VNBarcodeSymbology] = argReader.toSymbology() | ||
|
||
guard let filePath: String = argReader.string(key: "filePath") else { | ||
// TODO: fix error code | ||
result(FlutterError(code: "MobileScanner", | ||
message: "No image found in analyzeImage!", | ||
details: nil)) | ||
result(nil) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Like on iOS, use nil |
||
return | ||
} | ||
|
||
let fileUrl = URL(fileURLWithPath: filePath) | ||
|
||
guard let ciImage = CIImage(contentsOf: fileUrl) else { | ||
// TODO: fix error code | ||
result(FlutterError(code: "MobileScanner", | ||
message: "No image found in analyzeImage!", | ||
details: nil)) | ||
result(nil) | ||
return | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do prefer single quotes, and we had one violation. So I turned on the lint.