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

Discussion about read- write-/multipleproperties #219

Open
danielpeintner opened this issue Jun 9, 2020 · 17 comments
Open

Discussion about read- write-/multipleproperties #219

danielpeintner opened this issue Jun 9, 2020 · 17 comments
Labels
for next iteration Planned or postponed topics for the future priority: high Issues that will be tackled in 2024 wait-for-td

Comments

@danielpeintner
Copy link
Contributor

In the Scripting API call we did have an interesting discussion about what does op readmultipleproperties mean. We did have the feeling that the TD spec is not super clear about that case. I tried to look into it and found the following para (last para in 5.3.1.1 Thing)

If not specified otherwise (e.g., through a TD Context Extension), the request data of the readmultipleproperties operation is an Array that contains the intended PropertyAffordances instance names, which is serialized to the content type specified by the Form instance.

This seems to match with what we have in https://w3c.github.io/wot-scripting-api/#the-readmultipleproperties-method. There needs to be a dedicated form with op being readmultipleproperties and one can specify an array of property names.
Does everybody agree?

In the case of writemultipleproperties it is the same I think.

@zolkis
Copy link
Contributor

zolkis commented Jun 9, 2020

Ah, right, the TD spec has this workaround that Forms mean a different thing on the Thing level than on the interactions level. That should clarify it, since at Thing level we must take all Forms with op === "readmultipleproperties" and get the property names from there. But wait...

There needs to be a dedicated form with op being readmultipleproperties and one can specify an array of property names.

In which of the Form fields one can specify property names? AFAICT it is missing from Form.

@egekorkan
Copy link
Contributor

There is this relevant issue in the TD specification:
w3c/wot-thing-description#848

@zolkis
Copy link
Contributor

zolkis commented Jun 9, 2020

However, this issue has not yet been solved in the TD spec.

@danielpeintner
Copy link
Contributor Author

In which of the Form fields one can specify property names? AFAICT it is missing from Form.

My understanding is that if op readmultipleproperties is provided the endpoint needs to support any possible combinations of property names provided as array

see TD wording I cited in fist comment

.. the request data of the readmultipleproperties operation is an Array that contains the intended PropertyAffordances instance names

Hence the TD does not need to mention which names are supported.

@zolkis
Copy link
Contributor

zolkis commented Jun 10, 2020

It's not the TD who provides the names, but the application.
So where an app (or the ConsumedThing implementation) can write that array, and using what media type, encoding, schema etc?
Meaning: what is the "request data" and where/how is it defined?

@danielpeintner
Copy link
Contributor Author

Correct, the property names string array gets passed to the provided href as input (... mentioned as "request data" in the cited para)

That's at least how I read it... and the wording might be improved.

@zolkis
Copy link
Contributor

zolkis commented Jun 10, 2020

I left a related comment on the TD issue. IMHO some work needs to be done there. I guess the TD TF will suggest we make a PR. For reasons outlined in the TD issue, I am not happy with the "solution" to encode the property list in the href.
But even that should be better explained.

@danielpeintner
Copy link
Contributor Author

I left a related comment on the TD issue. IMHO some work needs to be done there.

+1

I am not happy with the "solution" to encode the property list in the href.

This is not what I meant to say. I think the GET call on href should contain the array in the body

GET domain/fooThing/someProps
["propA", "propB"]

Anyway, lets work on clarifying it in the TD.

@zolkis
Copy link
Contributor

zolkis commented Jun 18, 2020

What we can do is to leave that up to the implementations and formulate the Scripting algorithms in a general way (e.g. fetch the array of names from the request - without going into details). Until this is resolved in the TD spec it should be okay.

@zolkis zolkis added the F2F Topics related to Face-to-Face meetings label Jun 18, 2020
@zolkis
Copy link
Contributor

zolkis commented Jul 20, 2020

In the virtual F2F, we agreed that readMultipleProperties() is not strictly needed and could be removed.

As discussed on the Scripting call on 20.7.2020, we agreed to tag/publish the current version, discontinue readMultipleProperties() from the API, and if the need arises, we can add it back later.

@danielpeintner
Copy link
Contributor Author

Note: I suggest to remove the F2F label and re-label it with a newly created TD label

@relu91
Copy link
Member

relu91 commented Dec 20, 2021

As discussed on the Scripting call on 20.7.2020, we agreed to tag/publish the current version, discontinue readMultipleProperties() from the API, and if the need arises, we can add it back later.

I don't think this has been implemented. We still have readMultipleProperties method in our spec.

@danielpeintner
Copy link
Contributor Author

Good point!
How shall we proceed, discuss it once again in the next call or apply the change and remove it from the API?

@relu91
Copy link
Member

relu91 commented Dec 21, 2021

Given that we reached a consensus already I think we should apply the changes. If I remember in the TD we somehow clarified a little how the list of selected properties should look like (forcing to be a json array ?), but this probably works only for greenfield devices... finding an already existing API with the right serialization is a matter of luck.

@danielpeintner
Copy link
Contributor Author

Given that we reached a consensus already I think we should apply the changes

👍

If I remember in the TD we somehow clarified a little how the list of selected properties should look like (forcing to be a json array ?), but this probably works only for greenfield devices... finding an already existing API with the right serialization is a matter of luck.

This is correct. On the other hand the API could also handle it internally by doing Promise.all() but I think we said this is not what we want...

@zolkis
Copy link
Contributor

zolkis commented Dec 21, 2021

Right. Promise.all() can be done from an app. But getting all properties with one request is a useful feature - that we could support when possible, but should not fake it when it's not there.

@danielpeintner
Copy link
Contributor Author

I added the label "Needed by other TF " for w3c/wot-thing-description#848

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for next iteration Planned or postponed topics for the future priority: high Issues that will be tackled in 2024 wait-for-td
Projects
None yet
Development

No branches or pull requests

4 participants