Align value function to Scripting API
#1217
Conversation
|
Ah it seems that I didn't properly rebase with master, after lunch I'll fix it. Sorry! |
…th spec Note that the exploreDirectory method has to be updated to reflect the new check for the presence of DataSchema in the value function. Tests have been updated too. In particular opcua required an ad hoc parsing. fix eclipse-thingweb#1216
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #1217 +/- ##
==========================================
+ Coverage 77.73% 77.75% +0.01%
==========================================
Files 84 84
Lines 17443 17444 +1
Branches 1761 1765 +4
==========================================
+ Hits 13559 13563 +4
+ Misses 3848 3845 -3
Partials 36 36 ☔ View full report in Codecov by Sentry. |
| let rawThingDescriptions: WoT.ThingDescription[]; | ||
| try { | ||
| const thingsPropertyOutput = await this.directory.readProperty("things"); | ||
| rawThingDescriptions = (await thingsPropertyOutput.value()) as WoT.ThingDescription[]; |
There was a problem hiding this comment.
Could we use the generic "version" of value<T> here to avoid the casting?
| rawThingDescriptions = (await thingsPropertyOutput.value()) as WoT.ThingDescription[]; | |
| rawThingDescriptions = await thingsPropertyOutput.value<WoT.ThingDescription[]>()); |
There was a problem hiding this comment.
Sadly no, Given the fact that CosumeThing implementation returns WoT.InteractionOutput and not our InteractionOutput. As I said another time the generic in the value function is mostly experimental... we should think if it makes sense to have it in the public APIs.
There was a problem hiding this comment.
I see, then we can mark this thread as resolved :) Or should we open an issue for the evaluation of the generic in the value function?
There was a problem hiding this comment.
I think the issue should be raised in the Scripting API repo, first (maybe it is already there?) and if it fails we have to evaluate how to do it here. @danielpeintner what do you think?
There was a problem hiding this comment.
I am not aware of a related issue in the Scripting API repo. Raising the aspect makes sense and maybe gives some more feedback from others...
Co-authored-by: danielpeintner <daniel.peintner@gmail.com>
As discussed in #1216, the current implementation of the value function didn't take into account the validation of the DataSchema of the target affordance. This PR fix #1216 but it also introduces some required changes to take into account the new behavior in other places of the codebase. In particular, it updates the
exploreDirectorymethod andThingDiscoveryProcessclass moving the fetching of the list of ThingDescirption from the method to the async iterator. The main reason for this change is that I figured out that with a DataSchema thereadPropertywould throw before entering even starting theThingDiscoveryProcesswhich would be strange and violate the contract of the methodexploreDirectory(errors for the list should be conveyed as errors in the ThingDesscovery object). Plus it has the benefit of lazy loading the list of TDs and they might be even controlled withlimitandoffsetlater. Handling the case in which the TDD does not have the DataSchema declared is still a TODO (but I can include in this PR).Finally, I fixed a minor problem in the binding-file that was preventing a clean test run on my local machine.