refactor: break conversion out into separate files and use canboatjs #8
Conversation
|
Would it make sense if each individual conversion module would in their I'm a complete newbie in JS, so it may well be I've missed something trivial here... |
index.js
Outdated
| var conversions = load_conversions(app, plugin) | ||
| conversions = [].concat.apply([], conversions) | ||
|
|
||
| var types = { |
There was a problem hiding this comment.
It seems like the conversion types are an essential concept when defining the conversions. I struggled to understand their differences by following the code only. Could you maybe add a some comments clarifying the different conversion types?
|
It now supports the conversion returning an error |
|
I've also changed the type stuff and added a comment to document it. |
|
Don’t do anything with the arrays yet. I need to change the way that’s done. |
|
ok @mairas take a look at the battery conversion. Yours will look something like that. |
index.js
Outdated
|
|
||
| onDelta - You will get all deltas via app.signalk.on('delta', ...). Please do no use this unless absolutely necessary. | ||
|
|
||
| bacanjs - The conversion should specify a variable called 'keys' which is an array of the Signal K paths that the convesion needs |
There was a problem hiding this comment.
Could this be renamed to e.g. onValueChange? As far as I understood, baconjs refers to specific implementation, not the source concept.
Also: typo (on line 33, too).
|
The sub-list approach looks good to me! Thanks! |
conversions/battery.js
Outdated
| return options.BATTERY.batteries.map(battery => { | ||
| return { | ||
| keys: batteryKeys.map(key => `electrical.batteries.${battery.signalkId}.${key}`), | ||
| timeouts: batteryKeys.map(key => 60000), |
There was a problem hiding this comment.
If all the timeouts are the same maybe a single number would do here and the calling code could handle arrays and single numbers. Pretty minor...
sbender9
changed the title
No description provided.