Skip to content

Commit c0b8897

Browse files
wilkinswwilkinsw
authored
Merge pull request #51 from Daxbot/50-block-transfer-upload-takes-too-long
50 block transfer upload takes too long
2 parents e78c730 + a3acda0 commit c0b8897

File tree

5 files changed

+119
-66
lines changed

5 files changed

+119
-66
lines changed

‎package-lock.json

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "canopen",
3-
"version": "6.1.3",
3+
"version": "6.2.0",
44
"description": "CANopen implementation for Javascript",
55
"main": "index.js",
66
"scripts": {

‎source/protocol/sdo_client.js

Lines changed: 28 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,6 @@ class SdoClient extends Protocol {
117117
* @param {number} [args.timeout] - time before transfer is aborted.
118118
* @param {boolean} [args.blockTransfer] - use block transfer protocol.
119119
* @param {number} [args.blockInterval] - minimum time between data blocks.
120-
* @param {number} [args.blockInterval] - minimum time between data blocks.
121120
* @param {number} [args.cobIdRx] - COB-ID server -> client.
122121
* @returns {Promise<Buffer | number | bigint | string | Date>} resolves when the upload is complete.
123122
* @fires Protocol#message
@@ -129,7 +128,7 @@ class SdoClient extends Protocol {
129128
const timeout = args.timeout || 30;
130129
const dataType = args.dataType || null;
131130
const blockTransfer = args.blockTransfer || false;
132-
const blockInterval = args.blockInterval || 2;
131+
const blockInterval = args.blockInterval;
133132
const cobIdRx = args.cobIdRx || null;
134133

135134
let server = this._getServer({ deviceId, cobIdRx });
@@ -199,7 +198,7 @@ class SdoClient extends Protocol {
199198
const timeout = args.timeout || 30;
200199
const dataType = args.dataType || null;
201200
const blockTransfer = args.blockTransfer || false;
202-
const blockInterval = args.blockInterval || 2;
201+
const blockInterval = args.blockInterval;
203202
const cobIdRx = args.cobIdRx || null;
204203

205204
let server = this._getServer({ deviceId, cobIdRx });
@@ -682,47 +681,45 @@ class SdoClient extends Protocol {
682681
/**
683682
* Download a data block.
684683
*
685-
* Sub-blocks are scheduled using setInterval to avoid blocking during
684+
* Sub-blocks are scheduled on the event loop to avoid blocking during
686685
* large transfers.
687686
*
688687
* @param {SdoTransfer} transfer - SDO context.
689688
* @fires Protocol#message
690689
* @private
691690
*/
692691
_blockDownloadProcess(transfer) {
693-
if (transfer.blockTimer) {
694-
// Re-schedule timer
695-
clearInterval(transfer.blockTimer);
692+
if (!transfer.active) {
693+
// Transfer was interrupted
694+
return;
696695
}
697696

698-
transfer.blockTimer = setInterval(() => {
699-
if (!transfer.active) {
700-
// Transfer was interrupted
701-
clearInterval(transfer.blockTimer);
702-
transfer.blockTimer = null;
703-
return;
704-
}
705-
706-
const sendBuffer = Buffer.alloc(8);
707-
const offset = 7 * (transfer.blockSequence
708-
+ (transfer.blockCount * transfer.blockSize));
697+
const sendBuffer = Buffer.alloc(8);
698+
const offset = 7 * (transfer.blockSequence
699+
+ (transfer.blockCount * transfer.blockSize));
709700

710-
sendBuffer[0] = ++transfer.blockSequence;
711-
if ((offset + 7) >= transfer.data.length) {
712-
sendBuffer[0] |= 0x80; // Last block
713-
transfer.blockFinished = true;
714-
}
701+
sendBuffer[0] = ++transfer.blockSequence;
702+
if ((offset + 7) >= transfer.data.length) {
703+
sendBuffer[0] |= 0x80; // Last block
704+
transfer.blockFinished = true;
705+
}
715706

716-
transfer.data.copy(sendBuffer, 1, offset, offset + 7);
717-
this.send(transfer.cobId, sendBuffer);
718-
transfer.refresh();
707+
transfer.data.copy(sendBuffer, 1, offset, offset + 7);
708+
this.send(transfer.cobId, sendBuffer);
709+
transfer.refresh();
719710

720-
if (transfer.blockFinished
721-
|| transfer.blockSequence >= transfer.blockSize) {
722-
clearInterval(transfer.blockTimer);
723-
transfer.blockTimer = null;
711+
if (!transfer.blockFinished
712+
&& transfer.blockSequence < transfer.blockSize) {
713+
if(transfer.blockInterval === 0) {
714+
// Fire segments as fast as possible
715+
setImmediate(() => this._blockDownloadProcess(transfer));
716+
}
717+
else {
718+
// If blockInterval is undefined, then default to 1 ms
719+
setTimeout(() => this._blockDownloadProcess(transfer),
720+
transfer.blockInterval || 1)
724721
}
725-
}, transfer.blockInterval || 1);
722+
}
726723
}
727724

728725
/**

‎source/protocol/sdo_server.js

Lines changed: 49 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ class SdoServer extends Protocol {
2929
super(eds);
3030
this.transfers = {};
3131
this._blockSize = 127;
32+
this._blockInterval = null;
3233
}
3334

3435
/**
@@ -40,6 +41,16 @@ class SdoServer extends Protocol {
4041
return this._blockSize;
4142
}
4243

44+
/**
45+
* Time delay between each segment during a block transfer.
46+
*
47+
* @type {number}
48+
* @since 6.2.0
49+
*/
50+
get blockInterval() {
51+
return this._blockInterval;
52+
}
53+
4354
/**
4455
* Set the number of segments per block when serving block transfers.
4556
*
@@ -53,6 +64,19 @@ class SdoServer extends Protocol {
5364
this._blockSize = value;
5465
}
5566

67+
/**
68+
* Set the time delay between each segment during a block transfer.
69+
*
70+
* @param {number} value - block interval in milliseconds.
71+
* @since 6.2.0
72+
*/
73+
setBlockInterval(value) {
74+
if(value < 0)
75+
throw RangeError('blockInterval must be positive or zero');
76+
77+
this._blockInterval = value;
78+
}
79+
5680
/**
5781
* Start the module.
5882
*
@@ -470,48 +494,45 @@ class SdoServer extends Protocol {
470494
/**
471495
* Download a data block.
472496
*
473-
* Sub-blocks are scheduled using setInterval to avoid blocking during
497+
* Sub-blocks are scheduled on the event loop to avoid blocking during
474498
* large transfers.
475499
*
476500
* @param {SdoTransfer} client - SDO context.
477501
* @fires Protocol#message
478502
* @private
479503
*/
480504
_blockUploadProcess(client) {
481-
if (client.blockTimer) {
482-
// Re-schedule timer
483-
clearInterval(client.blockTimer);
505+
if (!client.active) {
506+
// Transfer was interrupted
507+
return;
484508
}
485509

486-
client.blockTimer = setInterval(() => {
487-
if (!client.active) {
488-
// Transfer was interrupted
489-
clearInterval(client.blockTimer);
490-
client.blockTimer = null;
491-
return;
492-
}
493-
494-
const sendBuffer = Buffer.alloc(8);
495-
const offset = 7 * (client.blockSequence
496-
+ (client.blockCount * client.blockSize));
510+
const sendBuffer = Buffer.alloc(8);
511+
const offset = 7 * (client.blockSequence
512+
+ (client.blockCount * client.blockSize));
497513

498-
sendBuffer[0] = ++client.blockSequence;
499-
if ((offset + 7) >= client.data.length) {
500-
sendBuffer[0] |= 0x80; // Last block
501-
client.blockFinished = true;
502-
}
514+
sendBuffer[0] = ++client.blockSequence;
515+
if ((offset + 7) >= client.data.length) {
516+
sendBuffer[0] |= 0x80; // Last block
517+
client.blockFinished = true;
518+
}
503519

504-
client.data.copy(sendBuffer, 1, offset, offset + 7);
505-
this.send(client.cobId, sendBuffer);
520+
client.data.copy(sendBuffer, 1, offset, offset + 7);
521+
this.send(client.cobId, sendBuffer);
506522

507-
client.refresh();
523+
client.refresh();
508524

509-
if (client.blockFinished
510-
|| client.blockSequence >= client.blockSize) {
511-
clearInterval(client.blockTimer);
512-
client.blockTimer = null;
525+
if (!client.blockFinished && client.blockSequence < client.blockSize) {
526+
if(this._blockInterval === 0) {
527+
// Fire segments as fast as possible
528+
setImmediate(() => this._blockUploadProcess(client));
529+
}
530+
else {
531+
// If blockInterval is undefined, then default to 1 ms
532+
setTimeout(() => this._blockUploadProcess(client),
533+
this._blockInterval || 1);
513534
}
514-
});
535+
}
515536
}
516537

517538
/**

‎test/protocol/test_sdo.js

Lines changed: 39 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,41 @@ describe('Sdo', function () {
234234
device.stop();
235235
}
236236
});
237+
238+
it('should handle large transfers', async function () {
239+
const device = new Device({ id: 0xA, loopback: true });
240+
device.eds.addSdoClientParameter(device.id);
241+
device.eds.addSdoServerParameter(device.id);
242+
device.start();
243+
244+
const data = Buffer.alloc(65*1024);
245+
for (let i = 0; i < data.length; ++i)
246+
data[i] = Math.floor(Math.random() * 0xff);
247+
248+
device.eds.addEntry(0x1234, {
249+
parameterName: 'A long buffer',
250+
dataType: DataType.DOMAIN,
251+
accessType: AccessType.READ_WRITE,
252+
});
253+
254+
await device.sdo.download({
255+
serverId: device.id,
256+
data: data,
257+
dataType: DataType.DOMAIN,
258+
index: 0x1234,
259+
blockTransfer: false,
260+
});
261+
262+
const result = await device.sdo.upload({
263+
serverId: device.id,
264+
dataType: DataType.DOMAIN,
265+
index: 0x1234,
266+
blockTransfer: false,
267+
});
268+
269+
expect(Buffer.compare(data, result)).to.equal(0);
270+
device.stop();
271+
});
237272
});
238273

239274
describe('Block transfer', function () {
@@ -306,8 +341,6 @@ describe('Sdo', function () {
306341
}
307342

308343
it('should handle large transfers', async function () {
309-
this.timeout(60000);
310-
311344
const device = new Device({ id: 0xA, loopback: true });
312345
device.eds.addSdoClientParameter(device.id);
313346
device.eds.addSdoServerParameter(device.id);
@@ -317,8 +350,9 @@ describe('Sdo', function () {
317350
for (let i = 0; i < data.length; ++i)
318351
data[i] = Math.floor(Math.random() * 0xff);
319352

320-
device.protocol.sdoClient.blockSize = 1;
321-
device.protocol.sdoServer.blockSize = 1;
353+
device.sdo.setBlockSize(127);
354+
device.sdoServer.setBlockSize(127);
355+
device.sdoServer.setBlockInterval(0);
322356

323357
device.eds.addEntry(0x1234, {
324358
parameterName: 'A long buffer',
@@ -332,6 +366,7 @@ describe('Sdo', function () {
332366
dataType: DataType.DOMAIN,
333367
index: 0x1234,
334368
blockTransfer: true,
369+
blockInterval: 0,
335370
});
336371

337372
const result = await device.sdo.upload({

0 commit comments

Comments
 (0)