Add ability to force recorders to complete before allowing step to advance for ActiveSteps

[eschramm]

Closes https://github.com/CareEvolution/MyDataHelps-iOS/issues/1437.

The root cause of this bug is that the recorders completing and adding their results to the results array races with the completion/advancement of the TimedWalkStep. While the file results do eventually write in the expected location and append their result, if the current step of the task is not the same, the result gets appended to the ORK[1]TaskViewController's _managedResults dictionary with an index that will never be reached when enumerating for the final task result returned to the delegate.

In order to prevent this from happening, we need to make the recorders are returning results deterministically and before the advancing of the step. In reviewing the ORK[1]ActiveStepViewController, it appears that the intent was for the ActiveTask to call finish which then eventually calls goForward to proceed with navigation. However, the ORK[1]TimedWalkStep (and now CE-deprecated ORK1ReactionTimeStep - the newer version does not use recorders) goes about this differently - explicitly setting continueEnabled to YES to show a "Next" button. The result of this is that the goForward is called at the ORK1StepViewController level inverting the order of operations inside the ORK[1]ActiveTaskViewController. This means the step advances before finish is called and the recorders end up writing to a unique key (due to the stack count of identifiers at that time) that will not match the expected count when the step has not advanced.

Consequently, at the same time, it was found that Consumers was not deserializing the explicitly included recorder file inside the ORK[1]RangeOfMotionResult. It seems gratuitous to have to vend that FileResult inside the custom structure versus letting ORK[1]ActiveTask handle the recorders as is and funnel all recorder actions to happen there where we could more directly prevent the race condition.

The race condition an be short circuited with a simple async with delay call on the main queue via GCD, but it's not truly deterministic. Originally, I attempted to force any call to finish to be gated by validating all the recorders have completed, but this now affects ANY subclass of ActiveTask greatly increasing the testing surface area of the change. Instead, the approach presented here limits the scope by adding a BOOL to the ORK[1]ActiveStep hierarchy and only gating if this is set.

This technique is used to improve the RangeOfMotion results - delegating the handling of recorders to the ORK[1]ActiveTaskViewController. This would allow more flexibility to add additional recorders in the future using a more standard architecture.

The ORK[1]TimedWalk still required more work to capture the events and force a determined order to let the ActiveStepViewController handle the recorders. Now, the TimedWalkStepViewController will capture the goForward call from the Next button and stop the recorders, then call directly into goForwardOnceRecordersHaveCompleted: which preserves the current behavior of not calling finish directly, but forcing the recorders to complete before advancing the step.

Testing:

Testing the following "predefined" ActiveTasks should demonstrate consistent inclusion of the FileResults in the export for the following Tasks in MDHD:

• Range of Motion (Knee)
  • Device Motion recorder
• Range of Motion (Shoulder)
  • Device Motion recorder
• Timed Walk
  • Accelerometer recorder
  • Device Motion recorder
  • Location recorder
  • Pedometer recorder
    By performing each task, and generating an export you should see a file per included recorder (as defined above).

Regression testing to ensure no loss of function should also be performed on other ActiveTasks such as:

• Stroop
  • no recorders
• Short Walk (Gait and Balance)
  • Accelerometer recorder
  • Device Motion recorder
  • Pedometer recorder

[mmertsock]

LGTM 🚢

[mmertsock]

Minor: I think it makes more sense to put this NSLog statement before the dispatch_after rather than inside the dispatch block

[eschramm]

Good catch!