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

fix: stream read not working past 1 read #1925

Merged
merged 1 commit into from
Aug 31, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions packages/binding-abstract/binding-abstract.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,12 +85,12 @@ The in progress reads must error when the port is closed with an error object th
throw new TypeError('"buffer" is not a Buffer')
}

if (typeof offset !== 'number') {
throw new TypeError('"offset" is not an integer')
if (typeof offset !== 'number' || isNaN(length)) {
throw new TypeError(`"offset" is not an integer got "${isNaN(length) ? 'NaN' : typeof offset}"`)
}

if (typeof length !== 'number') {
throw new TypeError('"length" is not an integer')
if (typeof length !== 'number' || isNaN(length)) {
throw new TypeError(`"length" is not an integer got "${isNaN(length) ? 'NaN' : typeof length}"`)
}

debug('read')
Expand Down
6 changes: 3 additions & 3 deletions packages/binding-mock/binding-mock.js
Original file line number Diff line number Diff line change
Expand Up @@ -149,10 +149,10 @@ class MockBinding extends AbstractBinding {
})
}
const data = this.port.data.slice(0, length)
const readLength = data.copy(buffer, offset)
const bytesRead = data.copy(buffer, offset)
this.port.data = this.port.data.slice(length)
debug(this.serialNumber, 'read', readLength, 'bytes')
return readLength
debug(this.serialNumber, 'read', bytesRead, 'bytes')
return { bytesRead, buffer }
}

async write(buffer) {
Expand Down
5 changes: 3 additions & 2 deletions packages/bindings/lib/bindings.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -544,9 +544,10 @@ function testBinding(bindingName, Binding, testPort) {
return binding.read(buffer, 0, buffer.length)
})

it('returns at maximum the requested number of bytes', () => {
return binding.read(buffer, 0, 1).then(bytesRead => {
it('returns at maximum the requested number of bytes and the buffer', () => {
return binding.read(buffer, 0, 1).then(({ bytesRead, buffer: returnedBuffer }) => {
assert.equal(bytesRead, 1)
assert.equal(buffer, returnedBuffer)
})
})
})
Expand Down
7 changes: 5 additions & 2 deletions packages/bindings/lib/win32.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,15 @@ class WindowsBinding extends AbstractBinding {

async read(buffer, offset, length) {
await super.read(buffer, offset, length)
return asyncRead(this.fd, buffer, offset, length).catch(err => {
try {
const bytesRead = await asyncRead(this.fd, buffer, offset, length)
return { bytesRead, buffer }
} catch (err) {
if (!this.isOpen) {
err.canceled = true
}
throw err
})
}
}

async write(buffer) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const crypto = require('crypto')
const SerialPort = require('../')
const SerialPort = require('.')

let platform
switch (process.platform) {
Expand Down
6 changes: 3 additions & 3 deletions packages/stream/stream.js
Original file line number Diff line number Diff line change
Expand Up @@ -370,10 +370,10 @@ SerialPort.prototype._read = function(bytesToRead) {
const start = pool.used

// the actual read.
debug('_read', `reading`)
debug('_read', `reading`, { start, toRead })
this.binding.read(pool, start, toRead).then(
bytesRead => {
debug('binding.read', `finished`)
({ bytesRead }) => {
debug('binding.read', `finished`, { bytesRead })
// zero bytes means read means we've hit EOF? Maybe this should be an error
if (bytesRead === 0) {
debug('binding.read', 'Zero bytes read closing readable stream')
Expand Down
19 changes: 16 additions & 3 deletions packages/stream/stream.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,7 @@ describe('SerialPort', () => {
})
})

it('emits the close event and runs the callback', done => {
it('emits the "close" event and runs the callback', done => {
let called = 0
const doneIfTwice = function() {
called++
Expand All @@ -472,7 +472,7 @@ describe('SerialPort', () => {
port.on('close', doneIfTwice)
})

it('emits an error event or error callback but not both', done => {
it('emits an "error" event or error callback but not both', done => {
const port = new SerialPort('/dev/exists', { autoOpen: false })
let called = 0
const doneIfTwice = function(err) {
Expand All @@ -487,7 +487,7 @@ describe('SerialPort', () => {
port.close(doneIfTwice)
})

it('fires a close event after being reopened', done => {
it('emits a "close" event after being reopened', done => {
const port = new SerialPort('/dev/exists', () => {
const closeSpy = sandbox.spy()
port.on('close', closeSpy)
Expand Down Expand Up @@ -894,6 +894,19 @@ describe('SerialPort', () => {
port.binding.write(testData)
})
})

it('emits data events with resuming', async () => {
const testData = Buffer.from('I am a really short string')
const port = new SerialPort('/dev/exists', { bindingOptions: { echo: true } })
await new Promise(resolve => port.on('open', resolve))
await new Promise(resolve => port.write(testData, resolve))
await new Promise(resolve => port.once('readable', resolve))
const data1 = port.read()
await new Promise(resolve => port.write(testData, resolve))
await new Promise(resolve => port.once('readable', resolve))
const data2 = port.read()
assert.deepEqual(Buffer.concat([data1, data2]), Buffer.concat([testData, testData]))
})
})

describe('disconnect close errors', () => {
Expand Down