Add battery plugin #49
Conversation
packages/battery/LICENSE
Outdated
| @@ -0,0 +1,27 @@ | |||
| // Copyright 2017 Your Company. All rights reserved. | |||
There was a problem hiding this comment.
Also generally missing copyright header in the .dart|.h|.m|.java files that "links" to this LICENSE file
There was a problem hiding this comment.
Good catch, updated.
It's impossible to know which places need updating after running flutter create --plugin :(
packages/battery/README.md
Outdated
| A Flutter plugin to access various information about the battery of the device the app is running on. | ||
|
|
||
| ## Usage | ||
| To use this plugin, add url_launcher as a [dependency in your pubspec.yaml file](https://flutter.io/platform-plugins/). |
| @@ -0,0 +1,113 @@ | |||
| package io.flutter.plugins.battery; | |||
There was a problem hiding this comment.
Missing copyright header in this file
| @@ -0,0 +1,45 @@ | |||
| #include "AppDelegate.h" | |||
| @@ -0,0 +1,87 @@ | |||
| import 'dart:async'; | |||
packages/battery/LICENSE
Outdated
| @@ -0,0 +1,27 @@ | |||
| // Copyright 2017 Your Company. All rights reserved. | |||
There was a problem hiding this comment.
Also generally missing copyright header in the .dart|.h|.m|.java files that "links" to this LICENSE file
| @@ -0,0 +1,70 @@ | |||
| // Copyright 2017 The Chromium Authors. All rights reserved. | |||
packages/battery/LICENSE
Outdated
| @@ -0,0 +1,26 @@ | |||
| Copyright 2017, the Flutter project authors. All rights reserved. | |||
There was a problem hiding this comment.
Please make sure this exactly matches https://github.com/flutter/flutter/blob/master/LICENSE
There was a problem hiding this comment.
This is copied from https://github.com/flutter/plugins/blob/master/LICENSE. Is that one incorrect?
There was a problem hiding this comment.
Updated to file pointed to by @Hixie.
|
Comments addressed. PTAL. |
| return [super application:application didFinishLaunchingWithOptions:launchOptions]; | ||
| } | ||
|
|
||
| - (void)applicationWillResignActive:(UIApplication *)application { |
There was a problem hiding this comment.
I am not sure why you keep the methods below?
There was a problem hiding this comment.
Removed. I wish we wouldn't generate them in the first place...
| battery: | ||
| path: ../ | ||
|
|
||
| # For information on the generic Dart part of this file, see the |
There was a problem hiding this comment.
Consider removing the generated comments.
| @@ -0,0 +1,20 @@ | |||
| # | |||
There was a problem hiding this comment.
I think we have agreed not to maintain this file. We only have it to make CocoaPods happy.
There was a problem hiding this comment.
That's news to me :). Is that documented somewhere? There should probably be a comment on top of this file telling people to just ignore its values.
| expect(await battery.batteryLevel, 42); | ||
| }); | ||
|
|
||
| group('battery sate', () { |
| setUp(() { | ||
| methodChannel = new MockMethodChannel(); | ||
| eventChannel = new MockEventChannel(); | ||
| battery = new Battery.private(methodChannel, eventChannel); |
There was a problem hiding this comment.
[nit] The mockery shouldn't be necessary. In particular, there should be no need to expose channel use from your production code.
Channels are completely stateless objects and don't keep references to their method call handlers. This means you can test every aspect of the communication behavior of Battery just by reusing the channel name in another channel object, created by your test. Then call batteryTestChannel.setMockMethodCallHandler in place of recording mock expectations to test proper calling and result handling for outgoing method calls. Or use BinaryMessages.handlePlatformMessage for simulating incoming events on the event channel.
There was a problem hiding this comment.
I looked at the tests of the url-launcher plugin and compared to that I found the tests with mocking more readable. Also, the EventChannel doesn't seem to provide a similar mechanism for testing, so I would be back to regular mocks anyways?
|
|
||
| /// Fires whenever the battery state changes. | ||
| Stream<BatteryState> get onBatteryStateChanged { | ||
| if (_onBatteryStateChanged == null) { |
There was a problem hiding this comment.
Can't you just make the Stream<BatteryState> a static final field and rely on Dart's lazy initialization of such?
Basically my question is: do we need a Battery object at all?
Maybe we do, to be able to mock it when testing dependent libs. Otherwise those test would have to know about the channel names used in Battery.
There was a problem hiding this comment.
I do want to keep Battery as its own object to allow app developer to mock it out easily.
Converted into plugin from https://github.com/flutter/flutter/tree/master/examples/platform_channel.
Fixes flutter/flutter#10091.