Allow to connect without resetting.

[baffo32]

This patch allows the user to connect and disconnect from the 32-bit board without resetting it or changing the streaming state.

This is useful to connect to the board without interrupting SD logging. This would also be invaluable for a CLI version of the API, so tools could engage the board state without having to be responsible for it.

This obviously wouldn't work for the 8-bit board, which I do not have to test with. Does this library support the 8-bit board?

[baffo32]

Whoops, failing test. Guess this needs more work. Withdrawn for now.

[andrewjaykeller]

Run tests with out a dongle plugged in with npm test

[andrewjaykeller]

Also I think we should make that arg optional and default to the current functionality to prevent a breaking change.

[baffo32]

Tests pass now. The arg is indeed optional, and defaults to the current functionality. When not included it will be null and hence evaluate to false in tests.

[codecov-io]

Current coverage is 92.92% (diff: 100%)

Merging #82 into master will increase coverage by 0.12%

@@             master        #82   diff @@
==========================================
  Files             4          4          
  Lines          2193       2219    +26   
  Methods         128        128          
  Messages          0          0          
  Branches        478        482     +4   
==========================================
+ Hits           2035       2062    +27   
+ Misses          158        157     -1   
  Partials          0          0          

Powered by Codecov. Last update d633525...e2d657a

[andrewjaykeller]

Did you add coverage for these? If you need help writing the tests I will happily help!!

[baffo32]

Sorry, I have not added tests!

Here's my notes. If you find this easy by all means jump on it. Otherwise I'll learn the testing API eventually.

When a second parameter of true is passed to connect(), no commands shall be written to the board, the parsing mode will be k.OBCIParsingNormal, and .streaming will be detected true or false depending on whether or not the board and dongle are already actually streaming prior to connection.

When a parameter of true is passed to disconnect(), streamStop() will not be called even if the board is streaming.

[andrewjaykeller]

I now understand the purpose of this PR.

Very interesting. I dig it. Good idea. Ok let's get these tests working!!! We will use spys! I will pull your pr and crank them out first thing tomorrow. Mine is AJ please to e-meet you! Thanks for your help. This is such a good idea.

[andrewjaykeller]

I've been working on it so just give me a little before making changes please!

[baffo32]

Hi my name is Karl. Thank you for looking at this, if it's any trouble don't worry I'll get to it. The timeout is actually sample rate converted from Hz to seconds, times two, so it is only some hundreds of milliseconds. - Sent from my phone

[andrewjaykeller]

gotcha! ok yup almost done writing the tests

[andrewjaykeller]

85 might solve some issues with this pr

[andrewjaykeller]

The more I think about this PR, the more I think it should really be by default that we don't send things to the board and try to detect the stream and if not streaming then send the reset commands.

[baffo32]

I agree, but that would break backwards compatibility.

[andrewjaykeller]

@baffo32 very excited to see if these tests pass!!!

[baffo32]

still a WIP but I think I made a lot of progress today. I haven't enabled the xit tests yet.

[andrewjaykeller]

Ahh ok awesome

[andrewjaykeller]

@baffo32 ok i updated and merged #88 into a new branch called 1.4.0 as requested

[andrewjaykeller]

@baffo32 I am looking at your code and there still needs some work on disconnect

I'm going to rebase your most recent commit over 1.4.0 branch ^ nope

[baffo32]

Yes, that was what I was going to do next. I've been splitting the needed changes into issues so that they could be re-evaluated in light of any changes you made in 1.4.0, which I haven't reviewed yet. (also the work here is split over a few different commits between you and me, not all of them labeled with regard to what they address).

I'm going to try to get disconnect going locally on the branch as-is, so that I understand any related issues, and then address whatever the state of 1.4.0 is.

[andrewjaykeller]

Yup my apologies, i will pr your don't reset branch with the rebased #82 over 1.4.0 ^terrible idea

[andrewjaykeller]

@baffo32 please look at #94 and I plan to merge that with master to push repo to 1.3.3 then you can continue working and we can deal with semi-standard later

[baffo32]

Mostly done. Everything merges fine.

I'd be glad to redo this, now I've partitioned off all the parts needed, again on top of 1.4.0 . I think standardizing the style is valuable.

[baffo32]

@aj-ptw Do you think rebasing off 1.3.3 (rather than using existing merge) is worth it? If I were to go through and make a rebase work it seems it would be more valuable with 1.4.0

[andrewjaykeller]

Do you think rebasing off 1.3.3 (rather than using existing merge) is worth it? If I were to go through and make a rebase work it seems it would be more valuable with 1.4.0

I don't follow. You rebased off 1.4.0 already?

[baffo32]

No, this has been merged with 1.3.3 . But I would like to rebase off 1.4.0 with your go-ahead.

I'm not very familiar with rebasing; I'm more a merger. It would be simpler for me to just redo the work and give the commits proper labels.

[andrewjaykeller]

It's a disaster trying to rebase this off of 1.4.0, i tried haha

The other way around it much much better, where we merge this with master, call it 1.3.4 and rebase 1.4.0 off of that

[andrewjaykeller]

1.4.1 could add #50 too

[baffo32]

I think it would make more sense to redo the work completely, which is not a problem now I've identified all the issues one runs into. Alternatively 1.4.0 could be rebased off this.

50 will probably need tests; it's not immediately ready.

The 1.4 series could also adjust the API to make this dontReset behavior the default, as you mentioned earlier.

[andrewjaykeller]

I think it would make more sense to redo the work completely, which is not a problem now I've identified all the issues one runs into. Alternatively 1.4.0 could be rebased off this.

Ok then. Please let me rebase 1.4.0 off master and then you can redo the work completely

[andrewjaykeller]

@baffo32 ok an hour later 1.4.0 branch is rebased and ready to be built on top of. Don't do work off master any more!

[baffo32]

ok

[andrewjaykeller]

This PR should be closed because of plan because #96 will contain the relevant code covered in this PR.