update SPI proposal

[soundanalogous]

Opening for comment. Updates to the SPI proposal per discussion in https://github.com/firmata/protocol/issues/26.

[fivdi]

Assuming that character special device files as described here are being used to access SPI devices on Linux, here is how the commands from the SPI proposal could be mapped to Linux.

SPI_BEGIN

• Does nothing on Linux as an SPI bus is initialized automatically at boot time

SPI_DEVICE_CONFIG

• opens file /dev/spidev<channel>.<deviceId> and saves the returned file descriptor for future ioctl() requests.
• Makes the appropriate ioctl() requests to configure dataMode, bitOrder, maxSpeed and csPinOptions on /dev/spidev<channel>.<deviceId>

SPI_BEGIN_TRANSACTION

• Does nothing on Linux

SPI_END_TRANSACTION

• Does nothing on Linux

SPI_TRANSFER

• Makes an ioctl() request to perform the full-duplex write/read transfer on /dev/spidev<channel>.<deviceId>

SPI_WRITE

• Makes an ioctl() request to perform the write on /dev/spidev<channel>.<deviceId>

SPI_READ

• Makes an ioctl() request to perform the read on /dev/spidev<channel>.<deviceId>

SPI_REPLY

• The data received by SPI_TRANSFER or SPI_READ

SPI_END

• Closes the file descriptors associated with the channel

[fivdi]

The default bitOrder is LSBFIRST. Give that the Raspberry Pi only supports MSBFIRST I would assume that most devices are MSBFIRST. This makes me wonder whether the default of LSBFIRST is correct. Would a default of MSBFIRST not be better?

[fivdi]

The proposal for SPI_DEVICE_CONFIG contains the following paragraph:

A chip select pin (csPin) can optionally be specified. This pin will be controlled per the rules specified in csPinOptions. For uncommon use cases of CS or other required HW pins per certain SPI devices, it is better to control them separately by not specifying a CS pin and instead using Firmata DIGITAL_MESSAGE to control the CS pin state. If a CS pin is specified, the csPinOptions for that pin must also be specified.

On Linux the csPin is normally configured automatically at boot time and controlled automatically by the SPI driver. In this case userspace code doesn't have anything to do with the csPin and I wouldn't expect that it be necessary to specify what the csPin is in an SPI_DEVICE_CONFIG command on Linux. However, it's still necessary to specify what the csPinOptions options are on Linux.

On the other hand, if the csPin is not specified, the semantics for SPI_DEVICE_CONFIG is that usespace code is responsible for controlling the csPin.

In addition, Linux does have a noChipSelect option. If noChipSelect is set, Linux has nothing to do with the csPin. Either there is no csPin or the csPin is controlled by userspace code. At least I think this is how it works but I'm not 100% sure as I have never actually used the Linux noChipSelect option.

Something isn't quire right here yet. It's not clear to me how an SPI_DEVICE_CONFIG command can be used to to tell Linux that the csPin either doesn't exist or is controlled by userspace code. If userspace code was forced to specify what the csPin is on Linux then it would work.

I'll have to think about this a bit and experiment with the Linux noChipSelect to see if it does what I think it does.

One last point related to csPin. csPin is optional so some encoding csPin will be used to specify that there is no csPin. No matter what encoding is used, there may be a platform where that encoding is a valid pin number.

[soundanalogous]

I could flip the order of csPin and csPinOptions and then make one option "no cs pin control":

...
11: csPinOptions          bit 0: CS_PIN_ENABLED (0 = disabled, 1 = enabled)
                          bit 1: CS_ACTIVE_STATE (0 = Active LOW (default)
                                                  1 = Active HIGH)
                          bits 2-6: reserved for future options
12: csPin                 [optional] (0-127) The chip select pin number
12|13: END_SYSEX

I'm not sure if "enabled/disabled" is the right wording yet. It can be clarified in the description. That way specifying a pin number can still be optional even if the active state may be required for some platforms. And if bit 0 is 0 that's a signal for platforms like Arduino that the user will manually control the CS pin state and a csPin does not need to be specified and if one is, it is ignored.

[soundanalogous]

The default bitOrder is LSBFIRST

This was mapping to Arduino.

EDIT: I should change it to MSBFIRST though since even the Arduino SPI library documentation says most devices use MSBFIRST.

[soundanalogous]

On Linux the csPin is normally configured automatically at boot time and controlled automatically by the SPI driver

I wonder how reliable automatic control is. For example, how does the auto pin handling know what the users intention is in the following typical scenario where multiple commands are sent between the cs pin toggle states:

// pseudocode
1. SPI.csPin(LOW)
2. SPI.transfer(read_address)
3. result = SPI.transfer(bytes_to_transfer)
4. SPI.csPin(HIGH)
5. return result

I assume then for Linux the CS pin is probably automatically toggled once per SPI.transfer so the read_address would be byte 0 in the array and there is only a single call to SPI.transfer and any call to SPI.transfer is framed by a toggle of the CS pin.

[soundanalogous]

One thing that may be missing from the SPI proposal is an optional delay between when the CS pin is set to the Active state and when SPI.transfer is called. I have seen various delays specified in different device datasheets. Usually they are no more than a couple hundred microseconds. If needed that could be an additional parameter (at least 2 bytes) in SPI_CONFIG_DEVICE. If the value is zero, ignore it (no delay) if > 0 use that delay. Tricky thing then is microseconds vs milliseconds since the microsecond value could be high if a device ever requires a long delay (thus requiring multiple bytes for the delay parameter).

[fivdi]

EDIT: I should change it to MSBFIRST though since even the Arduino SPI library documentation says most devices use MSBFIRST.

Good idea.

[fivdi]

Initial experiments with the Linux noChipSelect flag (aka SPI_NO_CS in C code) with the stock Raspbian driver on a Raspberry Pi indicate that the driver isn't particularly interested in this flag and continues to control the chip select pin even if this flag is set to true.

[soundanalogous]

Looking more into how to perform the following in Linux using ioctl:

// pseudocode
1. SPI.csPin(LOW)
2. SPI.transfer(read_address)
3. result = SPI.transfer(bytes_to_transfer)
4. SPI.csPin(HIGH)
5. return result

I see there is a cs_change parameter used when constructing a command to pass to SPI_IOC_MESSAGE. So it appears there is some work a user needs to do to get that sequence to work properly. Setting cs_change = 0 prevents the CS pin from being deselected between the first transfer in the pseudo code example (SPI.transfer(read_address)) and the subsequent transfer that gets the data. This actually maps well to the way I had csPinControl previously specified in the Firmata SPI proposal. In Linux, cs_change = 0 would map to cwPinControl = CS_DISABLE, but there is no mapping to CS_START_ONLY or CS_END_ONLY.

The other way to handle it (if we are to keep csPinControl out of SPI_TRANSFER) is to use SPI_TRANSACTION in linux to frame a "message" and for each SPI.transfer in that transaction, set cs_change = 0.

[fivdi]

I could flip the order of csPin and csPinOptions and then make one option "no cs pin control":

...
11: csPinOptions          bit 0: CS_PIN_ENABLED (0 = disabled, 1 = enabled)
                         bit 1: CS_ACTIVE_STATE (0 = Active LOW (default)
                                                 1 = Active HIGH)
                         bits 2-6: reserved for future options
12: csPin                 [optional] (0-127) The chip select pin number
12|13: END_SYSEX

I'm not sure if "enabled/disabled" is the right wording yet. It can be clarified in the description. That way specifying a pin number can still be optional even if the active state may be required for some platforms. And if bit 0 is 0 that's a signal for platforms like Arduino that the user will manually control the CS pin state and a csPin does not need to be specified and if one is, it is ignored.

Yes, that would resolve the issue.

[soundanalogous]

If I keep csPinControl I could actually eliminate the CS_START_ONLY and CS_END_ONLY options, keeping only CS_DISABLE (perhaps better renamed as CS_DESELECT). Then for every call to transfer, always set the CS pin to the active state (happens automatically already on Linux, would do in firmware for Arduino). There's no harm in doing this on every call since it's the same state. Then deselect at the end of the transfer only if CS_DESELECT is true. That would work for both Arduino and Linux.

EDIT: CS_DESELECT would default to true (matches Linux) since that's probably the more common use case. The user can override it by setting to 0.

[fivdi]

On Linux the csPin is normally configured automatically at boot time and controlled automatically by the SPI driver

I wonder how reliable automatic control is. For example, how does the auto pin handling know what the users intention is in the following typical scenario where multiple commands are sent between the cs pin toggle states:

// pseudocode

1. SPI.csPin(LOW)
2. SPI.transfer(read_address)
3. result = SPI.transfer(bytes_to_transfer)
4. SPI.csPin(HIGH)
5. return result

I assume then for Linux the CS pin is probably automatically toggled once per SPI.transfer so the read_address would be byte 0 in the array and there is only a single call to SPI.transfer and any call to SPI.transfer is framed by a toggle of the CS pin.

It's reliable in my opinion.

On Linux the pseudo code would look a little different.

// Linux pseudocode
typedef struct transfer_t {
  int length;
  int sendBuffer[64]
  int receiveBuffer[64]
  boolean cs_change // true to deselect device before starting the next transfer, default false
  int delay_usecs // delay after the last bit transfer before optionally deselecting the device before the next transfer, default 0
} transfer_t;

transfer_t array_of_transfers[2]

// fill read_address here, IMPORTANT cs_change should be false to prevent device deselect
// fill bytes_to_transfer here

array_of_transfers[0] = read_address
array_of_transfers[1] = bytes_to_transfer

SPI.doOneOrMoreTransfers(array_of_transfers, 2) // Do 2 transfers as a transaction

On Linux a single ioctl() request can perform multiple transfers as single transaction. Each transfer contains information specifying whether the CS pin should deselect the device before the next transfer (cs_change). It also contains a microsecond delay (delay_usecs).

Before the first transfer the device is selected. After the last transfer the device is deselected.

If I remember correctly the default max. size of a single transfer is 64 bytes and single ioctl() request can perform multiple transfers as long as the total length of all transfers doesn't exceed 4095 bytes.

Up until now my assumption was that you are not really interested in these Linux transactions because they can result in what would be large amounts of RAM being needed on Arduino UNO. My assumption here may be incorrect.

Perhaps the readme for this repo describes what's possible on Linux a littel better.

[fivdi]

The big difference between the Arduino transactions and the Linux transaction is that the Arduino transactions allow user code to execute between each transfer. On Linux this is not the case.

[soundanalogous]

My main concern with Arduino is CS pin handling and finding a way here that it can also well with Linux as well. That's the main hangup I've had with this SPI proposal and why it's taking me years to finalize it (well that time duration is mainly based on lack of feedback in the past).

[soundanalogous]

So when SPI support is included in an io-plugin for johnny-five I assume a SPI.transfer(params...) interface would be consistent regardless if the user has an Arduino or a Linux-compatible board right? Or is there flexibility in what is passed to the transfer method, such as passing a Message object for Linux and an array for Arduino? This all plays into how I eventually end up controlling the CS pin for Arduino applications since I'm trying to avoid having the user control the pin manually via separate DIGITAL_MESSAGE commands if possible.

[fivdi]

That's the main hangup I've had with this SPI proposal and why it's taking me years to finalize it (well that time duration is mainly based on lack of feedback in the past).

You're getting there, I can see light at the end of the tunnel :smile:

So when SPI support is included in an io-plugin for johnny-five I assume a SPI.transfer(params...) interface would be consistent regardless if the user has an Arduino or a Linux-compatible board right?

This is the way it is with I2C. I would imagine that it's a goal for SPI too. That being said, I haven't seen any discussions related to how SPI will be integrated into Johnny-Five.

Or is there flexibility in what is passed to the transfer method, such as passing a Message object for Linux and an array for Arduino?

To the best of my knowledge Johnny-Five doesn't know whether an Arduino or a Linux board is being used. Maybe Johnny-Five can pass an object containing everything that needed for both?

[fivdi]

Looking again at the following pseudo code:

// pseudocode
1. SPI.csPin(LOW)
2. SPI.transfer(read_address)
3. result = SPI.transfer(bytes_to_transfer)
4. SPI.csPin(HIGH)
5. return result

Is it possible to introduce a restrictions like this:

// pseudocode
1. create array/object read_address
2. create array/object bytes_to_transfer
3. // restriction: under absolutely no circumstances will the content of read_address be modified
4. // restriction: under absolutely no circumstances will the content of bytes_to_transfer be modified
5. SPI.csPin(LOW)
6. SPI.transfer(read_address)
7. // restriction: under absolutely no circumstances will the data read by the last transfer be looked at
8. result = SPI.transfer(bytes_to_transfer)
9. // restriction: under absolutely no circumstances will the data read by the last transfer be looked at
10. SPI.csPin(HIGH)
11. return result
12. // It's now possible to look at the data read by any transfer

The goal of these restrictions is to prevent the data read by the first transfer from being used to determine what's sent in the second transfer.

If these restrictions are possible, Linux transactions could also be used. In other words, all the transfers could be constructed up front before the first one is even performed. For an Arduino the transfers would be performed one by one. On Linux they would be performed by one ioctl() request.

[soundanalogous]

Arduino has a buffer option for SPI transfers so they don't need to be sent one-by-one, they just can't exceed about 30 bytes when using Firmata with an Arduino over Serial.

Here's a prototype I made a while back based on the prior SPI Firmata proposal: https://github.com/firmata/firmata.js/blob/spi-alpha/examples/spiAdxl345.js

That demonstrates the level of simplicity I'm hoping to enable on the client side. The readRegister and writeRegister methods. mask the weirdness of the pin control options. That (pin control options) approach also avoids having to send separate DIGITAL_MESSAGE commands to frame every transaction which helps reduce traffic on the transport (especially when using Serial).

[soundanalogous]

And this is the Arduino implementation of that prototype: https://github.com/firmata/arduino/blob/spi-alpha/utility/SPIFirmata.cpp

[fivdi]

Would it help if there was a Linux prototype?

I've used this io-plugin for prototyping in the past, it could be extended to support SPI. For SPI on Linux this module could be used.

If I'm not mistaken implementing a prototype would basically mean implementing the Board.prototype.spi* methods from this commit as equivalent PiIO.prototype.spi* methods here.

EDIT: The word "equivalent" above should be taken with a grain of salt as we already know that some of the SPI commands from the proposal can't really be mapped to Linux.

[soundanalogous]

Either a prototype or play through some method signatures. Here is an example with 2 options for what I think could work well, at least when interfacing with an Arduino board. Option A has an advantage of sending fewer Firmata messages over the wire, but Option B has a slightly cleaner API IMHO, but my preference is still for Option A.

// updated method signatures (with option param examples) since last commit:
//
// board.spiBegin(channel); // optional
// board.spiEnd(channel);
//
// Board.prototype.spiDeviceConfig({
//   channel: 0,
//   deviceId: 0,
//   bitOrder: board.SPI_BIT_ORDER.MSBFIRST, // default
//   dataMode: board.SPI_DATA_MODES.MODE0,
//   maxClockSpeed: 2500000,
//   wordSize: 8, // default
//   csActiveState: SPI_CS_ACTIVE_STATE.LOW, // default (or disabled = default?)
//   csPin: 4, // optional
// });
//
// board.spiBeginTransaction(channel, deviceId); // only for Option B below, no-op for Linux
// board.spiEndTransaction(channel, deviceId); // only for Option B below, no-op for Linux
//
// board.spiWrite(inBytes, {
//   channel: 0,
//   deviceId: 0,
//   csPinEnabled: false, // default (only for Option A below)
// });
//
// board.spiRead(numBytesToRead, {
//   channel: 0,
//   deviceId: 0,
//   csPinEnabled: false, // default (only for Option A below)
// }, callback);
//
// board.spiTransfer(inBytes, {
//   channel: 0,
//   deviceId: 0,
//   csPinEnabled: false, // default (only for Option A below)
// }, callback);

/**********************************************************************/

// Read register Option A, implicit transactions (begin/end transaction would be
// handled in the Arduino code based on csPinEnabled state).
// 2 Firmata messages (no begin/end transaction needed with this option)
this.spiWrite(register, {
  channel: 0,
  deviceId: 0,
  csPinEnabled: false, // default (don't toggle CS pin)
});
this.spiRead(numBytesToRead, {
  channel: 0,
  deviceId: 0,
  csPinEnabled: true, // toggle CS pin
}, callback);

// Option A as a spi transfer instead of separate write/read
// write to a register (8 bit) and read 6 bytes
// 1 Firmata message (if total message < 64 bytes), but user needs to parse
// data out of return (throw away first returned byte)
// set csPinEnabled: false to link multiple spiTransfer messages as a single transaction
this.spiTransfer(
  [register, 0, 0, 0, 0, 0, 0],
  {
    channel: 0,
    deviceId: 0,
    csPinEnabled: true, // toggle CS pin at end of transfer
  },
  callback
);

/**********************************************************************/

// Read register Option B, explicit transactions (Arduino)
// 4 Firmata messages (CS pin set to active state in begin, toggled in end)
// write and read are simpler, but at the expense of transmitting
// 2 additional Firmata messages
this.spiBeginTransaction(channel, deviceId); // no-op for Linux
this.spiWrite(register, {channel: 0, deviceId: 0});
this.spiRead(numBytesToRead, {channel: 0, deviceId: 0}, callback);
this.spiEndTransaction(channel, deviceId); // no-op for Linux

// Option B as a spi transfer
// 3 Firmata messages (CS pin set to active state in begin, toggled in end)
this.spiBeginTransaction(channel, deviceId); // no-op for Linux
this.spiTransfer(
  [register, 0, 0, 0, 0, 0, 0],
  {
    channel: 0,
    deviceId: 0
  },
  callback
);
this.spiEndTransaction(channel, deviceId); // no-op for Linux

[fivdi]

A initial commit for a Linux SPI prototype can be seen here. It's minimalistic and still needs work but it functions correctly. It should be relatively easy to add the missing pieces later.

• There are implementations for spiDeviceConfig and spiTransfer.
• The implementations of spiDeviceConfig and spiTransfer are not complete yet, there is no error checking yet, arguments that are optional are not necessarily optional yet, ...
• spiBegin, spiEnd, spiBeginTransaction and spiEndTransaction don't do anything.
• The example lis3dh.js runs and functions correctly.

Observations/Questions

(1) In the API proposed here sometimes channel and deviceId appear to be optional but sometimes they appear to be required. For example, it looks like they are optional for spiTransfer but required for spiBeginTransaction. Should the same options not always be optional?

(2) Why is continuous read needed for I2C but not needed for SPI? This isn't actually an issue for Linux, I'm just curious as to why it's not needed for SPI.

(3) Controlling the CS pin manually from user code on Linux will be quite difficult. If we assume that methods like spiRead, spiWrite and spiTransfer are async then it's going to be difficult to know when to change the state of the CS pin from user code. For example, if an async spiWrite method is called when exactly should the state of the CS pin be changed by user code? There may already be several previous calls to async methods that have not completed yet that are queued waiting to be run (the Linux SPI driver queues them.) Consider the following code:

board.spiWrite(inBytes1);
board.spiWrite(inBytes2);

Lets say that user code should set the CS pin low before spiWrite and high after spiWrite. The code could be modified to this:

board.digitalWrite(CS_PIN, 0);
board.spiWrite(inBytes1);
board.digitalWrite(CS_PIN, 1);

board.digitalWrite(CS_PIN, 0);
board.spiWrite(inBytes2);
board.digitalWrite(CS_PIN, 1);

However, this will not work. digitalWrite is typically a synchronous method on Linux. In Pi-IO digitalWrite is non-blocking and each call takes about 500 nanoseconds to execute. The four calls to digitalWrite above may have completed before the first byte of data from the first async spiWrite has been written. In addition, the async spiWrite method doesn't have a completion callback that could be used to help synchronize things. To be honest, I don't know if it will be necessary to control the CS pin from user code on Linux so I don't know if this will be an issue.

(4) The events emitted when spiTransfer and spiRead have completed are named with the pattern "spi-data-deviceid-channel". For device 0 on channel 0 the event name is always "spi-data-0-0". Lets say the user runs the following code:

  board.io.on('spi-data-0-0', function (reply) {
    // reply is an array of bytes but to which spiTransfer does it correspond?
  });

  // The WHO_AM_I register contains the device id which should be 0x33
  var readAddress = register.WHO_AM_I | READ_BIT;
  board.io.spiTransfer([readAddress, 0], transferOpts, function(data) {
    console.log('Device ID: 0x' + data[1].toString(16));
  });

  // Data rate 50Hz, normal mode, X, Y and Z axis enabled
  board.io.spiTransfer([register.CTRL_REG1, 0x47], transferOpts);

  // Block data update enabled, +/- 2G, high resolution enabled
  board.io.spiTransfer([register.CTRL_REG4, 0x88], transferOpts);

If the event name is always "spi-data-0-0" and the reply is simply an array of bytes how can the user know which spiTransfer or spiRead the reply corresponds to?

Note that the event names for I2C in similar scenarios include the register number.

 var event = 'I2C-reply' + address + '-' + (register !== null ? register : 0);

However, with SPI including a register number will not be enough. Some SPI devices don't even have registers as such, for example the MCP3008 analog to digital converter.

(5) I think there's a bug in the following firmata code from spiTransfer:

  if (callback) {
    this.once("spi-data-" + this.spiDeviceIds[channel] + "-" + channel, callback);
  }

If spiTransfer is called with a callback twice in quick succession then two handlers will be registered. When the first spiTransfer completes, both handlers will be run. The handler for the second call to spiTransfer will be run when the first call completes rather than when the second call completes.

(6) I attempted to implement an example using an ADXL345 similar to this one but I couldn't manage to get the ADXL345 to work on a Raspberry Pi 2.

[soundanalogous]

(1) In the API proposed here sometimes channel and deviceId appear to be optional but sometimes they appear to be required. For example, it looks like they are optional for spiTransfer but required for spiBeginTransaction. Should the same options not always be optional?

Sloppy work on my part. They should always be optional.

(2) Why is continuous read needed for I2C but not needed for SPI? This isn't actually an issue for Linux, I'm just curious as to why it's not needed for SPI.

Read continuous was added as a convenience for I2C many years ago. I just hadn't considered also adding it for SPI yet, but I could if there is a good case for it.

(3) Controlling the CS pin manually from user code on Linux will be quite difficult.

One of the main things I'm getting at, but maybe I'm not making myself clear is that I want to eliminate manual control of the CS pin since it requires sending too many Firmata messages in quick succession. That's why I had proposed the adding the csPinEnabled parameter so spiWrite, spiRead and spiTransfer. csPinEnabled would map to cs_change in Linux and would enable sequencing commands into messages for both Arduino and Linux implementations.

the async spiWrite method doesn't have a completion callback that could be used to help synchronize things

I could add a callback, but spiWrite with a callback would essentially just be spiTransfer.

If the event name is always "spi-data-0-0" and the reply is simply an array of bytes how can the user know which spiTransfer or spiRead the reply corresponds to?

I guess I had never run into that issue in the Arduino implementation. I could see it becoming a problem if there was an error though even in an Arduino implementation. One way to handle this is adding one more param which is an ID for each call to spiTransfer that can be used match the reply correctly. Maybe the ID is 7 bits and rolls over at 128 and it could be incremented automatically during each call spiTransfer so the user would never need to deal with it directly.

(5) I think there's a bug in the following firmata code from spiTransfer:

Possibly. I wasn't sure if this.once automatically deregistered the previous call if a new call was executed. If that's not the case then it is definitely a bug. However it's another issue the ID value I mentioned above would fix.

(6) I attempted to implement an example using an ADXL345 similar to this one but I couldn't manage to get the ADXL345 to work on a Raspberry Pi 2.

I don't even have an ADXL345 to test with. I just wrote that code as an example since it's a popular sensor, but I only had an old LIS3LV02DQ accelerometer which I'm pretty sure not many people have.

[soundanalogous]

There is one thing about the Linux implementation that make me nervous. From what you are saying, it sounds like there is no way to guaranteed the order in which bytes are written to the spi device when sending two or more spiTransfer messages in quick succession. Seems this could be a big issue if a particular spi device expects commands in a specific order. It it possible to use a synchronous write for spiTransfer, spiRead and spiWrite to guarantee data is written to the spi device in the proper order?

I guess this is potentially a JavaScript issue and not just a Linux issue, but I haven't run into it specifically when using firmata.js with an Arduino.

[fivdi]

One of the main things I'm getting at, but maybe I'm not making myself clear is that I want to eliminate manual control of the CS pin since it requires sending too many Firmata messages in quick succession.

Ok. Manual control isn't needed on Linux with the approach that I'm following.

I could add a callback, but spiWrite with a callback would essentially just be spiTransfer.

It wouldn't be fully like spiTransfer. It wouldn't read data like spiTransfer does. The advantage of the completion callbacks is that it gives the caller better control over things. I think spiWrite should have a completion callback.

I guess I had never run into that issue in the Arduino implementation. I could see it becoming a problem if there was an error though even in an Arduino implementation. One way to handle this is adding one more param which is an ID for each call to spiTransfer that can be used match the reply correctly. Maybe the ID is 7 bits and rolls over at 128 and it could be incremented automatically during each call spiTransfer so the user would never need to deal with it directly.

This would make it possible to distinguish the different operations from one another. I'm not sure it will be entirely adequate for for Johnny-Five. Currently Johnny-Five can determine the exact name of an event upfront. This would no longer be possible. There would need to be a way to get the event name.

Possibly. I wasn't sure if this.once automatically deregistered the previous call if a new call was executed. If that's not the case then it is definitely a bug.

Invoking once doesn't remove any previously registered listeners. It adds a new one.

There is one thing about the Linux implementation that make me nervous. From what you are saying, it sounds like there is no way to guaranteed the order in which bytes are written to the spi device when sending two or more spiTransfer messages in quick succession. Seems this could be a big issue if a particular spi device expects commands in a specific order.

There's no need to be nervous :smile: I intend using mutexify to guarantee the order. There will be a mutex for each SPI device which will guarantee that all operations for a device will be run in the order in which they were invoked. The mutex will be used for all operations, not just writes.

It it possible to use a synchronous write for spiTransfer, spiRead and spiWrite to guarantee data is written to the spi device in the proper order?

I don't think it's a good idea to use synchronous methods for SPI. The user has a lot of control over the baud rate. They could set the baud rate to a low value, say 1kHz or 10kHz, and then continuously call spiTransfer. The JavaScript thread in the Node.js process would then be blocked a lot of the time.

[soundanalogous]

I think spiWrite should have a completion callback.

So I could send a SPI-REPLY for spiWrite that is essentially 0x01 for success and 0x00 for failure (based on a timeout or other signal). Does that sound like the right approach?

Currently Johnny-Five can determine the exact name of an event upfront. This would no longer be possible. There would need to be a way to get the event name.

johnny-five simply passes through the firmata.js API for board level features like i2c, uart, etc. However I don't know how that works exactly for Linux so perhaps that is where the problem lies? Otherwise I think the following changes would work:

// within board.prototype.spiTransfer...
var numBytes = inBytes.length;
// this would really be in a separate function and used for spiRead and spiWrite as well
this.transferId = this.transferId >= 127 ? 0 : ++this.transferId;
var data = [
  START_SYSEX,
  SPI_DATA,
  SPI_TRANSFER,
];
data.push(channel);
data.push(this.transferId); // new param
data.push(pinControl);
data.push(numBytes);
if (inBytes) {
  for (var i = 0, len = numBytes; i < len; i++) {
    data.push(inBytes[i] & 0x007F);
    data.push((inBytes[i] >> 7) & 0x007F);
  }
}
data.push(END_SYSEX);
if (numBytes > 0) {
  writeToTransport(this, data);
}

if (callback) {
  // I flipped channel and deviceId
  this.once("spi-data-" + channel + "-" + this.spiDeviceIds[channel] + "-" + this.transferId, callback);
}

// and the transferId comes back in the reply:
SYSEX_RESPONSE[SPI_DATA] = function(board) {
  var command = board.currentBuffer[2];
  var deviceId = board.currentBuffer[3] >> 2;
  var channel = board.currentBuffer[3] & 0x03;
  var transferId = board.currentBuffer[4]; // new param
  var numBytes = board.currentBuffer[5];
  var reply = [];

  if (command === SPI_REPLY) {
    for (var i = 6, len = board.currentBuffer.length; i < len - 1; i += 2) {
      reply.push((board.currentBuffer[i + 1] << 7) | board.currentBuffer[i]);
    }
    // I flipped channel & deviceId
    board.emit("spi-data-" + channel + "-" + deviceId + "-" + transferId, reply);
  }
};

I intend using mutexify to guarantee the order.

Cool! I'll make note of that lib.

[fivdi]

Sorry about the delay in answering, I spent the last three days at a workshop.

So I could send a SPI-REPLY for spiWrite that is essentially 0x01 for success and 0x00 for failure (based on a timeout or other signal). Does that sound like the right approach?

If the SPI-REPLY that's sent for spiWrite here result in an event being emitted here and the event handler for this event results in the spiWrite completion callback being invoked then this sounds like the correct approach to me.

EDIT: I can now see from the above comment that SYSEX_RESPONSE[SPI_DATA] = function(board) emits the event I'd expect it to emit.

johnny-five simply passes through the firmata.js API for board level features like i2c, uart, etc. However I don't know how that works exactly for Linux so perhaps that is where the problem lies?

Up until today all I was aware of was that the Johnny-Five IO-Plugin specification states that IO-Plugins must emit events with specific names at specific times. For example, the IO-Plugin specification states that an i2c-reply-${address}-${register} event must be emitted when an i2cRead has completed.

I just had a look at the Johnny-Five Board class and now realize that I incorrectly assumed that these events were important for some aspect of the Johnny-Five implementation. This is not the case. Johnny-Five itself doesn't appear to have any interest in these events at all.

Emitting these events on Linux is not an issue and can be handled easily. For example, the i2c-reply-${address}-${register} event for i2cRead/i2creadOnce on Linux is emitted here in linux-io. pi-io inherits its I2C implementation from linux-io.

[dtex]

Johnny-Five itself doesn't appear to have any interest in these events at all.

That's a good observation. There are many firmata.js dependents other than Johnny-Five and emitting those events gives developers an option on how to architect their client code. By feature matching firmata.js we give those other clients the option to use other platforms. It's a great approach and I wish more people were aware that the IO-Plugins are not meant for just Johnny-Five.

[fivdi]

If a firmata.js dependent wants to add a one-time listener for event spi-data-${channel}-${deviceid}-${transferId} like this:

this.once("spi-data-" + channel + "-" + deviceId + "-" + transferId, callback);

how can they determine the value of transferId?

[dtex]

Is it possible that's just not practical? You could emit spi-data-${channel}-${deviceid} and let the client parse the message to determine the content. I haven't seen many SPI devices, but in the ones I've looked at the first byte (or-so) is the register address that was read.

Sorry if this should be obvious. I'm just coming up to speed on SPI.

[dtex]

Pinging @nebrius and @ajfisher because they have both shown interest in SPI in the past and should know that this conversation is happening.

[fivdi]

Is it possible that's just not practical?

Having thought about this a little longer, the problem is probably easy to solve. spiTransfer, spiRead, and spiWrite could simply return this.transferId. It would then be possible for the user to determine what the event name is, i.e., spi-data-${channel}-${deviceid}-${transferId}.

You could emit spi-data-${channel}-${deviceid} and let the client parse the message to determine the content.

See observation/question (4) in this comment for arguments as to why spi-data-${channel}-${deviceid}-${transferId} rather than spi-data-${channel}-${deviceid} is needed.

I haven't seen many SPI devices, but in the ones I've looked at the first byte (or-so) is the register address that was read.

I haven't seen a huge number either, however, the Raspberry Pi doesn't have any analog input pins so ADC requires additional hardware. A popular SPI device used for ADC is the MCP3008. The MCP3008 doesn't have registers.

[dtex]

I notice that the I2C methods all return this and I would expect SPI methods to behave the same in that regard.

firmata.js needs the transferId so it knows which callback to fire but the MCP3008 will be a device in the Expander class which will this.emit("analog-read-" + pin, value); from the callback. So just like with I2C, J5 wouldn't need the event.

Given that, maybe we should ping @rwaldron and see if that event is really necessary. Maybe it is just flotsam from an earlier implementation?

[dtex]

Also, thanks for your patience while I try to understand the challenges. I appreciate the additional explanation.

[soundanalogous]

I'm not sure why a developer would ever need to know the event name. I'd assume that may as well be private. All they need is to pass a callback to spiTransfer, spiWrite, or spiRead.

[soundanalogous]

Here's another SPI use case we should make sure the proposal covers. There are camera modules where you clock out an image which can be hundreds or thousands of bytes. Here's what that looks like in an Arduino sketch:

myCAM.CS_LOW();
myCAM.set_fifo_burst();

response = "--frame\r\n";
response += "Content-Type: image/jpeg\r\n\r\n";
server.sendContent(response);

static const size_t bufferSize = 1024; // 4096
static uint8_t buffer[bufferSize] = {0xFF};

while (len) {
    size_t will_copy = (len < bufferSize) ? len : bufferSize;
    SPI.transferBytes(&buffer[0], &buffer[0], will_copy);
    if (!client.connected()) break;
    client.write(&buffer[0], will_copy);
    len -= will_copy;
}
myCAM.CS_HIGH();

Notice how CS is pulled LOW at the beginning, then there are a bunch of SPI transfers in a loop and once the loop exits CS is pulled HIGH. What is important here is the entire series of SPI transfers needs to be bound by a single LOW to HIGH toggle of the CS pin. If trying something like this using Firmata, the buffer would be much smaller than 1024 bytes.

[fivdi]

I notice that the I2C methods all return this and I would expect SPI methods to behave the same in that regard.

Some of the firmata.js I2C methods return this and some don't. For example, i2cRead and i2cReadOnce do but sendI2CReadRequest and sendI2CWriteRequest don't. That being said, sendI2CReadRequest and sendI2CWriteRequest are deprecated.

firmata.js needs the transferId so it knows which callback to fire but the MCP3008 will be a device in the Expander class which will this.emit("analog-read-" + pin, value); from the callback. So just like with I2C, J5 wouldn't need the event.

@dtex Here you say that the spi-data-${channel}-${deviceid}-${transferId} events are more or less irrelevant from a Johnny-Five perspective which is correct. On the other hand, in this comment you mention that the same events are of great importance from a firmata.js perspective:

There are many firmata.js dependents other than Johnny-Five and emitting those events gives developers an option on how to architect their client code. By feature matching firmata.js we give those other clients the option to use other platforms. It's a great approach and I wish more people were aware that the IO-Plugins are not meant for just Johnny-Five.

I'm not sure why a developer would ever need to know the event name. I'd assume that may as well be private. All they need is to pass a callback to spiTransfer, spiWrite, or spiRead.

@soundanalogous I agree with what you say here. I, personally, would also use the completion callback and not the events. On the other hand, what you say here appears to contradict what @dtex says about the importance of these events for firmata.js dependents other than Johnny-Five.

To cut a long story short, if the spi-data-${channel}-${deviceid}-${transferId} events are a private implementation detail of firmata.js then we don't have an issue here. If they are public, then the user needs to be given a way to determine what the transferId is as the transferId will be needed to build the event name.

Also, please don't forget that the IO-Plugins specification has made events like i2c-reply... public irrespective of whether or not firmata.js regards them as private.

[dtex]

you mention that the same events are of great importance from a firmata.js perspective

I was simply musing on why the i2c-reply events might exist. Feature parity with firmata.js is the important thing, not these events. I should have separated those two thoughts more clearly.

It sounds like we agree that the transferId's do not need to be public for SPI because there is another way.

To maintain the pattern established by i2c we could keep spi-data-${channel}-${deviceid} which admittedly won't help when the SPI replies must be sorted, but it will work when the purpose of the data never changes or it can be derived from the data itself. In the case where replies must be sorted, the client will simply have to depend on the callback pattern.

I'm sorry if it sounds like I'm politicking for spi-data-${channel}-${deviceid}. It's just that as we talk it all out it becomes more clear to me that this makes sense.

[fivdi]

To maintain the pattern established by i2c we could keep spi-data-${channel}-${deviceid}

The patterns established for I2C are i2c-reply-${address}-${register} when a register is involved and i2c-reply-${address}-0 when no register is involved. ${address} in I2C serves the same purpose as ${deviceid} in SPI. What the I2C patterns lack is a channel, i.e., the I2C pattern can only handle one I2C bus. The fact that the I2C patterns lacks a channel has indirectly led to issues on Linux boards in my opinion, for example this one.

Why do you think that keeping spi-data-${channel}-${deviceid} is maintaining the pattern established by I2C? For me, spi-data-${channel}-${deviceid}-${transferId} would maintain the pattern established by I2C more closely. ${deviceid} would be similar to ${address} and ${transferId} would be similar to ${register}.

To maintain the pattern established by i2c we could keep spi-data-${channel}-${deviceid} which admittedly won't help when the SPI replies must be sorted, but it will work when the purpose of the data never changes or it can be derived from the data itself. In the case where replies must be sorted, the client will simply have to depend on the callback pattern.

I'm having difficulties coming up with a concrete example of how the purpose of the data can be derived from the data itself. Do you have a concrete example for some SPI device?

I'm sorry if it sounds like I'm politicking for spi-data-${channel}-${deviceid}. It's just that as we talk it all out it becomes more clear to me that this makes sense.

spi-data-${channel}-${deviceid}-${transferId} has less weaknesses and makes more sense to me. i2c-reply-${bus}-${address}-${register} would also make more sense to me but that's a different story :smile:

[fivdi]

Here's another SPI use case we should make sure the proposal covers. There are camera modules where you clock out an image which can be hundreds or thousands of bytes.

For arguments sake lets say the images from the camera module are 10000 bytes. I'm not familiar with the camera module but think that there are two ways this could be handled well on Linux.

Alternative One

The default max SPI message size on Linux is typically 4096 bytes. On a Raspberry Pi the max SPI message size can be seen in /sys/module/spidev/parameters/bufsiz

pi@raspberrypi:~ $ cat /sys/module/spidev/parameters/bufsiz
4096
pi@raspberrypi:~ $ 

A 4096 byte message is too small for a 10000 byte image so the max SPI message size can be increased from 4096 to 10000 by adding spidev.bufsiz=10000 to /boot/cmdline.txt. It would then be possible to read the image from the camera with one SPI transfer.

Alternative Two

On Linux an SPI message is actually one or more SPI transfers. This means that it's possible to send a message with multiple SPI transfers from a user space application to the SPI driver with one call and the SPI driver will take care of all the details and handle everything as one transaction. Here's an example of how this can be achieved with the spi-device module. The example sends a single message with three transfers to the SPI driver. 4096 + 4096 + 1808 = 10000.

const spi = require('spi-device');

const camera = spi.open(0, 1, {mode: spi.MODE0, maxSpeedHz: 500000}, function (err) {
  if (err) throw err;

  const message = [
    {receiveBuffer: Buffer.alloc(4096), byteLength: 4096, chipSelectChange: false},
    {receiveBuffer: Buffer.alloc(4096), byteLength: 4096, chipSelectChange: false},
    {receiveBuffer: Buffer.alloc(1808), byteLength: 1808, chipSelectChange: false}
  ];

  camera.transfer(message, function (err, message) {
    if (err) throw err;

    // The image data is now available in 
    // message[0].receiveBuffer - the first 4096 bytes of the image
    // message[1].receiveBuffer - the next 4096 bytes of the image
    // message[2].receiveBuffer - the last 1808 bytes of the image
  });
});

Note how the chipSelectChange boolean option is being used here. Setting it to true will deselect the device before starting the next transfer. false means that the device will not be deselected before starting the next transfer.

I actually think it would be good if firmata.js and Johnny-Five IO-Plugins supported messages with multiple transfers.

The following line of C code from the Arduino example was ignored as I'm not sure what the implementation of set_fifo_burst should do.

myCAM.set_fifo_burst();

It will also be necessary for an IO-Plugin to tell Johnny-Five the maximum size of an SPI message.

[soundanalogous]

Thinking more realistically about the camera module use case, for firmata.js to support messages with multiple transfers may actually be impractical (if not impossible) for the following reason:

The (Serial) buffer size is limited to 64 bytes which is only 29 bytes of actual data (64 byte serial buffer - 6 non data bytes per message) / 2. So a 10k image would require 345 SPI_TRANSFER Firmata messages (where "Firmata message" is different from a SPI message) and the user would have to reassemble all 345 SPI_REPLY Firmata messages back together. Probably not a good use case for Firmata over Serial after all.

So a Linux-based board where the application is running entirely on the actual board HW has a huge advantage over an Arduino Firmata application there. Obviously no problem to pull an image purely from an Arduino sketch, but the RPC nature of Firmata over Serial has serious limitations for sure.

[fivdi]

@soundanalogous @dtex we started this conversation about SPI on Linux boards like the Raspberry Pi here. I think I've provided all the feedback I can for the moment or do you see the need for additional feedback?

[dtex]

I think you have described the challenge and the reasoning for the transferId return value and the "fully qualified" event names really well. I went quiet because I finally "got it" and don't have a good alternative. The only argument I have is "It feels wrong to introduce this new return value pattern", which is not a very helpful argument.

[soundanalogous]

I don't think we need the return value. I agree that doesn't seem quite right. What we could do is make the transferId an optional parameter of the read, write and transfer methods. That way if the developer/user wants to know the value, they are responsible for creating it and passing it to the method. If a developer/user doesn't care then it's handled automatically internally. The developer/user just has to ensure that the value does not exceed 127, but we can also guard against that within the method.

[soundanalogous]

I think we've made great progress here. Thanks a ton for your input so far! I'll update the proposal per the comments in these 2 threads and then ping you two when I've made the changes so we can ensure we're all on the same page. I may not get to it for a few days.

[dtex]

In firmata.js we could let the user pass in any string or number as an id, push that onto a transferIds array, then use the indexOf that value on the array as our transferID for the firmata message. That way platforms that don't use the firmata protocol aren't restricted to the 0-127 values and we can have semantically meaningful event names.

[soundanalogous]

That string would need to be unique per each message for a small sequence. I'd prefer a number but for non Arduino platforms we don't need to limit it to the 0-127 range.

[soundanalogous]

I've pushed an update based on the comments in this and the other related thread. Changes I've made:

• Increased number of SPI channels (busses) from 4 to 8
• Decreased number of device IDs per channel from 32 to 16
• Moved csPinOptions above the csPin parameter in SPI_DEVICE_CONFIG and added a new CS_PIN_CONTROL bit that informs whether or not csPin should be ignored.
• Added requestId to TRANSFER, READ, WRITE and REPLY messages (we were previously calling this transferId in the thread)
• Indicated SPI_BEGIN, SPI_BEGIN_TRANSACTION and SPI_END_TRANSACTION are optional
• Indicated SPI_WRITE should return a SPI_REPLY with a value of 1 if successful, or a value of 0 if there was an error.

I also added an alternate proposal named spi-proplsal-no-transaction.md since I'm still a bit torn on whether or not the explicit SPI_BEGIN_TRANSATION and SPI_END_TRANSACTION messages are needed even for Arduino applications or if the same functionality can safely be handled implicitly. Removing these messages reduces the number of messages that need to be sent over the transport, but inferring them in the Arduino implementation could be tricky and there is a risk things could get out of sync regarding calls to beginTransaction and endTransaction. It will require slightly more memory as there are a couple of additional variables to track per device. This approach would also require a way to manage the csPin state in the TRANSFER, READ or WRITE messages. I've borrowed the cs_change (named csPinChange in the proposal) function from the Linux SPI api to handle that, where a value of 0 means deselect the pin and 1 means don't change the pin (default behavior).

[fivdi]

On Linux the cs_change option is only used when an SPI message contains multiple transfers. Lets say a message contains N transfers. Before the 1st transfer the csPin will always select the device and after the Nth transfer the csPin will always deselect the device. The csPin will deselect the device after the Nth transfer irrespective of whether the cs_change option for the Nth transfer is true or false. The cs_change option will only be taken into account for transfers 1 through N-1. This means that the cs_change options as described above will not be of usage on Linux.