Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,9 @@ public List<Long> filter(@NonNull Long identifier, @NonNull List<Long> cameraInf
}

List<CameraInfo> filteredCameraInfos = cameraSelector.filter(cameraInfosForFilter);
final CameraInfoFlutterApiImpl cameraInfoFlutterApiImpl =
new CameraInfoFlutterApiImpl(binaryMessenger, instanceManager);
List<Long> filteredCameraInfosIds = new ArrayList<Long>();

for (CameraInfo cameraInfo : filteredCameraInfos) {
cameraInfoFlutterApiImpl.create(cameraInfo, result -> {});
Long filteredCameraInfoId = instanceManager.getIdentifierForStrongReference(cameraInfo);
filteredCameraInfosIds.add(filteredCameraInfoId);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@
import 'dart:async';

import 'package:camera_platform_interface/camera_platform_interface.dart';
import 'package:flutter/foundation.dart';

import 'camera_info.dart';
import 'camera_selector.dart';
import 'process_camera_provider.dart';

/// The Android implementation of [CameraPlatform] that uses the CameraX library.
class AndroidCameraCameraX extends CameraPlatform {
Expand All @@ -13,9 +18,67 @@ class AndroidCameraCameraX extends CameraPlatform {
CameraPlatform.instance = AndroidCameraCameraX();
}

ProcessCameraProvider? processCameraProvider;
CameraSelector? backCameraSelector;
CameraSelector? frontCameraSelector;
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest either making these private and keeping the setters or keeping them public (and marking them @visibleForTesting) and setting them directly.

Copy link
Contributor

Choose a reason for hiding this comment

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

(Optional suggestion) I'd suggest keeping the CameraSelectors public and remove the setters, but keep the processCameraProvider as is because if you move the null check to the setter for processCameraProvider like I commented below, it could be useful in this class, as well.



/// Returns list of all available cameras and their descriptions.
@override
Future<List<CameraDescription>> availableCameras() async {
throw UnimplementedError('availableCameras() is not implemented.');
final List<CameraDescription> cameraDescriptions = <CameraDescription>[];

processCameraProvider ??= await ProcessCameraProvider.getInstance();
Copy link
Contributor

Choose a reason for hiding this comment

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

If you keep the setter for this, I'd use it here for consistency. You can move the null check to that method, too.

final List<CameraInfo> cameraInfos =
await processCameraProvider!.getAvailableCameraInfos();

backCameraSelector ??= CameraSelector.getDefaultBackCamera();
frontCameraSelector ??= CameraSelector.getDefaultFrontCamera();
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest initializing these inline above versus in this method.


CameraLensDirection? cameraLensDirection;
int cameraCount = 0;
int? cameraSensorOrientation;
String? cameraName;

for (final CameraInfo cameraInfo in cameraInfos) {
// Determine the lens direction by filtering the CameraInfo
// TODO(gmackall): replace this with call to CameraInfo.getLensFacing when changes containing that method are available
if ((await backCameraSelector!.filter(<CameraInfo>[cameraInfo]))
.isNotEmpty) {
cameraLensDirection = CameraLensDirection.back;
} else if ((await frontCameraSelector!.filter(<CameraInfo>[cameraInfo]))
.isNotEmpty) {
cameraLensDirection = CameraLensDirection.front;
} else {
//Skip this CameraInfo as its lens direction is unknown
continue;
}

cameraSensorOrientation = await cameraInfo.getSensorRotationDegrees();
cameraName = 'Camera $cameraCount';
cameraCount++;

cameraDescriptions.add(CameraDescription(
name: cameraName,
lensDirection: cameraLensDirection,
sensorOrientation: cameraSensorOrientation));
}

return cameraDescriptions;
}

@visibleForTesting
void setDefaultFrontCameraSelector(CameraSelector frontCameraSelector) {
this.frontCameraSelector = frontCameraSelector;
}

@visibleForTesting
void setDefaultBackCameraSelector(CameraSelector backCameraSelector) {
this.backCameraSelector = backCameraSelector;
}

@visibleForTesting
void setProcessCameraProvider(ProcessCameraProvider processCameraProvider) {
this.processCameraProvider = processCameraProvider;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
import 'package:camera_android_camerax/camera_android_camerax.dart';
import 'package:camera_android_camerax/src/camera_info.dart';
import 'package:camera_android_camerax/src/camera_selector.dart';
import 'package:camera_android_camerax/src/process_camera_provider.dart';
import 'package:camera_platform_interface/camera_platform_interface.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:mockito/annotations.dart';
import 'package:mockito/mockito.dart';

import 'android_camera_camerax_test.mocks.dart';

@GenerateMocks(<Type>[
ProcessCameraProvider,
CameraSelector,
CameraInfo,
])
void main() {
TestWidgetsFlutterBinding.ensureInitialized();

test('Should fetch CameraDescription instances for available cameras',
() async {
// Arrange
final List<dynamic> returnData = <dynamic>[
<String, dynamic>{
'name': 'Camera 0',
'lensFacing': 'back',
'sensorOrientation': 0
},
<String, dynamic>{
'name': 'Camera 1',
'lensFacing': 'front',
'sensorOrientation': 90
}
];

//Create mocks to use
final MockProcessCameraProvider mockProcessCameraProvider =
MockProcessCameraProvider();
final MockCameraSelector mockBackCameraSelector = MockCameraSelector();
final MockCameraSelector mockFrontCameraSelector = MockCameraSelector();
final MockCameraInfo mockFrontCameraInfo = MockCameraInfo();
final MockCameraInfo mockBackCameraInfo = MockCameraInfo();
AndroidCameraCameraX.registerWith();

//Set class level ProcessCameraProvider and camera selectors to created mocks
final AndroidCameraCameraX androidCameraCamerax = AndroidCameraCameraX();
androidCameraCamerax.setDefaultBackCameraSelector(mockBackCameraSelector);
Copy link
Contributor

Choose a reason for hiding this comment

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

The changes look good but I think you need to update the test! This file will also need the license at the top:

// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

Copy link
Member Author

@gmackall gmackall Feb 8, 2023

Choose a reason for hiding this comment

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

Sorry about that! Fixed the tests, and added the license.

I also moved the null check back to the availableCameras method and out of setProcessCameraProvider because

  1. In my mind (as named) a setter should set the variable even if it is non-null, and
  2. If the null check is in the setter, then the tests will all need to mock the host platform reply to getting the instance of ProcessCameraProvider, because we follow that code path when we pass it as an argument to the setter (even though we don't set, because in the setter we do the null check and find the _processCameraProvider already set to the mock).

Open to moving it back if you prefer it that way though!

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not tied to my approach. This should be fine!

androidCameraCamerax.setDefaultFrontCameraSelector(mockFrontCameraSelector);
androidCameraCamerax.setProcessCameraProvider(mockProcessCameraProvider);

//Mock calls to native platform
when(mockProcessCameraProvider.getAvailableCameraInfos())
.thenAnswer((_) async => [mockBackCameraInfo, mockFrontCameraInfo]);
when(mockBackCameraSelector.filter([mockFrontCameraInfo]))
.thenAnswer((_) async => []);
when(mockBackCameraSelector.filter([mockBackCameraInfo]))
.thenAnswer((_) async => [mockBackCameraInfo]);
when(mockFrontCameraSelector.filter([mockBackCameraInfo]))
.thenAnswer((_) async => []);
when(mockFrontCameraSelector.filter([mockFrontCameraInfo]))
.thenAnswer((_) async => [mockFrontCameraInfo]);
when(mockBackCameraInfo.getSensorRotationDegrees())
.thenAnswer((_) async => 0);
when(mockFrontCameraInfo.getSensorRotationDegrees())
.thenAnswer((_) async => 90);

final List<CameraDescription> cameraDescriptions =
await androidCameraCamerax.availableCameras();

expect(cameraDescriptions.length, returnData.length);
for (int i = 0; i < returnData.length; i++) {
final Map<String, Object?> typedData =
(returnData[i] as Map<dynamic, dynamic>).cast<String, Object?>();
final CameraDescription cameraDescription = CameraDescription(
name: typedData['name']! as String,
lensDirection: (typedData['lensFacing']! as String) == 'front'
? CameraLensDirection.front
: CameraLensDirection.back,
sensorOrientation: typedData['sensorOrientation']! as int,
);
expect(cameraDescriptions[i], cameraDescription);
}
});
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
// Mocks generated by Mockito 5.3.2 from annotations
// in camera_android_camerax/test/android_camera_camerax_test.dart.
// Do not manually edit this file.

// ignore_for_file: no_leading_underscores_for_library_prefixes
import 'dart:async' as _i3;

import 'package:camera_android_camerax/src/camera_info.dart' as _i4;
import 'package:camera_android_camerax/src/camera_selector.dart' as _i5;
import 'package:camera_android_camerax/src/process_camera_provider.dart' as _i2;
import 'package:mockito/mockito.dart' as _i1;

// ignore_for_file: type=lint
// ignore_for_file: avoid_redundant_argument_values
// ignore_for_file: avoid_setters_without_getters
// ignore_for_file: comment_references
// ignore_for_file: implementation_imports
// ignore_for_file: invalid_use_of_visible_for_testing_member
// ignore_for_file: prefer_const_constructors
// ignore_for_file: unnecessary_parenthesis
// ignore_for_file: camel_case_types
// ignore_for_file: subtype_of_sealed_class

/// A class which mocks [ProcessCameraProvider].
///
/// See the documentation for Mockito's code generation for more information.
class MockProcessCameraProvider extends _i1.Mock
implements _i2.ProcessCameraProvider {
MockProcessCameraProvider() {
_i1.throwOnMissingStub(this);
}

@override
_i3.Future<List<_i4.CameraInfo>> getAvailableCameraInfos() =>
(super.noSuchMethod(
Invocation.method(
#getAvailableCameraInfos,
[],
),
returnValue: _i3.Future<List<_i4.CameraInfo>>.value(<_i4.CameraInfo>[]),
) as _i3.Future<List<_i4.CameraInfo>>);
}

/// A class which mocks [CameraSelector].
///
/// See the documentation for Mockito's code generation for more information.
class MockCameraSelector extends _i1.Mock implements _i5.CameraSelector {
MockCameraSelector() {
_i1.throwOnMissingStub(this);
}

@override
_i3.Future<List<_i4.CameraInfo>> filter(List<_i4.CameraInfo>? cameraInfos) =>
(super.noSuchMethod(
Invocation.method(
#filter,
[cameraInfos],
),
returnValue: _i3.Future<List<_i4.CameraInfo>>.value(<_i4.CameraInfo>[]),
) as _i3.Future<List<_i4.CameraInfo>>);
}

/// A class which mocks [CameraInfo].
///
/// See the documentation for Mockito's code generation for more information.
class MockCameraInfo extends _i1.Mock implements _i4.CameraInfo {
MockCameraInfo() {
_i1.throwOnMissingStub(this);
}

@override
_i3.Future<int> getSensorRotationDegrees() => (super.noSuchMethod(
Invocation.method(
#getSensorRotationDegrees,
[],
),
returnValue: _i3.Future<int>.value(0),
) as _i3.Future<int>);
}