intel / iotivity-node

Node.js bindings for IoTivity
https://www.iotivity.org/
42 stars 44 forks source link

Update JS API spec to V2. #30

Closed zolkis closed 8 years ago

zolkis commented 9 years ago

Fixes a couple of issues with the JS API vs the 1.0 version of the OIC Core Specification, such as:

The new API spec does all the above, plus:

hekra01 commented 9 years ago

remove args from findDevices() as it does an endpoint discovery

Then we dont have the possiblity to do unicast device discovery as in OCPlatform::getDeviceInfo(const std::string & host, ...)

zolkis commented 9 years ago

Then we dont have the possiblity to do unicast device discovery as in OCPlatform::getDeviceInfo(const std::string & host, ...)

I have looked in the C++ API implementation, and there are some unfulfilled promises there, including unicast device discovery: the API makes it look like it is possible, but as far as I could find, it is not implemented. Nor could I find it supported in the Core spec. If it is HTTP discovery (mDNS), then it is multicast. If it is CoAP discovery, the OIC Core spec says it is multicast. So I don't see support there for unicast device discovery.

Should I include it back "for any case" as an optional arg, or should I cut it out until we bump into the lack of it and add only then? Usually it's easier to add than remove things.

hekra01 commented 9 years ago

I have looked in the C++ API implementation, and there are some unfulfilled promises there, including unicast device discovery: the API makes it look like it is possible, but as far as I could find, it is not implemented

It tested it successfully. It works if port is provided in host. A jira issue already exists about your observation, I commented there also: https://jira.iotivity.org/browse/IOT-823

Nor could I find it supported in the Core spec

Core spec v0.9.9 at 11.3.2.2 Direct discovery: "The request may be made as a unicast or multicast."

There is also a test case for this in OIC_Compliance_and_Interoperability_Test_Specification_v0 58: "CT1.1.4 CoAP Based Unicast Discovery – /oic/d"

zolkis commented 9 years ago

It seems that the gap between the old (v1) and new (v2) API is closing. v2 moves more client methods to OicResource, making programming more clear and more intuitive.

A question about resource id. Right now we use {deviceId, resourcePath} tuple. Usually in Web API's they use URLs in this case. At least @kenchris is in favor of that. I wonder if should we use URLs (exposed as USVString) with the following notation: http://<address:port>/<path> means the device id is specified as address:port tuple, and oic://<deviceIdString>/<path> means the device id is an opaque string, and OIC is used for identifying the device. I emphasize this is a client abstraction for the JS API, not the OIC Core spec URL semantics. Implementations and clients would need to parse that string in order to separate resource path from device id. Leaving the {deviceId, path } tuple also works, albeit not so web'ish. I am leaning towards leaving this as a tuple. Opinions?

gabrielschulhof commented 9 years ago

I vote tuple.

poussa commented 9 years ago

I vote for tuple as well. oic scheme looks like overloading it.

hekra01 commented 9 years ago

I think we should add "secure" to the ConnectionMode

At least 2 cases for security:

OCPlatform::registerResource(handle, uri, type, interface, cb, OC_DISCOVERABLE | OC_OBSERVABLE | OC_SECURE);

PlatformConfig in IoTivity in fact should be device config IMHO.

OK, but then, I observe that when the JS spec/implementations are moved to the IoTivity repos, their semantics of platform/device management will differ from the ones used by the C/C++/Java APIs.

zolkis commented 9 years ago

I will rework the connectivity and security related parts. Thanks for the hints. However, it is not a problem if the JS API slightly differs from the native ones.

hekra01 commented 9 years ago

One other use case with security is the provisioning part: ownership, ACLs and certificates, etc... need to be managed, for secured IoTivity devices to communicate. (all details in OIC security spec)

OCProvisioningManager.h provides the APIs for this, and provisioning can be done from command line (provisioningclient.c or provisioningclient.cpp)

Maybe this part would make sense for addition to the JS APIs? (Use case: configure device provisioning from web application)

poussa commented 9 years ago

Provisioning and security must be part of the API.

zolkis commented 9 years ago

I suggest we include provisioning in the next version, v3. We should still leave this PR open until v2, and changes to v1 are finalized.

zolkis commented 8 years ago

With the latest developer feedback and latest changes, the differences between V1 and V2 APIs became minor. Now the proposed version is called the V2 API. Updated the PR name.

zqzhang commented 8 years ago

We should still leave this PR open until v2, and changes to v1 are finalized.

Do we finish this? Or is there anything block this PR merging?

zolkis commented 8 years ago

We should still leave this open. Implementation of the spec is ongoing, and we need that feedback to this PR. Also, we play with this in the REST API server and get feedback from there, too. We leave the spec open until it is fairly stable. Looks like soon (this week?) we could merge.

poussa commented 8 years ago

@zolkis Should we merge this since it has all been implemented in the code already, right?

zolkis commented 8 years ago

Yes, we could merge this.

zqzhang commented 8 years ago

@gabrielschulhof could you help merge this per the above comments?