Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Small fixes [for case of packet fragmentation] #102

Closed

Conversation

baffo32
Copy link
Contributor

@baffo32 baffo32 commented Oct 19, 2016

These adjustments allow the tests to pass if #101 is merged or used.

Incidentally also fixes some initialization issues when communicating with a real live board.

@codecov-io
Copy link

codecov-io commented Oct 19, 2016

Current coverage is 93.53% (diff: 100%)

Merging #102 into 1.4.0 will increase coverage by <.01%

@@              1.4.0       #102   diff @@
==========================================
  Files             4          4          
  Lines          2180       2181     +1   
  Methods         126        126          
  Messages          0          0          
  Branches        475        475          
==========================================
+ Hits           2039       2040     +1   
  Misses          141        141          
  Partials          0          0          

Powered by Codecov. Last update 7d78748...cc4afcd

Copy link

@andrewjaykeller andrewjaykeller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So these are the tests for #101 ? If yes, why not combine both PRs? Curious on this one and also confused on the order I'm supposed to merge.

@@ -1597,6 +1597,8 @@ function OpenBCIFactory () {
this.curParsingMode = k.OBCIParsingNormal;
this.buffer = null;
this.emit('ready');
} else {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good!

@baffo32
Copy link
Contributor Author

baffo32 commented Oct 19, 2016

Just fixes, not tests.

Sorry, I guess this should probably be on top of #101. The two relevant fixes are still applicable without #101, they just don't change any test results without it. I am still learning about latency timers etc to possibly rework #101 to emulate the chip better.

@andrewjaykeller
Copy link

I'm surprised this passed tests because fragmentation is not in master

@baffo32
Copy link
Contributor Author

baffo32 commented Oct 19, 2016

will rebase onto #100

@baffo32 baffo32 closed this Oct 19, 2016
@baffo32 baffo32 deleted the 1.4.0-fragmentation-tests branch October 19, 2016 11:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants