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

Wide review for automotive APIs #239

Closed
alexshalamov opened this issue Oct 2, 2017 · 7 comments
Closed

Wide review for automotive APIs #239

alexshalamov opened this issue Oct 2, 2017 · 7 comments

Comments

@alexshalamov
Copy link

During Device and Sensors WG call, I've got an action point to review automotive specs with regards to generic sensor APIs, so far, I could't find good synergy points, however, found few issues that might need to be fixed (could be false positives 😄).

Vehicle Information Access API

  1. Missing IDL definitions for many interfaces, Vehicle Interface, Zone Interface and other interfaces. For example, Navigator Interface can be defined as:

     partial interface Navigator {
         readonly attribute Vehicle vehicle;
     };
    
  2. Unclear how Zone.equals works. Can the driver zone be different in two instances?

  3. VehicleInterface Interface name can be confusing, since there is already Vehicle Interface. As the purpose of VehicleInterface Interface is to provide access to properties, maybe worth renaming it, e.g., to VehicleProperty.

  4. It is unclear what is the data type for resolved Promise from VehicleInterface.get(). Is it Object, Dictionary or any?

  5. Why unsigned short is used as VehicleSignalInterface.subscribe(...) subscription id? Is it enough? What if it overflows?

  6. VehicleSignalInterface.unsubscribe(...) what if invalid subscription id is provided?

  7. VehicleSignalInterface Interface maybe it would be better if developers would be able to use VehicleProperty.get VehicleProperty.set and observe changes of a property.

     interface VehicleProperty : DataAvailability {
         Promise<Dictionary> get(optional Zone zone);
         Promise set(Dictionary value, optional Zone zone);
         unsigned long subscribe(VehicleInterfaceCallback callback, optional Zone zone, optional unsigned long period);
         void unsubscribe(unsigned long handle);
         readonly attribute Zone[] zones;
         readonly History history;
     };
     
     // Door data that can be fetched asynchronously
     dictionary DoorData {
         readonly attribute DoorOpenStatus status;
         attribute boolean lock;
         readonly attribute Zone? zone;
     }
     
     // get / set use DoorData dictionary
     interface Door : VehicleProperty {
     }
     
     // Extension for Vehicle interface
     partial interface Vehicle {
         readonly attribute Door door;
     }
    
  8. Data Availability and History interface

    These interfaces should be somehow defined in VehicleInterface Interface, thru aggregation / inheritance.

Vehicle Data

It is unclear how to extend / define data types. Example is misleading, Vehicle.whizzer returns VehicleSignalInterface that is generic interface, yet, note says that interface Whizzer is returned, which does not have VehicleSignalInterface in its inheritance tree.

Vehicle Information API Specification

Is this specification up-to-date? Looks like Vehicle Data is a wrapper on top of Vehicle Information API Specification or Vehicle Signal Server Specification. Maybe, some explainer document or examples would be nice to have, to understand when and what API should be used.

Vehicle Signal Server Specification

It is unclear what is the benefit of this API comparing to Vehicle Information API Specification. Is it for clients that do not implement VISClient interface?

@PeterWinzell
Copy link
Contributor

The first review was done on th old obsolete spec, this spec was replaced by VISS: https://w3c.github.io/automotive/vehicle_data/vehicle_information_service.html

So VIAS is spec on top of VISS.

@alexshalamov
Copy link
Author

@PeterWinzell Thanks, maybe someone could remove it from the README.md or add note that the specification is obsolete / deprecated.

@anssiko
Copy link
Member

anssiko commented Oct 3, 2017

Also the WG's home page at https://www.w3.org/auto/wg/ points to the obsolete spec.

@aShinjiroUrata
Copy link
Contributor

@anssiko , @alexshalamov , thanks for the comment and excuse me for confusion.

The latest spec on which WG members are currently working is below VISS/VIAS editors draft.

Editor's draft

Working Draft (or FPWD)

I think, homepage, wiki, github repo structures may be better to re-organize.

@tguild
Copy link
Member

tguild commented Oct 10, 2017

I will be reviewing pages and making updates. Sorry for wasting your time Alex. My email to device sensors list included the link to what we were asking for review.

@alexshalamov
Copy link
Author

@tguild no worries, I will check new APIs soon and report if I find something.

@peterMelco
Copy link
Contributor

I think this one should be closed.

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

No branches or pull requests

6 participants