Data collection appears to run on a non-serial dispatch queue, but there doesn't appear to be any thread safety built into any of the collectors. Perform an audit for thread safety.

To make AppCore useable without needing to link the whole AppCore framework and with one's own AppDelegate, not a subclass of APCAppDelegate, some refactoring has to happen. Before I dig myself in I wanted to note a few ideas and see if it's a good way to go. I'll raise additional issues for specific things, hoping to collect some thoughts on the general refactoring direction here.

Untangle App Delegate

Ties to the specific implementation of APCAppDelegate is everywhere, often for sensible things like getting ahold of the data substrate instance, but not always. A possible way would be to create controller classes for the individual AppCore "modules", APCOnboarding and APCDataSubstrate are a start already. App delegates would have to implement a protocol so that they can return the controller instance to be used in the app. This would help modularizing and at the same time allow to clean up the initializationOptions dictionary.

The modules I currently see are:

• Data Substrate (with the current user, see below)
• Onboarding & Consenting
• Scheduling
• Data Collection

Untangle APCUser

The current implementation of the class representing the study subject, APCUser, is strongly tied to the Bridge SDK. In fact its sign-up and withdraw methods are only defined in the user's Bridge category. The current CoreData stack is likely to be of use to many ResearchKit developers, but still there will be studies with minimal on-device data that only need a name and the date of consent stored on device, possibly in the keychain.

I think the nicest way would be to convert APCDataSubstrate and APCUser into categories and make the current models implementations of those, for those who want to use them out of the box. The Bridge extended user should be a subclass of the current APCUser class, not being able to use methods not defined in the superclass/protocol.

Per Yuan Zhu's comment: add unit tests for the utility methods in the NSDate+Helper category.

#import <APCAppCore/APCActivitiesViewSection.h>

Might need to be added to the main .h file.

The sign-in view controller still handles many onboarding tasks by itself instead of using the manager, such as posting notifications. It should be updated, especially the signInSuccess method. I would give it a shot but don't have the means to test sign-in.

During initialization, APCDataSubstrate initializes its CoreData stack and a HKStore handle. These methods are defined in the categories CoreData and HealthKit, which makes for neat code separation but technically is not suitable for a category.

I'm getting layout constraint warnings like below (APCStudyOverviewCollectionViewController) for many AppCore view controllers out of the box. That is with no changes on my part.

Has anyone else seen these?

2015-07-22 15:18:00.452 myApp[3046:271833] Unable to simultaneously satisfy constraints.
Probably at least one of the constraints in the following list is one you don't want. Try this: (1) look at each constraint and try to figure out which you don't expect; (2) find the code that added the unwanted constraint or constraints and fix it. (Note: If you're seeing NSAutoresizingMaskLayoutConstraints that you don't understand, refer to the documentation for the UIView property translatesAutoresizingMaskIntoConstraints)
(
"<NSLayoutConstraint:0x7fa546014d30 V:[UIView:0x7fa546012cd0(343)]>",
"<NSLayoutConstraint:0x7fa5460153e0 UIView:0x7fa546012cd0.top == UIView:0x7fa546012bc0.topMargin - 8>",
"<NSLayoutConstraint:0x7fa546015480 UIView:0x7fa546012bc0.bottomMargin == UIView:0x7fa546012cd0.bottom - 8>",
"<NSAutoresizingMaskLayoutConstraint:0x7fa546018090 h=--& v=--& V:[UIView:0x7fa546012bc0(50)]>"
)

Will attempt to recover by breaking constraint
<NSLayoutConstraint:0x7fa546014d30 V:[UIView:0x7fa546012cd0(343)]>

Make a symbolic breakpoint at UIViewAlertForUnsatisfiableConstraints to catch this in the debugger.
The methods in the UIConstraintBasedLayoutDebugging category on UIView listed in <UIKit/UIView.h> may also be helpful.

Similar to issue #16 , the onboarding process, especially APCOnboarding and APCOnboardingTask, import all the things plus APCAppDelegate.h.

To make it usable with only parts of the onboarding process the header imports need to be cleaned up and some dependencies removed.

Similar to #47, code used to setup and configure onboarding could be moved to a onboarding manager class. This would free up the app delegate and allow for easy customization by subclassing. For simple tie in to the app, AppCore may ask the app delegate, via protocol, for the onboarding manager.

I'm not sure if onboarding should be rolled into consenting or stay separate. It seems to me keeping them separate is desirable so as to not create a huge manager class with slightly different responsibilities.

Also cf. #41

Constants in APCAppDelegate should at the very least have an "APC" prefix if writing them into the application's user defaults domain rather than a library-specific domain.

There’s a new tone audiometry active task in ResearchKit, so it would be useful to have an audiogram dashboard item to visualize the results.

Per this comment from @jwe-apple, replace the FOUNDATION_EXPORT macro with an equivalent macro declared inside the project.

Code uses the following comparison:

self.user.sharedOptionSelection != [NSNumber numberWithInteger:SBBConsentShareScopeNone]

This looks unlikely to be what's intended.

In the Asthma app we've observed crashes when responding to a HealthKit observer query.

It looks like the consentDate could be nil at APCAppDelegate.m lines 730ff:

                    NSUInteger anchorForSampleType = [[NSUserDefaults standardUserDefaults] integerForKey:sampleType.identifier];
                    NSPredicate *predicate = nil;
                    NSDate *consentDate = self.dataSubstrate.currentUser.consentSignatureDate;

                    if (anchorForSampleType == 0) {
                        predicate = [NSPredicate predicateWithFormat:@"%K >= %@",
                                     HKPredicateKeyPathStartDate,
                                     [consentDate startOfDay]];
                    }

Which would lead to an invalid predicate on the anchored object query.

Reuse the existing CLLocationManager. See PR #2

As mentioned in #41, the data substrate and especially APCUser are tightly coupled to various parts of AppCore and the Bridge SDK. It would be nice to have these as protocols but keep the existing code as implementations of those protocols. Additionally, APCDataSubstrate (as-is) should take care of the stack initialization, not the app delegate.

The app delegate's initializationOptions dictionary contains a lot of different settings pertaining to different areas, it would be nice to delegate those responsibilities out to a controller object belonging to the "module" in question, as discussed in #41.

Also, some arrays in the dictionary, such as kAppProfileElementsListKey, rely on an NSNumber representation of enum values. This is weird to use from Swift, where you need to wrap those in something like

NSNumber(unsignedLong: enumCase.rawValue)

Issues like these could be solved more elegantly from a "module" controller.

Project builds and runs, but I get this error when trying to submit to the app store. Any ideas? I have been going crazy for the past day trying to get this submitted and have tried a lot of different things without any luck...

Thanks for your help

Both APCTabBarViewController and APCPasscodeViewController import all AppCore headers, making it impossible to use them outside of the whole bundle.

Re: commit 12b4dfd (Implement onboarding manager)
#49

We see that the consent pdf is wider than the view. May be an issue with css in html file, or some problem with the storyboard.

Since ZipZap and OpenSSL are included with AppCore.

Something along the lines:

static NSMutableDictionary* NSMutableDictionary createQuery(id class, id service, id account) {
    ...
}

Providing we cover all the parameters for all the methods, such as kSecReturnData, kSecReturnAttributes, kSecMatchLimit, kSecAttrAccessGroup and not constrain the query.

I'm getting - "Pointer is missing a nullability type specifier (__nonnull or __nullable)" - warnings on the APCPasscodeViewControllerDelegate protocol definition:

• (void)passcodeViewControllerDidSucceed:(APCPasscodeViewController *)viewController;
• (void)passcodeViewControllerDidFail:(APCPasscodeViewController *)viewController;

APCPermissionsManager is effectively a global variable holding a representation of the configuration of permissions, returning the state of permissions, and requesting those permissions when necessary. While userInfoItemTypes behave like permissions, they are not actually permissions.

APCPermissionsManager has the following property in the header file:

@property (copy, nonatomic) NSArray *userInfoItemTypes;

The userInfoItemTypes are used to configure view controller elements by prepareContent() methods in APCProfileViewController, APCSignUpMedicalInfoViewController, and APCSignUpGeneralInfoViewController.

A future review of this architecture might for example, add a userInfoConfiguration object to resolve this issue. Discussion in #117 recommended that neither APCPermissionsManager or APCOnboardingManager be the home for the array of userInfoItemTypes.

APCScoring fails with an 'incompatible unit' error, when I query for the Sleep Analysis metric. This is likely because Sleep Analysis us unit-less. But the method seems to require an HKUnity parameter.

The latest iPod has a motion coprocessor but is excluded from kAPHDashboardItemTypeFitness because APCDeviceHardware check hardware generation instead using availability APIs.

APCScheduleExpressionParserTests's testListParsing, testNumberParsing, and APCScheduleExpressionPointSelectorTests's testPointSelector seem to fail.

I'm seeing unpredictable behavior with APCKeychain, where APCKeychainStore.stringForKey incorrectly returns an empty value about 1/3 of the time. Where APCKeychainStore incorrectly returns empty values, I can force-close the app and see the correct values returned on subsequent queries.

I'm getting and setting keychain values via Swift, like this:

var email : String {
    get {
        if let value = APCKeychainStore.stringForKey(kEmailPropertyName) {
            return value
        }
        return ""
    }
    set {
        APCKeychainStore.setString(newValue, forKey: kEmailPropertyName)
    }
}

var password : String {
    get {
        if let value = APCKeychainStore.stringForKey(kPasswordPropertyName) {
            return value
        }
        return ""
    }
    set {
        APCKeychainStore.setString(newValue, forKey: kPasswordPropertyName)
    }
}

APCPasscodeViewController disappears when the PIN is entered correctly. If it is subsequently shown again the 4 input indicators are still blobs, not dashes.

This can be resolved by deallocating APCPasscodeViewController when PIN entry succeeds, however in the reference implementation in APCAppDelegate this is not the case as it's being retained (https://github.com/ResearchKit/AppCore/blob/master/APCAppCore/APCAppCore/Startup/APCAppDelegate.m#L975).

Hi,
When I create a new iOS project using latest of ResearchKit framework, AppCore and BridgeSDK has build errors and can't be built.

/AppCore/APCAppCore/APCAppCore/DataSubstrate/Model/APCSmartSurveyTask.m:99:68: Property 'questions' not found on object of type 'SBBSurvey *'

/AppCore/APCAppCore/APCAppCore/DataSubstrate/Model/APCSmartSurveyTask.m:438:18: Property 'detail' not found on object of type 'SBBSurveyQuestion *'

/AppCore/APCAppCore/APCAppCore/DataSubstrate/Model/APCSmartSurveyTask.m:439:33: Property 'detail' not found on object of type 'SBBSurveyQuestion *'

/AppCore/APCAppCore/APCAppCore/Startup/APCAppDelegate.m:90:31: Unknown type name 'APCDemographicUploader'

/AppCore/APCAppCore/APCAppCore/Startup/APCAppDelegate.m:90:1: Property with 'retain (or strong)' attribute must be of object type

/AppCore/APCAppCore/APCAppCore/DataSubstrate/Model/APCSmartSurveyTask.m:572:50: No known class method for selector 'scaleAnswerFormatWithMaxValue:minValue:step:defaultValue:'

In the ResearchKit, the Chart Module can't filter data by 5 days/ 1 week/ 1 month...
But AppCore can do this, (In APCGraphViewController.m)

So I would like Integrate AppCore into my app,
But I can't put files which relate Dashboard into my app,

My question is, Does is exist example app using AppCore framework?

Code used to setup and configure consenting could be moved to a new consent manager class. This would free up the app delegate and allow for easy customization by subclassing. For simple tie in to the app, AppCore may ask the app delegate, which should be adhering to a new consent manager provider protocol, for the consent manager.

cf. #41

Are there plans to include documentation on how to integrate AppCore into a fresh project—or perhaps a demo app similar in purpose to ORKCatalog, showcasing a generic app using AppCore?

It would be ideal if AppCore could be tested and built without dependencies. A test app for exercising features would also help.

The enum APCSignUpPermissionsType defines cases named like kSignUpPermissionsTypeXY, omitting the "APC" at the beginning.

There is a similar issue with APCUserInfoItemType, which defines cases with 2 different trunks:

• kAPCUserInfoItemTypeXY
• kAPCSettingsItemTypeXY

The latter would require creating a new enum, which might cause problems if the types should be mixable.

There are several auto layout conflicts in the onboarding screens, mostly labels that want to be centered, want a given width and have a fixed width from one of the edges. While the fixed width makes some sense to have English text nicely wrapped onto two lines, the fixed distance to the edge is unnecessary and creates a conflict on different screen sizes.

To keep track of last time the app was used, the app delegate sets kLastUsedTimeKey on termination/moving to background. APCTabBarViewController does the same – with the same key that it defines itself – when PIN entry succeeds.

It seems the tab controllers implementation can be removed entirely. If not it should probably be moved to the delegate method that the app delegate implements and the define should be removed.

The data verification code should not be in this repo, since the data verification server is not included.