[Accepted with Revisions] SDL 0087 - Sequentially Send Multiple RPCs

[theresalech]

Hello SDL community,

The review of "Sequentially Send Multiple RPCs" begins now and runs through August 29, 2017. The proposal is available here:

https://github.com/smartdevicelink/sdl_evolution/blob/master/proposals/0087-send-multiple-rpcs.md

Reviews are an important part of the SDL evolution process. All reviews should be sent to the associated Github issue at:

#263

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of SDL. When writing your review, here are some questions you might want to answer in your review:

• Is the problem being addressed significant enough to warrant a change to SDL?
• Does this proposal fit well with the feel and direction of SDL?
• If you have used competitors with a similar feature, how do you feel that this proposal compares to those?
• How much effort did you put into your review? A glance, a quick reading, or an in-depth study?
  Please state explicitly whether you believe that the proposal should be accepted into SDL.

More information about the SDL evolution process is available at

https://github.com/smartdevicelink/sdl_evolution/blob/master/process.md

Thank you,
Theresa Lech

Program Manager - Livio
theresa@livio.io

[brandon-salahat-tm]

@joeljfischer
Since this proposal leaves the actual implementation up for discussion, I wanted to share two implementations that we have used internally.

This first code block is a batch send function for doing multiple RPCs in sequence, it just recursively dispatches each RPC as the previous one completes. The error handling is excluded from this code snippet, but could be extended to meet the error handling interface in this proposal. I think this implementation could be a pretty straight translation back into Objective C

func send(tests: [SDLRPCRequest]) {
        guard tests.count > 0 else { return }
        
        var mutableTests = tests
        let test = mutableTests.popLast()! //we already verified the size, so this should be safe.
        
        ProxyManager.sharedInstance.sdlManager.send(test) { (request, response, error) in
            self.send(tests: mutableTests)
        }
        
    }

This implementation fires the RPCs off as fast as possible. It uses a Swift framework named BrightFutures to keep the syntax concise. BrightFutures wraps around NSOperationQueue, so a more verbose implementation could be translated back into Objective C. Some of the error handling has also been omitted from this example, partially due to the fact the "Either" nature of error handling with Futures cannot be translated into Objective C easily.

func batchSend(requests: [SDLRPCRequest], withResponseHandler handler: SDLBatchResponseHandler? = .none) {
        let responseFutures = requests.map { (request) -> Future<SDLSendResponse, NoError> in
            return sendFutureWrapper(request: request)
        }
        
        responseFutures.sequence().onSuccess { (responses) in
            handler?(responses)
        }.onFailure { (fatalError) in
            NSLog("[Batch Send]: This shouldn't ever execute")
            handler?([])
        }
    }


private func sendFutureWrapper(request: SDLRPCRequest, errorHandler: SDLErrorHandler? = .none) -> Future<SDLSendResponse, NoError> {
        return Future<SDLSendResponse, NoError> { completion in
            self.send(request, withResponseHandler: { (request, response, error) in
                let sdlResponse = SDLSendResponse(originalRequest: request, response: response, error: error)
                errorHandler?.handleSDLResult(request: request, response: response, error: error)
                completion(Result(value: sdlResponse))
            })
        }
    }

One concern that I have with the shotgun approach in the 2nd example, is that this does not work well for certain RPCs, and can also cause weird behaviors across SDL implementations if too many RPCs are dispatched at the same time. I am curious what everyone's thoughts would be on setting a maximum size for the input array on the batch asynchronous send function?

I am also curious if we think it would be worth the effort to prevent the asynchronous function from sending certain RPC combinations that we know won't work well out of sequence (create and perform interaction for example). If not, perhaps we can well document some of those use cases that should be avoided?

Overall we think it is a good approach to provide both modes of batch sending.

Are there any thoughts on doing this in Android as well?

[theresalech]

The Steering Committee voted to accept this proposal with the following revisions: revise to add Android counterpart to this proposal. Additionally, when implementing, will need to investigate at what rate RPCs can be sent and received, as well as which RPCs do not need to be sent asynchronously.

[theresalech]

@joeljfischer @joeygrover please advise when a new PR has been entered to update the proposal to reflect the agreed upon revisions. I'll then merge the PR so the proposal is up to date, and enter issues in respective repositories for implementation. Thanks!

[theresalech]

Proposal has been revised to include Android. Issues have been entered:

Android
iOS

[theresalech]

Updated issue name and proposal file to remove "iOS" as this was revised to include Android as well.