reyiyo / virtual-serialport

A drop-in virtual replacement for node-serialport's SerialPort object
37 stars 13 forks source link

is it updated? #9

Closed roccomuso closed 7 years ago

roccomuso commented 7 years ago

That's what i was looking for to write automated tests for my repo: https://github.com/roccomuso/iot-433mhz :) Does it expose the same method of serialport >= 4.0.0 ?

Apollon77 commented 7 years ago

I would also be highly interested ... (and willing to support)

roccomuso commented 7 years ago

I've compared it with the serialport v4.0.7, here the missing methods:

Instead, the missing events are:

Apollon77 commented 7 years ago

What about the idea to create a new branch for this topic to use later?

It would also be gread to have a basic "pause/resume/read" simulation (internal Buffer where reads are added when on pause, and re-emitted when changed to "resume" or so.

BTW: The original node-serialport do not remove all Listeners on close! SO Virtual-Serialport should also not do that :-)

morganrallen commented 7 years ago

I've tried to address some of these issues in my two pull requests.

12 will emit the close event as expected. It also drops the removeAllListeners call.

13 fixes the constructor call to match SerialPort. openImmediately is also changed to options.autoOpen to match serialport's options.

@roccomuso it looks like you have done that last release, are you interested in having this module more closely resemble the original node-serialport? I'm glad to help fill out the rest of that list.

Thanks for getting this list going!

Apollon77 commented 7 years ago

I also have a "self patched" version to be used for some testing where I changed behaviour to match with serialport 4.x ... so i also could send this as PR

morganrallen commented 7 years ago

It would be great to see more PRs against this. If these changes don't look like they'll get picked up I intend on forking and republishing under a new name. I need better tests against serialport and to get those I need a module that matches serialport as closely as possible. I think this will make sense for most people.

roccomuso commented 7 years ago

PR are welcome :) But since we don't have CI tests just make sure to publish something that works well and don't break stuff. We'll release a minor update then.

morganrallen commented 7 years ago

13 should probably be more than a minor update as it changes the way the port is instantiated.

As far as publishing, it looks like only you ( @roccomuso ) @reyiyo can publish under virtual-serialport which I think is ideal, but it'll be up to one of you to determine versioning. In my opinion bringing in these changes should bump to 1.0.0 and go from there.

I'm also thinking about how to handle testing.

roccomuso commented 7 years ago

Yeah sure, if there's some breaking changes we'll push a major release.

morganrallen commented 7 years ago

Awesome! Look forward to helping get virtual-serialport aligned with node-serialport

Thanks working with us on this.

morganrallen commented 7 years ago

Also, for for what its worth, despite the lack of proper tests each of these PRs updates the primitive test to demonstrate their functionality.

morganrallen commented 7 years ago

Is there anything presently blocking a release? Glad to cleanup anything needing it, would love to see release soon. Let me know if anything else needs doing, thanks.

Apollon77 commented 7 years ago

I also sent in my PR #14 what I needed to do to have testing compatible to virtualport 4.x

morganrallen commented 7 years ago

This appears to be the same as #12 & #13 and should have your needs covered as well, those also include basic tests.

I also started working on better tests last night which I'll submit as a separate PR when I have time to finish it.

Apollon77 commented 7 years ago

only partially :-) I think the work is to manually combile all three PRs to get "the best out of it" ...

morganrallen commented 7 years ago

The problem I see with that is we'd be keeping the 4 argument constructor which is not the same as node-serialport. Otherwise the PRs are the same. This is also why I suggested a major version bump to enforce the fact this is a breaking change.

Apollon77 commented 7 years ago

For the constructor the changes are the same, yes, but there is also more. And yes ... a major version like 4.0 to be in sync with serialport version would make sense

reyiyo commented 7 years ago

All merged, sorry for the delay. I'll try to keep this repo up to date if you want to send PRs. Also, we can set up travis CI for running tests.

Also, a 4.0.0 release is published to npm to keep in sync with node-serialport's versioning.

Thank you all!