eclipse-thingweb / eclipse-thingweb/test-things

Smart Coffee Machine - Fixes/Updates/Proposal

Open
#58 13 comments 0 reactions 0 assignees View on GitHub
good first issue
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.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.