Evented speech controller #4
Conversation
| @@ -23,6 +25,20 @@ export default class MainController extends BaseController { | |||
| 'reminders': remindersController, | |||
| }; | |||
|
|
|||
| const speechController = new SpeechController(); | |||
| speechController.on( | |||
There was a problem hiding this comment.
At the minute, I'm not certain of the best way to send these events to the correct UI (currently active controller) to display something - is there something simple we can do?
There was a problem hiding this comment.
I would move this block earlier in the constructor so that the speechController instance can be passed as an option to the controllers (just like mountNode). Then it'll be up to the controllers to do whatever they want with it.
| .pipe(rename('js/polyfills/fetch.js')) | ||
| .pipe(rename('js/polyfills/fetch.js')), | ||
|
|
||
| gulp.src('./app/js/third_party/webrtc-adapter.js') |
There was a problem hiding this comment.
Can you place these files in the components folder instead third_party (see above with lodash and moment)?
| @@ -0,0 +1,717 @@ | |||
| /** | |||
There was a problem hiding this comment.
The way we're handling external dependencies is usually the following:
- Install via npm (In this case, it's not published on npm so the syntax to use in
package.jsonis something like"JsSpeechRecognizer": "github:dreamdom/JsSpeechRecognizer#master"(not sure if the master is required) - Copy the files you need from
node_modulestoapp/js/componentsusing thecopy-app-commongulp task (seecopy-app-devandcopy-app-productiontasks if you need to copy different files on dev and prod environments) - Ideally load the file using es2015 modules, or via a
<script>tag if not compatible
This avoid checking in external files into the tree.
Sorry for not having documenting this part.
Does it make sense to you?
| @@ -68,6 +68,8 @@ gulp.task('copy-app-common', () => { | |||
| return merge( | |||
| gulp.src([ | |||
| `${APP_ROOT}**`, | |||
| `${APP_ROOT}/data`, | |||
| `${APP_ROOT}/js/third_party`, | |||
There was a problem hiding this comment.
Is third_party folder still needed?
…naged dependencies
| @@ -34,6 +50,10 @@ export default class MainController extends BaseController { | |||
| }); | |||
| } | |||
|
|
|||
| this[p.speechController].start().then(() => { | |||
| console.log('Speech controller started'); | |||
| }); | |||
There was a problem hiding this comment.
Maybe in the future we want to wait until the speech controller has started before starting the app to prevent nasty race conditions.
I don't think that's likely to happen though. What do you think?
There was a problem hiding this comment.
I don't think this is likely to happen as it is, as the only interface we expose is events right now - I think it's OK for now.
| import JsSpeechRecognizer from 'components/jsspeechrecognizer'; | ||
|
|
||
| export default class WakeWordRecogniser { | ||
| constructor(options={}) { |
| this[p.wakewordModelUrl] = '/data/wakeword_model.json'; | ||
| } | ||
|
|
||
| _initializeSpeechRecognition() { |
There was a problem hiding this comment.
For all private methods, let's stick to the "symbols properties" pattern (See const p = Object.freeze({... above),
| _startSpeechRecognition: Symbol('_startSpeechRecognition'), | ||
| _handleSpeechRecognitionEnd: Symbol('_handleSpeechRecognitionEnd'), | ||
| _parseIntent: Symbol('_parseIntent'), | ||
| _actOnIntent: Symbol('_actOnIntent'), |
There was a problem hiding this comment.
nit: no need to prefix these properties with _.
|
r+ with the nits addressed. Good job! |
No description provided.