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

[Rejected] SDL 0133 - Enhanced iOS Proxy Interface #379

Closed
theresalech opened this issue Jan 24, 2018 · 14 comments
Closed

[Rejected] SDL 0133 - Enhanced iOS Proxy Interface #379

theresalech opened this issue Jan 24, 2018 · 14 comments

Comments

@theresalech
Copy link
Contributor

Hello SDL community,

The review of "Enhanced iOS Proxy Interface" begins now and runs through January 30, 2018. The proposal is available here:

https://github.com/smartdevicelink/sdl_evolution/blob/master/proposals/0133-EnhancediOSProxyInterface.md

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

#379

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

@joeljfischer
Copy link
Contributor

This is an extensive proposal, some of which I agree with (the core idea), some I don't (a lot of the details), and I think a lot of expansion, discussion, prototyping, API design, and work on this is necessary before we can really say that it's good to be approved. This is a starting point, I understand that, but I don't think that we can really approve a starting point that is this nascent. With that said, here are my points and concerns.

  • I think this would have to release alongside a comparable Android version. I'm not sure if we can really discuss and implement this feature until we have an equivalent Android proposal, discussion, and implementation. The current divergence between Android and iOS libs is probably too wide as it is, but this would diverge them so far apart that knowing SDL for one platform would be essentially useless for working on the other.

  • I don't think this proposal is nearly comprehensive enough. There are only some ideas about the API, but nothing concrete. Furthermore, it only seems to cover, essentially, Shows and SoftButtons (which are actually a part of Show). Is that intentional? If so, we are merely helping with significantly less complex portions of the app than, say, Choice Interaction Sets. I would rather see a manager for dealing with the insanely difficult to do correctly Choice Interactions and allow the Show Manager proposal referenced to be brought back up as a temporary help. This proposal doesn't address choice sets, the menu, alerts, etc.

  • Some of the complexity this paints over is rather useful, making the system, in my opinion, too simple. For example, given the API ideas present, it would impossible to only have the forward & play buttons active while the back button is not present. It appears to be impossible to change a soft button due to, for example, a button press (due to buttons being returned to a delegate request). Among other things.

  • There's no discussion of how things like UIButton map to soft buttons. In addition, I think this is a mistake, and SDL-specific objects should be used. If we use native objects, we are subject to (1) Apple changes, (2) mistaken expectations due to "missing" functionality, and (3) reduced functionality due to SDL features that don't map into UIKit features. The need for a developer to learn how to use an "SDLButton" is not so severe that we should drop into UIKit.

  • I don't agree with the proposed delegate pattern, as I believe it's too limiting. Why not just have properties on the "ViewController"s that the developer can set into? (This bleeds into the below point)

  • There's no discussion of what these "ViewController" objects look like. Are they the developer's actual ViewControllers? That's what seems to be implied without being stated. I think that's a mistake (it contributes to the Massive View Controller problem and is far too limiting for complex apps and any app where their UI won't match up with SDL 1:1), and "ViewController"-like objects should be given to the developer to instantiate / subclass and pass into a simple presentation method on SDLManager. If we really want to move to MVC, we should work on making the head unit screen presented as the V, the library more of the C, and enabling devs to more easily hook in their M.

These are just a few of the issues, concerns, and notes I have on this proposal. This is such a large topic that in my opinion, it's hard to discuss without an extremely thorough proposal with a nailed down API or in a workshop setting. I support the vision of making the SDL library simpler for developers and painting over the complexity when possible. The broad vision of this is good and will work with additional work, though I do believe there are more pressing areas begging for simplification.

@theresalech
Copy link
Contributor Author

The Steering Committee voted to defer this proposal, keeping it in review until our next meeting on 2018-02-06 to allow the author to respond to the comments provided and also to allow other members to review and comment on the proposal. The Project Maintainer stressed that this is a very important proposal for all members to review, as it involves a substantial change for both proxies. It was noted that after this additional week of review, we will likely take the conversation offline to discuss in greater detail in a workshop setting.

@brandon-salahat-tm
Copy link
Contributor

@joeljfischer

I think this would have to release alongside a comparable Android version. I'm not sure if we can really discuss and implement this feature until we have an equivalent Android proposal, discussion, and implementation. The current divergence between Android and iOS libs is probably too wide as it is, but this would diverge them so far apart that knowing SDL for one platform would be essentially useless for working on the other.

Agreed. I think @eslerner sent over the draft she is working on for Android. I think this should be in a separate proposal since the improved Android interface should be tailored to conform to the standard Android design patterns/practices.

I don't think this proposal is nearly comprehensive enough. There are only some ideas about the API, but nothing concrete. Furthermore, it only seems to cover, essentially, Shows and SoftButtons (which are actually a part of Show). Is that intentional? If so, we are merely helping with significantly less complex portions of the app than, say, Choice Interaction Sets. I would rather see a manager for dealing with the insanely difficult to do correctly Choice Interactions and allow the Show Manager proposal referenced to be brought back up as a temporary help. This proposal doesn't address choice sets, the menu, alerts, etc.

This proposal aims to address the main display interface for SDL, which is Show and it's satellite implementation details (so perhaps the proposal name should be tweaked). It is based on consistent feedback that we have received from application partners. There has been an expressed desire to see SDL interfaces that more closely resemble the native design patterns, but Show has been the interface most frequently called out. The interaction sets also need an enhanced interface, but that seemed to be a lot of work independent to improving the display interface, and probably needs its own proposal.

Some of the complexity this paints over is rather useful, making the system, in my opinion, too simple. For example, given the API ideas present, it would impossible to only have the forward & play buttons active while the back button is not present. It appears to be impossible to change a soft button due to, for example, a button press (due to buttons being returned to a delegate request). Among other things.

In the proof of concepting work this proposal is based on, we tried a few different approaches with media and other non-soft buttons. An example is given in the proposal of what a media delegate could look like, but that's not to say that the controls couldn't be mated to UIButtons like what is proposed for soft buttons.

Softbutton highlighted state could be mated to the UIButton selected state, which can be toggled if needed. The goal of this proposal is not to make this the only interface to Show, but to provide a simple and intuitive interface to happy-path/common SDL integrations that don't want or need to utilize some of the lower level features/complexities.

There's no discussion of how things like UIButton map to soft buttons. In addition, I think this is a mistake, and SDL-specific objects should be used. If we use native objects, we are subject to (1) Apple changes, (2) mistaken expectations due to "missing" functionality, and (3) reduced functionality due to SDL features that don't map into UIKit features. The need for a developer to learn how to use an "SDLButton" is not so severe that we should drop into UIKit.

App integrations that want to directly use SDLButton or other SDL features that do not easily map into UIKit can still use the lower level interface, for example the proposed Show Manager. However the desire has been expressed many times for an integration option that maps much cleaner to native components/principles. It is not very difficult to map most SDLButton events/functionality to UIButton. I believe a few methods of doing this were discussed in one of the haptic proposals previously, I will try to find some code snippets.

There's no discussion of what these "ViewController" objects look like. Are they the developer's actual ViewControllers? That's what seems to be implied without being stated. I think that's a mistake (it contributes to the Massive View Controller problem and is far too limiting for complex apps and any app where their UI won't match up with SDL 1:1), and "ViewController"-like objects should be given to the developer to instantiate / subclass and pass into a simple presentation method on SDLManager. If we really want to move to MVC, we should work on making the head unit screen presented as the V, the library more of the C, and enabling devs to more easily hook in their M.

The proposal suggests that the proxy could provide ViewControllers for the common SDL templates (the ones specified in the developer guides), but also leave the delegate public should an app developer want to utilize it directly via a Swift extension or similar. The delegate could be made private and only SDL View Controllers exposed to integrations, but leaving it public was based on some of the feedback we have received.

These are just a few of the issues, concerns, and notes I have on this proposal. This is such a large topic that in my opinion, it's hard to discuss without an extremely thorough proposal with a nailed down API or in a workshop setting. I support the vision of making the SDL library simpler for developers and painting over the complexity when possible. The broad vision of this is good and will work with additional work, though I do believe there are more pressing areas begging for simplification.

Agreed that the API needs to be nailed down in a workshop setting. The proposal was left somewhat conceptual/open ended because we believed that needed to happen. Most likely a lot of the SDL APIs need this kind of attention as discussed earlier in this comment. We made this proposal for enhancing the Show interface first, because that is what we have received the most feedback on.

@ghost
Copy link

ghost commented Feb 1, 2018

This remebers me of a concept from Ford to improve the SDL libraries. It's little more than a year ago... The concept was to mimic the design of the native OS (Android or iOS).

For Android there exist a lot of code that could be used for that.

API lifecycle (pretty much what iOS managers are doing): smartdevicelink/sdl_java_suite#299
API lock screen (kind of part of the lifecycle): smartdevicelink/sdl_java_suite#300
API view (SdlActivity etc. mimic the Android software design): smartdevicelink/sdl_java_suite#301
API permission manager smartdevicelink/sdl_java_suite#302

A similar (but conceptional) approach was done for iOS. It shows a concept which is similar to UIApplication, UIViewController and UIViews (and subclasses). See attached PDF for details ios-enhanced-framework.pdf

It was never proposed to the SDLC as it's more revolutionary to the libraries. I tried to divide the concept into smaller pieces but it makes sense to review the entire concept instead.

The concept might need to be updated but it's still valid and it's already at a stage for future workshops.

@joeygrover
Copy link
Member

@kshala-ford yea the initial discussions we had for the "enhanced frameworks" were along the same lines as this proposal. However, we've asked for evolution proposals based on those works in the past with no response or timeline as to when they might be included. And at this point they are seemingly stale.

Also, in my opinion, they were overengineered and essentially created an entire new framework over the existing framework that would greatly increase the maintenance burden. I think there were some good ideas there that we can leverage, but I believe we should aim for a simpler solution that provides what developers need, not the exhaustive functionality that was initially created.

@joeljfischer
Copy link
Contributor

This proposal aims to address the main display interface for SDL, which is Show and it's satellite implementation details (so perhaps the proposal name should be tweaked). It is based on consistent feedback that we have received from application partners. There has been an expressed desire to see SDL interfaces that more closely resemble the native design patterns, but Show has been the interface most frequently called out. The interaction sets also need an enhanced interface, but that seemed to be a lot of work independent to improving the display interface, and probably needs its own proposal.

Agreed, however, I would argue that the difficulty of working with Choice Sets is greater than Shows / Soft Buttons, especially if the Show Manager / Image Upload Manager are implemented. That is where we should focus our increase in complexity and work, in my opinion.

The goal of this proposal is not to make this the only interface to Show, but to provide a simple and intuitive interface to happy-path/common SDL integrations that don't want or need to utilize some of the lower level features/complexities.

The issue with this as I currently understand it is that for anything non-trivial, this whole approach won't work well enough as is.

App integrations that want to directly use SDLButton or other SDL features that do not easily map into UIKit can still use the lower level interface, for example the proposed Show Manager.

I think this is a mistake. We should not have two primary integration methods, only one that unfolds in complexity as more features are needed. To implement, document, and maintain two wholly different in style integrations is asking for trouble.

However the desire has been expressed many times for an integration option that maps much cleaner to native components/principles. It is not very difficult to map most SDLButton events/functionality to UIButton. I believe a few methods of doing this were discussed in one of the haptic proposals previously, I will try to find some code snippets.

I think there is a big difference between something that maps better to native principles and something that uses the native components in a way they are not designed to be used. A developer should not have an issue with using a custom object, especially if it's well – and specifically – designed for the platform. It's only going to confuse people if they're using native UIKit components in an environment they weren't meant for. Our UI is not UIKit, and it shouldn't use UIKit components directly. If we want to apply similar principles insofar as they map, without straining them then I'm for that. If our UI architecture diverges too sharply for UIKit (which it may) then I would argue for building a more comprehensible UI system for sdl_ios that maps well to the actual UI architecture of SDL, not to try to force UIKit architecture where it does not map. This is only asking for trouble, as it becomes less comprehensible and increases our maintenance burden.

My counter-proposal would look more like this:

  • SDL-defined SDLViewControllers for each template. The developer subclasses one and this defines the template that will be used.
  • A dataSource-like component for soft buttons, with a request for the number of buttons, and a provider for each. These would use the SDLSoftButtonWrapper class of the Show Manager Proposal in order to handle multiple states of buttons (A reloadButtons should be implemented to update the buttons).
  • The media template would have an additional dataSource for subscribe buttons. A dev would return an array of which ones they want to subscribe to(?) I'm not sure how handlers for each would be registered to work with that.
  • Text fields and images are available as properties directly on the view controller class. When they are modified either (1) They are immediately queued to be updated, (2) A coalescing timer will send all updates 0.10 seconds after the first change occurs, or (3) The developer manually tells it to update with an update call.
  • The Show Manager runs under the hood of all of this.
  • The developer passes an initial ViewController to the proxy, and any new ones they pass will automatically cause a SetDisplayLayout. Pushing and popping could be handled.

This would not directly use any UIKit methods, but it is inspired by them. This hopefully gives developers an easier path to broad reasoning about the basics of the framework, while not reducing the power of it or it's flexibility to be used with other components. They would have to learn SDL-specific UI objects (SDLSoftButtonWrapper and states specifically) but since they have the broad reasoning down and the details are specifically designed to match how SDL works, with documentation and examples, it shouldn't be too difficult to learn. I also believe this architecture can and should map in its essentials between Android and iOS.

@ghost
Copy link

ghost commented Feb 2, 2018

@joeygrover

the initial discussions we had for the "enhanced frameworks" were along the same lines as this proposal. However, we've asked for evolution proposals based on those works in the past with no response or timeline as to when they might be included. And at this point they are seemingly stale.

I'm sorry about that. I tried to separate the concept of the enhanced framework into smaller pieces but it never turned out well without compromises to the concept and the resulting developer experience. Furthermore if the concept would be divided many (or even all) proposals would need to be reivewed still as changes to one proposal affects another proposal (as happened to remote control). It would make much more sense to review the concept of enhanced framework and branch out features that can be separated from the foundation during workshops.

Also, in my opinion, they were overengineered

I fully disagree to call it "overengineered". The effort spend already on android covers many use cases and app conditions. For some people any foreign code might look complicated or "overengineered" but after reviewing or trying out the code people see that propblems they didn't know it's required to cover are properly solved.

and essentially created an entire new framework over the existing framework that would greatly increase the maintenance burden.

Isn't that what we are trying to do here? Adding another layer using managers or similar to simplify the use of complicated APIs? Didn't iOS already start to that with the managers? I honestly don't know how other code does not increase burden by the same (or higher) amount.

I think there were some good ideas there that we can leverage, but I believe we should aim for a simpler solution that provides what developers need, not the exhaustive functionality that was initially created.

I think your judgement is made without seriously reviewing and trying out the concept of the Android code. It makes it so much easier for app developers to integrate SDL as it provides well known APIs (context and activity based). It covers pretty much all the proposal describes and what it wants to acheive not by using original Android classes but by acting like them in classes like SdlActivity (not inherited from Activity).

I'm not saying we should accept it as such and release a new Android version. It should be considered as a candidate implementation of an "enhanced framework" and reviewed by all members of the steering committee. SDL Android is far behind the iOS implementation and there's so much code provided on a silver plate. We should not just refuse to use it. It's going to be much more complicated to start from scratch and repeat the conceptional and development phase compared to use the provided implementation as a basis and start from there.

PS: I'm pretty sure no one knew about the existence of the provided code. Therefore I propose that members of the steering committee review the implementation to decide if it should be be the basis for an enhanced Anroid framework.

@ghost
Copy link

ghost commented Feb 2, 2018

This would not directly use any UIKit methods, but it is inspired by them. This hopefully gives developers an easier path to broad reasoning about the basics of the framework, while not reducing the power of it or it's flexibility to be used with other components. They would have to learn SDL-specific UI objects (SDLSoftButtonWrapper and states specifically) but since they have the broad reasoning down and the details are specifically designed to match how SDL works, with documentation and examples, it shouldn't be too difficult to learn. I also believe this architecture can and should map in its essentials between Android and iOS.

I fully agree. We should not use UIKit but use the idea and build new classes that developers easily understands as they are experienced with UIKit.

My counter-proposal would look more like this:
...

An SDLViewController makes a lot of sense but I'm afraid of a bunch of subclasses for each template. The set of templates varies per head unit which might cause problems.

Instead of template based subclasses and view related properties directly on the view controller classes we should add a couple view classes that manage UI related behavior.

  • The SDLViewController should contain a single property to the view of type SDLView
  • The class SDLView is the base class for the root view (aka SDLWindow) which points to a rootViewController and is managed by the lifecycle
  • The SDLTemplateView can hold subviews and decides on what template to use (it sends SetDisplayLayout depending on the subviews or developer preference)
  • The SDLTextView manages main fields for Show, media track etc. I don't know the details of the show manager but it would be pretty much the view for it
  • The SDLImageView takes care of a single graphic item on the screen. As there are templates with multiple graphics the SDLTemplateView would allow DOUBLE_GRAPHIC templates
  • The SDLButtonView would take care of the list of buttons the app wants to use within the view controller. Depending on the layout mode the button view would take care the button will be presented depending on the image/artwork

SDLView hierarchy

A hierarchy of the view controllers would also make sense allowing the app to stack view controllers. Allowing to subclass the View controller class for modal UI e.g. Alert and (probably the most challenging) interactions. See attached picture:

SDLViewController hierarchy

@joeljfischer
Copy link
Contributor

@kshala-ford I think your proposal both skews too far toward UIKit and is too complex. My proposal is fairly simple and maps fairly well to how SDL UI works, and while the criticism that some head units don't implement all templates is a good one, it's not insurmountable. The number of subclasses necessary is another good criticism, but I don't think that it's more classes than you are proposing, it's relatively easy to reason about, and each class is rather simple and easy to maintain. Certainly easier and simpler to my mind than allowing the developer to assemble their own UI and then attempting to guess which template most closely matches that, which is what your proposal seems to be doing.

The Show manager proposal is also currently under review, so I would highly recommend that you read and review that one as well.

@kshala
Copy link

kshala commented Feb 2, 2018

@joeljfischer Creating subclasses is a quite static approach for the variable set of templates which is not clearly defined by sdl. You’re proposing >22 subclasses covering any potential template out there. Imagine a developer uses a template that’s not supported on a head unit but another template would also be fine. Your approach does not allow the flexibility. It just fails. The question is “how” should it fail. Exception? Crash? Unload? Fallback?

The developer cannot reuse code for another template unless everything extracted and aggregated. Otherwise it won’t be possible to just move to another template.

Bottom line I believe the amount of classes and the loss of flexibility is less of an issue of using a template view (ish) way.

I don’t agree to say it’s too close to UIKit. This concept requires a minimum effort for a developer as the whole concept is already well known. That’s at least the feedback I gathered from developers (Ford partners and hackathon members). The closer the better with less effort in reading and understanding documentation and faster development.

Believe me when I say the concept’s complexity reaches the same level as your approach or the one of the original proposal. Keep in mind that this concept is already a couple steps beyond and covers use cases that you still need to solve. In my point of view your organic growth of an enhanced api causes much slower and requires more effort to us and to developers. We need to solve design problems we don’t see yet and we need to keep the possibility of backward compatibility much more than with my approach. developers need to keep track with every single release to refactor code due to many deprecations. that can cause new bugs and cost to development and testing. At the end of the day the effort will be much higher and take much more time doing such small pieces of enhancement.

I’ve seen the other proposals already but have not fully reviewed them.

I believe it makes sense to schedule workshops for such enhancements.

@brandon-salahat-tm
Copy link
Contributor

@joeljfischer @kshala

It seems like there has been a lot of discussion, so I don't want to rehash the points that have been made.

I do want to point out that I don't see the problem building something on top of UIKit to suit our purposes. I think that is the most straightforward way to build the most friendly interface possible.

Both of the mentioned counter-proposals involve many UIKit concepts, for example view controllers. I don't see any technical limitation that would prevent us from using UIViewController for this purpose. If designed correctly this could allow SDL template transitions and app logic to be managed by the native view stack, off screen. I think this would address some common complaints we have heard about managing view state for SDL applications.

As far as individual components (buttons, labels, images, etc), I think those need to be evaluated one at a time, but I think regardless of evaluation there is no technical limitation from building interfaces for these on top of UIKit.

The most painless integration is obviously to support these directly, plug and play with the native classes (UIControls, UILabels, UIImages, etc). As Joel has indicated this will abstract away some SDL complexities that may be useful to certain implementations. This means more complex/advanced implementations will have to utilize the lower level managers and constructs (which in this context would still be an improvement over current implementations based on Joel' current proposal for a show manager).

The pros with this approach is that typical or happy path implementations should have a very quick time to market, but it doesn't help more complicated implementations.

A compromise that I think addresses some of the concerns would be to utilize the proposed pattern, but instead of using UIButton, etc directly, create subclasses with the added SDL properties, etc (or modify the existing classes to conform to their UIKit equivalents. In this example perhaps SDLButton could subclass UIButton.

The pro of this approach is that it should be able to cover everything that most implementations need, the con is that it is not quite as seamless as the original proposal.

I think this approach will let developers build out SDL applications in the way they are accustomed to implementing things, and I am not sure implementing/extending things to conform to UIKit is any more or less burden than trying to emulate UIKits patterns in SDL-only classes.

@theresalech theresalech changed the title [In Review] SDL 0133 - Enhanced iOS Proxy Interface [Deferred] SDL 0133 - Enhanced iOS Proxy Interface Feb 7, 2018
@theresalech
Copy link
Contributor Author

The Steering Committee voted to defer this proposal, as they agreed that more detailed discussion should occur in a workshop setting. This proposal will be deferred until a workshop takes place to determine next steps.

@smartdevicelink smartdevicelink locked and limited conversation to collaborators Feb 7, 2018
@theresalech
Copy link
Contributor Author

theresalech commented Mar 20, 2018

During the workshop, it was determined that there are two options for the UI:

  1. Native UI Kit (Extend the Native UI Kit) - this proposal
  2. Mimic Native UI Kit (Take Native and Customize for SDL) - to be entered by @kshala

The initial pros and cons for each option are included in the workshop meeting minutes. The attendees at the workshop were in agreement to move forward with the second option. Once a proposal for the second option has been entered, the Steering Committee will vote between this proposal and that one.

@theresalech theresalech changed the title [Deferred] SDL 0133 - Enhanced iOS Proxy Interface [Rejected] SDL 0133 - Enhanced iOS Proxy Interface Jul 18, 2018
@theresalech
Copy link
Contributor Author

The Steering Committee voted to reject this proposal in favor of SDL 0156 - High level interface: Foundation

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants