Return an Observable instead of firing events
Closed this issue · 34 comments
Anyone interested in an RxJS Observable variant of this project?
https://github.com/Reactive-Extensions/RxJS/blob/master/doc/api/core/operators/start.md
Would that simplify the interface?
Yes
See this package as an example where an Observable is returned.
https://www.npmjs.com/package/robinhood-observer
If there is a lot of interest from the community I would be happy to create this.
var OpenBCI = require('openbci-observer')
var updateFrequency = 200 // Frequency with which to pull updates from the device in milliseconds.
var observer = OpenBCI().observeAll(updateFrequency)
var dataSubscription = observer
.map(tick => tick.results)
.subscribe(x => {
//This block of code is executed each time an event would have been called.
console.log(x);
// x would be the json representing the data received from the headset.
}, e => {
console.error(e)
}, () => console.log('data subscription disposed'));
//Unsubscribe to updates after 5 seconds.
setTimeout(function(){
dataSubscription.dispose();
}, 5000);I would envision the interface might look something like this.
Thoughts @aj-ptw ???
Would we have to have a different repo or could we make this default?
There is a lot of interest from the community to simplify the number of functions needs to stream to one.
@tashoecraft you work with RxJS, good application for it?
@alexcastillo thoughts?
@baffo32 you have been helping a lot, what are your thoughts?
@jspenc72 I think this is super cool and would love it for you to contribute it, just checked out the Robin Hood wrapper, still I don't understand why it's in another repo.
@jspenc72 were you considering going 'under the hood' and changing how the library functions to use rxjs instead of promises (if that even makes sense) internally? Perhaps it could be helpful to have another set of eyes reviewing and organizing the internal code.
I am working detecting runaway promises in tests. So, I would be curious if this will still be reasonable to do with the addition of rxjs: to assert at particular points in the program that, globally, no tasks are still running.
Looks good to me, at first glance.
@baffo32 I think this flows really well after merge addressing #95 and all the clean up work you're doing. If we are able to connect and well disconnect without resetting, this could play really well with the wrapper @jspenc72 wants to build. The more I think about it, seems like it would almost be a requirement. Or the wrapper doesn't disconnect once connected.
@aj-ptw , the robinhood-observer is in a different repo because the owner of the robinhood repo felt that observers were not needed and would be better built as a standalone project.
You don't have to have a different repo. Yes I would likely "go under the hood"
Ideally the observable would be tied directly to the incoming data stream from the device at the ealiest place possible. So if you have a stream coming in then the observable would be generated at when the stream is created and will be updated each time you receive a new packet from the stream.
Ok love it @jspenc72
The current roadmap is:
1.4.0 - Connection - October/Early November
- Adopting semi-standard linting #83
- Merge conflicts could be big.
- Gets all tests passing with a real board. #99
1.4.1 - Connection - Mid November
- Connect and disconnect from the board without stopping streaming. #95
- @baffo32 has been working on that and it's looking great.
1.4.2 - CLI - End of November
- Adds #50, a command line interface for the board.
2.0.0 - Ganglion/Observables - December 2016
From what I hear, the Ganglion is shipping at the end of November. #80 #81
- Creating an index entry file. With
ganglionand32 bit boardseparate. - Moves core code to a central file for boards to share.
Potential
- Observables (@jspenc72)
3.0.0 - Wifi - February 2017
- Connect to the board(s) through the new wifi shield. #79
- Faster and variable sampling rate.
- @jspenc72 seems like observables or something similar will be required at this point, you don't want an event firing 500-16000 times a second.
I have really tried to make auto testing a priority so please feel free to ask any questions about writing or running tests on the code you contribute. Given that road map, where do you think observables could go?
@aj-ptw I think making it Observable based is very logically and fitting.
I reviewed the code and see that there are come cases where a promise is probably "ok".
Do you see any advantage in keeping any of the existing promise based methods?
ie
.connect()
.disconnect()
Etc...
To create an observable to handle these might make that portion less friendly than a Simple
.then().catch()Interface
Thoughts? @aj-ptw
@jspenc72 anything is on the table. We are going for simplification so if creating an observable would only make it worse... i would say that's probably not best. Is there a circumstance where users would not want to use observables or even not be able to use observables?
var dataSubscription = observer
.map(tick => tick.results)
.subscribe(x => {
//This block of code is executed each time an event would have been called.
console.log(x);
// x would be the json representing the data received from the headset.
}, e => {
console.error(e)
}, () => console.log('data subscription disposed'));
This would be cool to get it down to just one line. Where you pass in the port name and boom, now you get events.
Ill write a few lines of code tonight and see what I can do
I asked regarding replacement not knowing the scope of behaviors Observable covered. It sounds like Promises should stick around.
Yes, Well definitely keep promises around. Right tool for the right job you know.. 👌
One note: be sure to do new work on the 1.4.0 branch so merging is reasonable. All the whitespace changed.
I think it will help to abstract away the tedious stuff required to start streaming data by default with a one liner observable created as shown above. But allow the user to manually call the methods by passing a config object, possibly when they import the library.
+1 for Observables support using RxJS.
Let me know if you would to get some input from the RxJS team.
@jspenc72 any progress on this? Branch 2.0.0 is looking about good on my end.
@aj-ptw sorry for the delayed response i have been playing with an RxJS implementation... and I think the best solution is to make a new repository as you previously mentioned... Although the raw data processing will remain similar the interface has become extremely different, doesn't resemble this project at all and is a complete rewrite.
I have created an sdk which would be a replacement that is written in TypeScript and will happily share it so we can get feedback from the community soon.
Cheers!
FYI its ES6 and fully compatible with the latest version of node.
Alright, after meeting with @teonbrooks we decided that the best way to move forward with our project is to create a lib that abstracts all the common operations (filtering, buffering, windowing, FFT, etc). So, I took a first pass at implementing Reactive Extensions for OpenBCI.
Here's the project repo and package:
https://github.com/NeuroJS/openbci-rx
https://www.npmjs.com/package/openbci-rx
Do you guys mind helping by testing?
Please feel free to log any issues in the issues page of the repo.
Wow awesome @alexcastillo closing this issue for now