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

Adds chip select delay option #209

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
55 changes: 42 additions & 13 deletions node/tessel-export.js
Original file line number Diff line number Diff line change
Expand Up @@ -917,6 +917,8 @@ Tessel.SPI = function(params, port) {

this.chipSelectActive = params.chipSelectActive === 'high' || params.chipSelectActive === 1 ? 1 : 0;

this.chipSelectDelay = (params.chipSelectDelayUs/1e3) || 0;
Copy link
Contributor

Choose a reason for hiding this comment

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


if (this.chipSelectActive) {
// active high, pull low for now
this.chipSelect.low();
Expand Down Expand Up @@ -969,11 +971,21 @@ Tessel.SPI = function(params, port) {
};

Tessel.SPI.prototype.send = function(data, callback) {
this._port.cork();

this.chipSelect.low();
this._port._tx(data, callback);
this.chipSelect.high();
this._port.uncork();

setTimeout(() => {
Copy link
Member

Choose a reason for hiding this comment

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

ideally you would only uncork if you wanted a delay, otherwise you're going to get 100us-1ms of delay whether you like it or not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

Copy link
Member

Choose a reason for hiding this comment

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

Your change isn't quite what I meant. In the case where you don't add a delay, you want to leave the entire transaction in one cork / uncork pair so it goes out in a single packet, but when you add a delay, you don't need cork/uncork, because you do want the commands separated.

// Only cork if we have specified a target delay, otherwise delay increases
if (this.chipSelectDelay > 0) {
this._port.cork();
}
this._port._tx(data, callback);
this.chipSelect.high();
if (this.chipSelectDelay > 0) {
this._port.uncork();
}
}, this.chipSelectDelay);
Copy link
Contributor

Choose a reason for hiding this comment

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

The setTimeout still has a cost—any other code called after send but before the end of the execution turn will now execute before any of this code is executed. I think this needs to be reconsidered

Copy link
Contributor

Choose a reason for hiding this comment

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


};

Copy link
Member

Choose a reason for hiding this comment

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

Did the chipSelect.high() get removed here?

Tessel.SPI.prototype.disable = function() {
Expand All @@ -984,19 +996,36 @@ Tessel.SPI.prototype.disable = function() {
};

Tessel.SPI.prototype.receive = function(data_len, callback) {
this._port.cork();

this.chipSelect.low();
this._port._rx(data_len, callback);
this.chipSelect.high();
this._port.uncork();

setTimeout(() => {
// Only cork if we have specified a target delay, otherwise delay increases
if (this.chipSelectDelay > 0) {
this._port.cork();
}
this._port._rx(data_len, callback);
if (this.chipSelectDelay > 0) {
this._port.uncork();
}
}, this.chipSelectDelay);
};

Tessel.SPI.prototype.transfer = function(data, callback) {
this._port.cork();

this.chipSelect.low();
this._port._txrx(data, callback);
this.chipSelect.high();
this._port.uncork();

setTimeout(() => {
// Only cork if we have specified a target delay, otherwise delay increases
if (this.chipSelectDelay > 0) {
this._port.cork();
}
this._port._txrx(data, callback);
this.chipSelect.high();
if (this.chipSelectDelay > 0) {
this._port.uncork();
}
}, this.chipSelectDelay);
};

Tessel.UART = function(port, options) {
Expand Down Expand Up @@ -1509,7 +1538,7 @@ function getWifiInfo() {
if (bcastMatches === null) {
recursiveWifi(network);
} else {
// Successful matches will have a result that looks like:
// Successful matches will have a result that looks like:
// ["Bcast:0.0.0.0", "Bcast", "0.0.0.0"]
if (bcastMatches.length === 3) {
network.ip = bcastMatches[2];
Expand Down
58 changes: 57 additions & 1 deletion node/test/unit/tessel.js
Original file line number Diff line number Diff line change
Expand Up @@ -2354,7 +2354,10 @@ exports['Tessel.SPI'] = {
return this.socket;
}.bind(this));

this.tessel = new Tessel();
// We want to disable other ports so that we can pass around mock data.
// Otherwise, port A and B get 'readable' events without having any
// queued callbacks (because they are queued on the port we create below).
this.tessel = new Tessel({ ports: {A: false, B: false} });

this.cork = sandbox.stub(Tessel.Port.prototype, 'cork');
this.uncork = sandbox.stub(Tessel.Port.prototype, 'uncork');
Expand Down Expand Up @@ -2432,6 +2435,59 @@ exports['Tessel.SPI'] = {
test.ok(this.spiDisable.calledOnce, true);

test.done();
},
chipSelectDelayOptions: function(test) {
test.expect(2);

var delayUs = 10000;
var usToMs = 1000;

var s1 = new this.port.SPI();

var s2 = new this.port.SPI({chipSelectDelayUs: delayUs});

test.equal(s1.chipSelectDelay, 0);

test.equal(s2.chipSelectDelay, delayUs/usToMs);

test.done();
},
chipSelectDelayFunctionality: function(test) {
test.expect(2);

var delayUs = 10000;
var usToMs = 1000;

var s1 = new this.port.SPI({chipSelectDelayUs: delayUs});

var timeoutSpy = sandbox.spy(global, 'setTimeout');

s1.transfer(new Buffer(1), () => {
// One timeout called within transfer and one in the test below
test.equal(timeoutSpy.callCount, 2);
// The first call (chip select delay) waited an appropriate amount of time)
test.equal(timeoutSpy.getCall(0).args[1], delayUs/usToMs);
// Restore setTimeout
timeoutSpy.restore();
test.done();
});

// Wait until after the chip select delay has passed
// and the callback was enqueued. Only return data on the first request
setTimeout(() => {
var called = false;
this.socket.read = () => {
if (called) {
return new Buffer([]);
}
called = true;

return new Buffer([0x84, 0x1]);
};

// Prod the socket to read our buffer
s1._port.sock.emit('readable');
}, (delayUs / 10) * 2 );
}
};

Expand Down