Skip to content
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

Push notifications #582

Merged
merged 15 commits into from
Feb 1, 2018
Merged

Push notifications #582

merged 15 commits into from
Feb 1, 2018

Conversation

ricardopereira
Copy link
Contributor

@ricardopereira ricardopereira commented Feb 9, 2017

Installation

You can install Ably for iOS through CocoaPods, Carthage or manually.

Installing through CocoaPods (recommended)

Add this line to your application's Podfile:

# For Xcode 7.3 and newer
pod 'Ably', :git => 'https://github.com/ably/ably-ios.git', :branch => 'push-beta-legacy'

And then install the dependency:

$ pod install

Then you can:

 // On Objective-C
#import <Ably/Ably.h>

// On Swift
import Ably

Installing through Carthage

Add this line to your application's Cartfile:

# For Xcode 7.3 and newer
github "ably/ably-ios" "push-beta-legacy"

And then run carthage update to build the framework and drag the built Ably.framework into your Xcode project.

Manual installation

  1. Clone the push branch: git clone -b push [email protected]:ably/ably-ios.git
  2. Drag the directory ably-ios/ably-ios into your project as a group.
  3. Ably depends on SocketRocket 0.5.1; get it from the releases page and follow its manual installation instructions.
  4. Ably also depends on msgpack 0.1.8; get it from the releases page and link it into your project.

@ricardopereira ricardopereira requested a review from tcard February 9, 2017 00:08
@ricardopereira ricardopereira self-assigned this Feb 9, 2017
Source/ARTPush.m Outdated
}

if (registerCallback) {
self.device.updateToken = registerCallback(deviceDetails, nil);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tcard maybe this should be async and not waiting for a return.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In which situation you want to pass an error to the registerCallback? I don't see any case.

Copy link
Contributor

Choose a reason for hiding this comment

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

In which situation you want to pass an error to the registerCallback? I don't see any case.

In none that I can think of, really. Maybe we'll end up removing it.

maybe this should be async and not waiting for a return.

Yes, it should. It returns io T. In iOS, ) -> io T translates roughly into , callback: (T, Error)). Just like authCallback.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed 08f9d01.

Copy link
Contributor

@tcard tcard left a comment

Choose a reason for hiding this comment

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

Looking pretty good! Some things need changing though.


extern NSString *const ARTDevicePlatform;

typedef NS_ENUM(NSUInteger, ARTDeviceFormFactor) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe we talked about those enums being plain strings in the client libraries. You did it for platform so probably I wasn't clear that it affects formFactor and transportType too.

The point is that, if we add new form factors in the future, the client libraries don't need to be updated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed 64e6dab.

Source/ARTPush.h Outdated
@interface ARTDeviceToken : NSData
@end

@interface ARTUpdateToken : NSString
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is type-safe? You can't ask for type Dog and get Animals. Don't you get a compile error or at least warning when passing a NSString as ARTUpdateToken?

(Same of all other classes above.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that was an initial solution but I changed it to a typedef (581e76f#diff-e040b62ac54206ef54da9e6409623314R18). Now it's working fine. 👍

}

- (instancetype)init {
return [self initWithId:[[NSUUID new] UUIDString]];
Copy link
Contributor

Choose a reason for hiding this comment

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

OK for now, but I did mention ULID, right? It's not crucial but it's also straightforward to use instead. It's what I use in Java so it'd be nice for IDs to be uniform.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tcard Yes, you mention it but there is no standard ULID generator. I will look into the paper of ULID and try to implement it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

BTW, does the Java has it already implemented in the language itself?

Copy link
Member

Choose a reason for hiding this comment

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

I will look into the paper of ULID and try to implement it.

I don't think we should be writing our own implementation if we can help it. Is that really necessary if we can't find a lib that does it for us?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, I used a library. Sorry, I assumed there was a library for Obj-C! It doesn't seem to be the case so you can be the first to make a CocoaPod :) Or I will if you don't want to!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is an C++ library, so I created an Objective-C wrapper of it: https://github.com/whitesmith/ulid. It still need some work but it's working. I'll improve it later to continue this work.

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice!

@implementation ARTDeviceDetails

+ (instancetype)fromLocalDevice {
return [[ARTDeviceDetails alloc] init];
Copy link
Contributor

Choose a reason for hiding this comment

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

This is missing all device info, so this is TODO, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tcard I forgot about it, 👍 . So I should initialise it with the deviceId that is stored and the current updateToken, if it's in memory?

Copy link
Contributor

Choose a reason for hiding this comment

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

Right.

dictionary[@"transportType"] = devicePushDetails.transportType;

if (devicePushDetails.deviceToken) {
// Normalizing token by removing symbols and spaces
Copy link
Contributor

Choose a reason for hiding this comment

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

Why? In tests I've used the device token as-is just fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tcard You tested it converting it into a string type? The token is a Hex string, something like <12ce7dda 8032c423 8f8bd40f 3484e5bb f4698da5 8b7fdf8d 5c55e0a2 XXXXXXXX>. So I should let it be as it is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed d67ba78.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, sorry, I was wrong, this is what I did: [deviceToken hexString] using some random hexString method I found googling. So yes, removing symbols and spaces should do the same. Sorry!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done 4ab1d74.

Source/ARTPush.h Outdated
/// Register a device, including the information necessary to deliver push notifications to it.
- (void)activate;
- (void)activate:(ARTDeviceDetails *)deviceDetails;
- (void)activate:(ARTDeviceDetails *)deviceDetails registerCallback:(nullable ARTUpdateToken* (^)(ARTDeviceDetails * _Nullable, ARTErrorInfo * _Nullable))registerCallback;
Copy link
Contributor

Choose a reason for hiding this comment

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

So a problem I've encountered in Java is that this callback doesn't survive closing the app. So if you call activate, then the app closes, then the device token arrives (starting the app, I guess?) and the activation completes, the callback won't exist at that point, will it?

In Java I'm solving this with an Android's BroadcastReceiver. I don't know about iOS.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I need to check that on a real device but from what I know is that the app remains in the background when the token is getting retrieved from APNS.

Source/ARTPush.m Outdated
_device = [ARTDeviceDetails fromLocalDevice];
_state = ARTPushStateDeactivated;
_deviceTokenEmitter = [[ARTEventEmitter alloc] init];
_jsonEncoder = [[ARTJsonLikeEncoder alloc] init];
Copy link
Contributor

Choose a reason for hiding this comment

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

This should respect rest.options.useBinaryProtocol and use MsgPack if true.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed bd73596.

Source/ARTPush.m Outdated
NSData *deviceToken = [[NSUserDefaults standardUserDefaults] dataForKey:ARTDeviceTokenKey];
if (!deviceToken) {
// Waiting for device token
[_deviceTokenEmitter once:^(ARTDeviceToken *deviceToken) {
Copy link
Contributor

Choose a reason for hiding this comment

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

As I said about registerCallback, this won't survive closing the app.

Source/ARTPush.m Outdated
}

if (registerCallback) {
self.device.updateToken = registerCallback(deviceDetails, nil);
Copy link
Contributor

Choose a reason for hiding this comment

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

In which situation you want to pass an error to the registerCallback? I don't see any case.

In none that I can think of, really. Maybe we'll end up removing it.

maybe this should be async and not waiting for a return.

Yes, it should. It returns io T. In iOS, ) -> io T translates roughly into , callback: (T, Error)). Just like authCallback.

@@ -0,0 +1,47 @@
//
// ARTPushRecipient.h
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry you took all the effort to do this, but we really want to keep this "stringly-typed" so that adding new recipient types on the server don't require changing anything in the client libraries.

So ARTPushRecipient should just be NSDictionary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done 58f7eab.

Copy link
Member

@mattheworiordan mattheworiordan left a comment

Choose a reason for hiding this comment

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

Nice progress!

#import "ARTDevicePushDetails.h"

NSString *const ARTDevicePlatform = @"ios";
NSString *const ARTDeviceFormFactor = @"mobile";
Copy link
Member

Choose a reason for hiding this comment

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

Really hard-coded const? Given this could run on tablet or watch, I don't think this is right. Equally, we should really support MacOS at some point too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done 002c882.

}

- (instancetype)init {
return [self initWithId:[[NSUUID new] UUIDString]];
Copy link
Member

Choose a reason for hiding this comment

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

I will look into the paper of ULID and try to implement it.

I don't think we should be writing our own implementation if we can help it. Is that really necessary if we can't find a lib that does it for us?

}];
}

- (void)subscribeForClientId:(NSString *)clientId {
Copy link
Member

Choose a reason for hiding this comment

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

Would clientId not be implicit @tcard? Or do you think it's better to leave it as explicit as it's more flexible in future?

Copy link
Contributor

Choose a reason for hiding this comment

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

In the IDL it is implicit. But I don't know, I guess it can work like Presence#enterClient?

Copy link
Member

Choose a reason for hiding this comment

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

In the IDL it is implicit. But I don't know, I guess it can work like Presence#enterClient?

Not sure I see a use case for a client device to specify clientId other than the one implicit in the token. Servers on the other hand would do this frequently, is this API intended for server-side access as well?

Copy link
Contributor

Choose a reason for hiding this comment

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

Servers on the other hand would do this frequently, is this API intended for server-side access as well?

No, they would use the admin API most likely.

Copy link
Member

Choose a reason for hiding this comment

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

Well I would say that we shouldn't encourage people to try and register their device for a clientId that doesn't match their token, so if we want to offer this we should perhaps have an overloaded method where by default clientId is not required.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we should perhaps have an overloaded method where by default clientId is not required

Done 2c68fa9.

@ricardopereira ricardopereira force-pushed the push branch 4 times, most recently from f56247e to 4d2d686 Compare February 20, 2017 15:16
@ricardopereira
Copy link
Contributor Author

@tcard PushAdmin is done. So, the final step is activation state machine? /cc @mattheworiordan

@ricardopereira ricardopereira force-pushed the push branch 2 times, most recently from 077f20b to 002c882 Compare February 22, 2017 14:24
Source/ARTPush.m Outdated
dispatch_once(&once, ^{
// Error: ARTPush.m:62:81: Instance variable '_httpExecutor' accessed in class method
//activationMachineInstance = [[ARTPushActivationStateMachine alloc] init:_httpExecutor];
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tcard ARTPushActivationStateMachine should be a singleton right? I need to have in mind which data the state machine needs to perform is work. Does it need more than the local device?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, it should be a singleton. The persistent data it needs is the current state, the pending events, the device ID (which, once generated, doesn't change), the device's update token and the APNs device token. Also, whether activate/deactivate were called with custom registration or not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tcard I can do requests without the auth token or key, right? I don't really need the Rest instance. So, I can create the singleton with no dependencies.

Copy link
Contributor

Choose a reason for hiding this comment

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

@ricardopereira No, you can't, you need to authenticate in order to register, update the registration, and unregister. Unless the user provides the custom callback, of course.

@ricardopereira
Copy link
Contributor Author

@tcard I think this is done.

@ricardopereira ricardopereira changed the title WIP: Push notifications Push notifications Mar 8, 2017
tcard
tcard previously requested changes Mar 20, 2017
Copy link
Contributor

@tcard tcard left a comment

Choose a reason for hiding this comment

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

Sorry I couldn't take a look at this sooner!

@property (nullable, nonatomic) NSString *clientId;
@property (nonatomic, readonly) NSString *platform;
@property (nonatomic, readonly) NSString *formFactor;
@property (nullable, nonatomic) NSDictionary<NSString *, NSString *> *metadata;
Copy link
Contributor

Choose a reason for hiding this comment

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

Non-nullable. It should be an empty map if not present.

extern NSString *const ARTDevicePushTransportType;

typedef NS_ENUM(NSUInteger, ARTDevicePushState) {
ARTDevicePushStateInitialized,
Copy link
Contributor

Choose a reason for hiding this comment

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

There's no initialized state. It should default to active.


@interface ARTLocalDevice : ARTDeviceDetails

@property (nonatomic, readonly) ARTDeviceToken *registrationToken;
Copy link
Contributor

Choose a reason for hiding this comment

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

This shouldn't be public.


@property (nonatomic, readonly) ARTDeviceToken *registrationToken;

- (instancetype)init;
Copy link
Contributor

Choose a reason for hiding this comment

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

Not public.

@property (nonatomic, readonly) ARTDeviceToken *registrationToken;

- (instancetype)init;
+ (ARTLocalDevice *)local;
Copy link
Contributor

Choose a reason for hiding this comment

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

Also not public. People should only get to the local device through rest.device().

- (instancetype)init:(id<ARTHTTPAuthenticatedExecutor>)httpExecutor withChannel:(ARTChannel *)channel;

- (void)subscribe;
- (void)subscribeDevice:(ARTDeviceId *)deviceId;
Copy link
Contributor

Choose a reason for hiding this comment

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

This and unsubscribeDevice shouldn't take an argument but implicitly use the local device.

@property (nonatomic, readonly) NSString *channelName;

- (instancetype)init NS_UNAVAILABLE;
- (instancetype)initWithDeviceId:(NSString *)deviceId andChannel:(NSString *)channelName;
Copy link
Contributor

Choose a reason for hiding this comment

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

In the spec, those are static methods named forDevice and forClientId, so we should stick to those.


@property (nonatomic, readonly) NSString *deviceId;
@property (nonatomic, readonly) NSString *clientId;
@property (nonatomic, readonly) NSString *channelName;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be just channel.


@interface ARTPushChannelSubscription : NSObject

@property (nonatomic, readonly) NSString *deviceId;
Copy link
Contributor

Choose a reason for hiding this comment

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

Nullable, and clientId too.

NSString *tokenBase64 = [tokenData base64EncodedStringWithOptions:0];
[request setValue:[NSString stringWithFormat:@"Bearer %@", tokenBase64] forHTTPHeaderField:@"Authorization"];
request.HTTPMethod = @"PUT";
request.HTTPBody = [[_httpExecutor defaultEncoder] encode:@{
Copy link
Contributor

Choose a reason for hiding this comment

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

This will just save the device token. It should send the whole thing.

#elif TARGET_OS_WATCH
NSString *const ARTDeviceFormFactor = @"watch";
#elif TARGET_OS_SIMULATOR
NSString *const ARTDeviceFormFactor = @"simulator";
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't the simulator simulate other form factors?

@tcard
Copy link
Contributor

tcard commented Apr 10, 2017

@ricardopereira This was getting urgent last week so I had to take over this, so don't worry about it. I'll push my changes (hopefully) shortly.

@ricardopereira
Copy link
Contributor Author

@tcard What about tests? Should I add them?

@tcard
Copy link
Contributor

tcard commented Apr 10, 2017

@ricardopereira Not yet, I think. The code we want to test is still in flux.

@mattheworiordan
Copy link
Member

The code we want to test is still in flux.

@tcard could we not add test coverage over the REST API at least? That's not changing much is it? See https://github.com/ably/ably-ruby/pull/115/files for a reasonable set of tests over this functionality or of course ably/ably-js@6294597

@ricardopereira
Copy link
Contributor Author

@tcard I will change the way how the user adds his custom register/deregister callback because there is no way to test it nicely using the AppDelegate: https://github.com/ably/ably-ios/blob/push/Source/ARTPushActivationStateMachine.m#L207-L224. I didn't thought about it while I was coding 😒

I will change it and use the NSNotificationCenter. The user can implement it like:

NSNotificationCenter.default.addObserver(
    UIApplication.shared,
    selector: #selector(AppNotifications.refreshNotification(_:)), //<-- Custom callback
    name: ARTCustomRegisterCallbackNotificationName, //<-- identifier
    object: nil
)

It gives the user more flexibility and a clean AppDelegate.

@tcard
Copy link
Contributor

tcard commented Apr 25, 2017

@ricardopereira Does that allow you to check whether the user added an observer or?

In any case, I don't think the former way is so hard to test. Instead of hardcoding [UIApplication sharedApplication].delegate, you can define a method that returns it and you can replace for testing using the hooks thing.

@ricardopereira
Copy link
Contributor Author

Nevermind, I can't detect if an ARTCustomRegisterCallbackNotificationName as registered observers.

ricardopereira and others added 6 commits October 2, 2017 11:55
ARTPushNotifications interface

Realtime: integrate Push type

Rest: integrate Push type

Push.activate

Push.publish

 - add PushRecipient protocol

PushChannel subscribe and unsubscribe

Fix registerCallback

Remove warnings

 - unused methods

Fix FormFactor

Fix: use device token as Hex string

Fix: should respect the default encoder

 - useBinaryProtocol

Remove ARTPushRecipient protocol and types

Add ULID pod

Use ULID as DeviceId

Push.deactivate

Load local device

Push: update device

PushChannel.subscriptions

Update ULID pod

PushAdmin

Update ULID pod

Remove warnings

JSON encoder: normalize deviceToken hex string

Fix: Push.publish request body

Update device forms

Activation State Machine: init

Activation State Machine: registration

Removed ablyPushAuthKey, ablyPushAuthToken and ablyPushClientId from push delegate

Add Rest argument on Push delegate

Fix things for push, and adapt to latest API.

Push: add registering delegate methods for Realtime too.

Add new private headers to modulemap.

Fix rebase merge conflicts.

Recalculate Podfile.lock checksum.
With a static method, the constructed class is always the parent
abstract class ARTPushActivationState, instead of the desired child
class.

Fixes #628.
tcard and others added 7 commits October 13, 2017 15:07
* MockHTTPExecutor

* PushAdmin

* Move `publish` method to Admin

* Add `get` method in PushDeviceRegistrations

* Should use response encoder type

* Fix save channel subscription

* Fix save device registration

* Better errors information

* Rename publish notification argument to data to conform specs

* Add equatable to DeviceDetails

* Fix DeviceRegistrations.get

* Add validations to PushAdmin.publish

* Fix RHS1a

* Fix RHS1b1

* Fix PushAdmin implementation

* Fix ambiguous reference to member 'dataTask(with:completionHandler:)'

* fixup! Fix PushAdmin implementation

* CI test

* fixup! Fix PushAdmin implementation
@ricardopereira
Copy link
Contributor Author

ricardopereira commented Jan 17, 2018

Test suite (https://travis-ci.org/ably/ably-ios/builds/325283018#L3028) is failing because of #669.

ricardopereira and others added 2 commits February 1, 2018 01:04
* Initial Push types

* Update project

* ARTPushNotifications interface

* Realtime: integrate Push type

* Rest: integrate Push type

* Push.activate

* fixup! Push.activate

* Push.publish

 - add PushRecipient protocol

* fixup! Push.publish

* PushChannel subscribe and unsubscribe

* Fix registerCallback

* Remove warnings

 - unused methods

* Fix FormFactor

* Fix: use device token as Hex string

* Fix: should respect the default encoder

 - useBinaryProtocol

* Remove ARTPushRecipient protocol and types

* Add ULID pod

* Use ULID as DeviceId

* Push.deactivate

* Load local device

* Push: update device

* PushChannel.subscriptions

* Update ULID pod

* PushAdmin

* Update ULID pod

* Remove warnings

* JSON encoder: normalize deviceToken hex string

* Fix: Push.publish request body

* Update device forms

* Activation State Machine: init

* Activation State Machine: registration

* Removed ablyPushAuthKey, ablyPushAuthToken and ablyPushClientId from push delegate

* Add Rest argument on Push delegate

* tmp

* Fix things for push, and adapt to latest API.

* Initial State Machine tests

* ARTLocalDeviceStorage

* Fix: ARTPushActivationState

* Storage should synchronize UserDefaults

* Storage Keys accessible on tests

* State Machine tests

* ActivationStateMachine: custom delegate and other fixes

* PushActivationStateMachine

* Implement Device storage

* Fix headers

* Fix tests
@funkyboy
Copy link
Contributor

funkyboy commented Feb 1, 2018

@ricardopereira Please let me know if tests pass also on iOS10 and 11.
Working on this now, so hopefully things will be more streamlined soon.

@ricardopereira
Copy link
Contributor Author

ricardopereira commented Feb 1, 2018

Done.

screen shot 2018-02-01 at 09 52 26

screen shot 2018-02-01 at 10 09 55

@@ -101,7 +101,7 @@ - (void)persist {

- (void)deviceRegistration:(ARTErrorInfo *)error {
#ifdef TARGET_OS_IOS
ARTLocalDevice *local = _rest.device;
ARTLocalDevice *local = _rest.device_nosync;

if (![[UIApplication sharedApplication].delegate conformsToProtocol:@protocol(ARTPushRegistererDelegate)]) {
[NSException raise:@"ARTPushRegistererDelegate must be implemented on AppDelegate" format:@""];
Copy link
Contributor

Choose a reason for hiding this comment

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

Wondering: can this fix or impact on this? #673

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@funkyboy If the user is using the push branch, then yes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was a fix for #654.

@funkyboy funkyboy merged commit 7012435 into master Feb 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants