eclipse-thingweb / eclipse-thingweb/test-things
Smart Coffee Machine - Fixes/Updates/Proposal
- Dominant language
- JavaScript
- Stars
- 9
- Forks
- 8
- PR merge metrics
- No merged PRs in 30d
Description
I would like to split the issue in 4 sections about the smart-coffee-machine samples in https://github.com/eclipse/thingweb.node-wot/tree/master/packages/examples/src/scripts
### 1. Bugs
The code uses promises in the wrong way (see [here](https://github.com/eclipse/thingweb.node-wot/blob/master/packages/examples/src/scripts/smart-coffee-machine.ts#L251-L267) or [here](https://github.com/eclipse/thingweb.node-wot/blob/master/packages/examples/src/scripts/smart-coffee-machine.ts#L271-L288), ..).
There should be *one* promise and errors should return `reject` and not resolve.
```
thing.setPropertyWriteHandler('availableResourceLevel', (val, options) => {
return new Promise((resolve, reject) => {
// Check if uriVariables are provided
if (options && typeof options === 'object' && 'uriVariables' in options) {
const uriVariables: any = options['uriVariables'];
if ('id' in uriVariables) {
return thing.readProperty('allAvailableResources').then((resources) => {
const id = uriVariables['id'];
resources[id] = val;
return thing.writeProperty('allAvailableResources', resources);
});
} else {
reject('...');
}
} else {
reject('Please specify id variable as uriVariables.');
}
});
});
```
Note: I did not check the code for validity but I think you get the problem (difference to your code).
### 2. Best Practices
Should we actually propose using uriVariables. I think this is something WoT needs to support given it exists but I suggest using variables in the body.
### 3. Improvements
It would be nice IF the `makeDrink` action is a longer running process. Hence a coffee should not be ready immediately. Moreover... in reality `makeDrink` quickly one after the other should fail given that the coffee machine is not ready yet.
Hence I also propose to remove the `maintenanceNeeded` property and use a more generic `status` that could be an enumeration like ["maintenanceNeeded", "inUse/brewing..", ...]
Doing so would also allow modelling the brewing process that takes some time and would refuse new drinks till the *old* on is ready or the machine is maintained...
### 4. WebUI improvements
With the addition of a `status` property we can also improve the UI hosted here: http://plugfest.thingweb.io/examples/smart-coffee-machine.html
The machine could be blinking while brewing the coffee.
Note: It is also necessary that you put the link where you get the SVG file for the coffee machine.
@fatadel can you take a look (I think 1. should be fixed. I don't feel strongly about the rest)
@sebastiankb @egekorkan @relu91 others, please comment if you have another opinion
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.