diff --git a/plugins/pi4j-plugin-ffm/src/main/java/com/pi4j/plugin/ffm/providers/spi/FFMSpi.java b/plugins/pi4j-plugin-ffm/src/main/java/com/pi4j/plugin/ffm/providers/spi/FFMSpi.java index 2f9d708b..0633d077 100644 --- a/plugins/pi4j-plugin-ffm/src/main/java/com/pi4j/plugin/ffm/providers/spi/FFMSpi.java +++ b/plugins/pi4j-plugin-ffm/src/main/java/com/pi4j/plugin/ffm/providers/spi/FFMSpi.java @@ -25,11 +25,32 @@ public class FFMSpi extends SpiBase implements Spi { private static final Logger logger = LoggerFactory.getLogger(FFMSpi.class); private static final String SPI_BUS = "/dev/spidev"; + + /** + * sysfs file exposing the maximum number of bytes the spidev driver accepts in a single + * SPI_IOC_MESSAGE transfer. Transfers larger than this value are rejected by the kernel with + * EMSGSIZE, so we read it once and split larger transfers into chunks of at most this size. + */ + private static final String SPIDEV_BUFSIZ_PATH = "/sys/module/spidev/parameters/bufsiz"; + + /** + * Fallback transfer chunk size used when {@link #SPIDEV_BUFSIZ_PATH} cannot be read. This matches + * the spidev driver's own compile-time default of 4096 bytes. + */ + private static final int DEFAULT_BUFFER_SIZE = 4096; + + /** + * The bufsiz value is text ASCII (not binary). It is an unsigned int, so the largest decimal + * representation is 10 bytes. + */ + private static final int MAX_BUFSIZ_FILE_SIZE = 10; + private final FileDescriptorNative FILE = new FileDescriptorNative(); private final IoctlNative IOCTL = new IoctlNative(); private int spiFileDescriptor; + private int bufferSize = DEFAULT_BUFFER_SIZE; private final String path; public FFMSpi(SpiProvider provider, SpiConfig config) { @@ -53,8 +74,6 @@ public Spi initialize(Context context) throws InitializeException { IOCTL.call(spiFileDescriptor, Command.getSpiIocWrMode(), config.mode().getMode()); logger.debug("{} - setting Read SPI Mode to {}.", path, config.mode()); IOCTL.call(spiFileDescriptor, Command.getSpiIocRdMode(), config.mode().getMode()); -// logger.debug("{} - setting Bit Ordering to {}.", path, config.?); -// IOCTL.call(spiFileDescriptor, Command.getSpiIocWrLsbFirst(), bitOrdering); logger.debug("{} - setting Write Byte Length to {}.", path, 8); IOCTL.call(spiFileDescriptor, Command.getSpiIocWrBitsPerWord(), 8); logger.debug("{} - setting Read Byte Length to {}.", path, 8); @@ -68,11 +87,45 @@ public Spi initialize(Context context) throws InitializeException { logger.debug("{} - setting readLsbFirst to {}.", path, config.getReadLsbFirst()); IOCTL.call(spiFileDescriptor, Command.getSpiIocRdLsbFirst(), config.getReadLsbFirst()); + this.bufferSize = readBufferSize(); + logger.debug("{} - SPI transfer chunk size (bufsiz) is {} bytes.", path, bufferSize); + this.isOpen = true; logger.info("{} - SPI Bus configured.", path); return this; } + /** + * Reads the spidev {@code bufsiz} module parameter, which is the maximum number of bytes accepted + * by the kernel in a single SPI_IOC_MESSAGE transfer. Falls back to {@link #DEFAULT_BUFFER_SIZE} + * when the sysfs file is missing, unreadable or holds an unexpected value. + * + * @return the maximum transfer chunk size in bytes + */ + private int readBufferSize() { + if (FILE.access(SPIDEV_BUFSIZ_PATH, FileFlag.R_OK) != 0) { + logger.debug("{} - '{}' is not accessible, falling back to default chunk size {}.", path, SPIDEV_BUFSIZ_PATH, DEFAULT_BUFFER_SIZE); + return DEFAULT_BUFFER_SIZE; + } + try { + var bufsizFd = FILE.open(SPIDEV_BUFSIZ_PATH, FileFlag.O_RDONLY); + var content = FILE.read(bufsizFd, new byte[MAX_BUFSIZ_FILE_SIZE], MAX_BUFSIZ_FILE_SIZE); + FILE.close(bufsizFd); + if (content == null || content.length == 0) { + return DEFAULT_BUFFER_SIZE; + } + var size = Integer.parseInt(new String(content).trim()); + if (size <= 0) { + logger.warn("{} - '{}' reported non-positive value {}, falling back to default chunk size {}.", path, SPIDEV_BUFSIZ_PATH, size, DEFAULT_BUFFER_SIZE); + return DEFAULT_BUFFER_SIZE; + } + return size; + } catch (RuntimeException e) { + logger.warn("{} - could not read '{}', falling back to default chunk size {}: {}", path, SPIDEV_BUFSIZ_PATH, DEFAULT_BUFFER_SIZE, e.getMessage()); + return DEFAULT_BUFFER_SIZE; + } + } + /** * {@inheritDoc} */ @@ -90,16 +143,26 @@ public int transfer(byte[] write, int writeOffset, byte[] read, int readOffset, checkClosed(); Objects.checkFromIndexSize(readOffset, numberOfBytes, read.length); Objects.checkFromIndexSize(writeOffset, numberOfBytes, write.length); - var writeData = Arrays.copyOfRange(write, writeOffset, numberOfBytes + writeOffset); - logger.trace("{} - Transferring data (length '{}')", path, numberOfBytes); + logger.trace("{} - Transferring data (length '{}') in chunks of at most {} bytes", path, numberOfBytes, bufferSize); logger.trace("{} - Write buffer: {}", path, HexFormatter.format(write)); - var spiTransfer = new SpiTransferBuffer(writeData, read, numberOfBytes); - spiTransfer = IOCTL.call(spiFileDescriptor, Command.getSpiIocMessage(1), spiTransfer); - var readBytes = spiTransfer.getRxBuffer(); - System.arraycopy(readBytes, 0, read, readOffset, numberOfBytes); + + // Split larger transfers into bufsiz-sized chunks, each issued as its own SPI_IOC_MESSAGE(1) + // ioctl call. These must NOT be batched into one SPI_IOC_MESSAGE(N): spidev's 'bufsiz' limit + // applies to the cumulative tx (and rx) total across all transfers within a single ioctl, so + // batching would sum the chunks back up and fail with EMSGSIZE - exactly what we avoid here. + var totalRead = 0; + for (var chunkOffset = 0; chunkOffset < numberOfBytes; chunkOffset += bufferSize) { + var chunkSize = Math.min(bufferSize, numberOfBytes - chunkOffset); + var writeData = Arrays.copyOfRange(write, writeOffset + chunkOffset, writeOffset + chunkOffset + chunkSize); + var spiTransfer = new SpiTransferBuffer(writeData, new byte[chunkSize], chunkSize); + spiTransfer = IOCTL.call(spiFileDescriptor, Command.getSpiIocMessage(1), spiTransfer); + var readBytes = spiTransfer.getRxBuffer(); + System.arraycopy(readBytes, 0, read, readOffset + chunkOffset, chunkSize); + totalRead += readBytes.length; + } logger.trace("{} - Read buffer: {}", path, HexFormatter.format(read)); - return readBytes.length; + return totalRead; } /** @@ -139,20 +202,57 @@ public void writeThenRead(byte[] write, int readDelayNanos, byte[] read) { */ @Override public void writeThenRead(byte[] write, int writeOffset, int writeLength, int readDelayNanos, byte[] read, int readOffset, int readLength) { + checkClosed(); Objects.checkFromIndexSize(readOffset, readLength, read.length); Objects.checkFromIndexSize(writeOffset, writeLength, write.length); - var writeData = Arrays.copyOfRange(write, writeOffset, writeLength + writeOffset); - var inputBuffer = new SpiTransferBuffer(writeData, new byte[0], writeLength, readDelayNanos / 1000); - var outputBuffer = new SpiTransferBuffer(new byte[0], read, readLength, readDelayNanos / 1000); - var transferBuffer = new SpiMultipleTransferBuffer(inputBuffer, outputBuffer); - checkClosed(); - logger.trace("{} - Transferring data (length '{}')", path, writeLength); + logger.trace("{} - Write-then-read (write '{}', read '{}') with chunk size {}", path, writeLength, readLength, bufferSize); logger.trace("{} - Write buffer: {}", path, HexFormatter.format(write)); - transferBuffer = IOCTL.call(spiFileDescriptor, Command.getSpiIocMessage(2), transferBuffer); - var readBytes = transferBuffer.transferBuffer()[1].getRxBuffer(); - System.arraycopy(readBytes, 0, read, readOffset, readLength); + var delayUsecs = readDelayNanos / 1000; + + // Fast path: both halves fit within a single bufsiz, so the whole exchange runs as one + // SPI_IOC_MESSAGE(2) under a single chip-select assertion (write, delay, then read). The + // bufsiz limit is checked per direction (tx total vs rx total), so the write-only and + // read-only transfers are bounded independently. + if (writeLength <= bufferSize && readLength <= bufferSize) { + var writeData = Arrays.copyOfRange(write, writeOffset, writeOffset + writeLength); + // The unused direction must be null (not an empty array): a non-null pointer with a + // non-zero len makes the kernel copy len bytes into/out of a zero-length buffer. + var inputBuffer = new SpiTransferBuffer(writeData, null, writeLength, delayUsecs); + var outputBuffer = new SpiTransferBuffer(null, read, readLength, delayUsecs); + + var transferBuffer = new SpiMultipleTransferBuffer(inputBuffer, outputBuffer); + transferBuffer = IOCTL.call(spiFileDescriptor, Command.getSpiIocMessage(2), transferBuffer); + var readBytes = transferBuffer.transferBuffer()[1].getRxBuffer(); + System.arraycopy(readBytes, 0, read, readOffset, readLength); + + logger.trace("{} - Read buffer: {}", path, HexFormatter.format(read)); + return; + } + + // Chunked path: the write and/or read exceed spidev's bufsiz. They cannot share a single + // ioctl (the limit is the cumulative tx/rx total per SPI_IOC_MESSAGE call), so the write + // phase and then the read phase are issued as separate bufsiz-sized SPI_IOC_MESSAGE(1) + // calls. This releases chip-select between chunks - unavoidable once a transfer is larger + // than bufsiz; the only way to keep a single assertion is to raise the kernel's bufsiz. + for (var chunkOffset = 0; chunkOffset < writeLength; chunkOffset += bufferSize) { + var chunkSize = Math.min(bufferSize, writeLength - chunkOffset); + var writeData = Arrays.copyOfRange(write, writeOffset + chunkOffset, writeOffset + chunkOffset + chunkSize); + // apply the read delay only after the final write chunk, i.e. between the write and read phases + var chunkDelay = chunkOffset + chunkSize >= writeLength ? delayUsecs : 0; + // null rx (write-only): a non-null zero-length rx buffer would be overrun by the kernel + var spiTransfer = new SpiTransferBuffer(writeData, null, chunkSize, chunkDelay); + IOCTL.call(spiFileDescriptor, Command.getSpiIocMessage(1), spiTransfer); + } + + for (var chunkOffset = 0; chunkOffset < readLength; chunkOffset += bufferSize) { + var chunkSize = Math.min(bufferSize, readLength - chunkOffset); + // null tx (read-only): clocks out zeros without reading from a zero-length tx buffer + var spiTransfer = new SpiTransferBuffer(null, new byte[chunkSize], chunkSize); + spiTransfer = IOCTL.call(spiFileDescriptor, Command.getSpiIocMessage(1), spiTransfer); + System.arraycopy(spiTransfer.getRxBuffer(), 0, read, readOffset + chunkOffset, chunkSize); + } logger.trace("{} - Read buffer: {}", path, HexFormatter.format(read)); } diff --git a/plugins/pi4j-plugin-ffm/src/test/java/com/pi4j/plugin/ffm/integration/SPITest.java b/plugins/pi4j-plugin-ffm/src/test/java/com/pi4j/plugin/ffm/integration/SPITest.java index 80c07b29..879f40ff 100644 --- a/plugins/pi4j-plugin-ffm/src/test/java/com/pi4j/plugin/ffm/integration/SPITest.java +++ b/plugins/pi4j-plugin-ffm/src/test/java/com/pi4j/plugin/ffm/integration/SPITest.java @@ -13,8 +13,12 @@ import org.junit.jupiter.api.condition.EnabledOnOs; import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Random; import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.condition.OS.LINUX; @@ -63,4 +67,56 @@ public void testSPIRead() { spi.read(buffer); assertArrayEquals(new byte[]{0, 0, 0, 0}, buffer); } + + @Test + public void testSPILargeTransfer() { + // Transfer more bytes than the spidev 'bufsiz' so the data must be split into several + // SPI_IOC_MESSAGE chunks. Without chunking the kernel would reject this with EMSGSIZE. + var bufsiz = readBufsiz(); + var size = bufsiz * 2 + 137; + + var write = new byte[size]; + new Random(42).nextBytes(write); + var read = new byte[size]; + + var transferred = spi.transfer(write, 0, read, 0, size); + + assertEquals(size, transferred); + // the mock SPI controller echoes the tx buffer straight back into rx + assertArrayEquals(write, read); + } + + @Test + public void testSPIWriteThenRead() { + // single-chunk write-then-read: the mock controller stores the written bytes and echoes + // them back on the subsequent read. + var read = new byte[4]; + spi.writeThenRead("Test".getBytes(), 0, 4, 0, read, 0, 4); + assertEquals("Test", new String(read)); + } + + @Test + public void testSPILargeWriteThenRead() { + // Write and read more bytes than the spidev 'bufsiz' so both phases must be chunked into + // several SPI_IOC_MESSAGE calls. The assertion is only that the kernel does not reject the + // request with EMSGSIZE - the mock controller cannot echo back a payload this large, so the + // received content is not verified here (that is covered by the unit test). + var bufsiz = readBufsiz(); + var size = bufsiz * 2 + 137; + + var write = new byte[size]; + new Random(7).nextBytes(write); + var read = new byte[size]; + + assertDoesNotThrow(() -> spi.writeThenRead(write, 0, size, 0, read, 0, size)); + } + + private static int readBufsiz() { + try { + return Integer.parseInt(Files.readString(Path.of("/sys/module/spidev/parameters/bufsiz")).trim()); + } catch (Exception e) { + // spidev compile-time default when the parameter cannot be read + return 4096; + } + } } diff --git a/plugins/pi4j-plugin-ffm/src/test/java/com/pi4j/plugin/ffm/unit/SPITest.java b/plugins/pi4j-plugin-ffm/src/test/java/com/pi4j/plugin/ffm/unit/SPITest.java index b408fbaa..a7a0d3aa 100644 --- a/plugins/pi4j-plugin-ffm/src/test/java/com/pi4j/plugin/ffm/unit/SPITest.java +++ b/plugins/pi4j-plugin-ffm/src/test/java/com/pi4j/plugin/ffm/unit/SPITest.java @@ -8,6 +8,7 @@ import com.pi4j.plugin.ffm.common.spi.SpiMultipleTransferBuffer; import com.pi4j.plugin.ffm.common.spi.SpiTransferBuffer; import com.pi4j.plugin.ffm.mocks.FileDescriptorNativeMock; +import com.pi4j.plugin.ffm.mocks.FileDescriptorNativeMock.FileDescriptorTestData; import com.pi4j.plugin.ffm.mocks.IoctlNativeMock; import com.pi4j.plugin.ffm.mocks.PermissionHelperMock; import com.pi4j.plugin.ffm.providers.spi.FFMSpiProviderImpl; @@ -16,10 +17,17 @@ import org.junit.jupiter.api.Test; import org.mockito.MockedStatic; +import java.io.ByteArrayOutputStream; +import java.util.Arrays; +import java.util.concurrent.atomic.AtomicInteger; + import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; public class SPITest { + private static final String SPIDEV_BUFSIZ_PATH = "/sys/module/spidev/parameters/bufsiz"; + private static Context pi4j; private static final MockedStatic permissionHelperMock = PermissionHelperMock.echo(); @@ -133,8 +141,9 @@ public void testWriteThenRead() { var spiTestData = new IoctlNativeMock.IoctlTestData(SpiMultipleTransferBuffer.class, (answer) -> { SpiMultipleTransferBuffer buffer = answer.getArgument(2); var inputBuffer = buffer.transferBuffer()[0]; - var outputBuffer = buffer.transferBuffer()[1]; - return new SpiMultipleTransferBuffer(inputBuffer, new SpiTransferBuffer(outputBuffer.getTxBuffer(), inputBuffer.getTxBuffer(), outputBuffer.getTxBuffer().length, 1000)); + // the read transfer is read-only (null tx), so echo the written bytes back as its rx + var written = inputBuffer.getTxBuffer(); + return new SpiMultipleTransferBuffer(inputBuffer, new SpiTransferBuffer(null, written, written.length, 1000)); }); try (var _ = FileDescriptorNativeMock.setup(); var _ = IoctlNativeMock.setup(spiTestData)) { @@ -150,4 +159,124 @@ public void testWriteThenRead() { assertArrayEquals("Test".getBytes(), buffer); } } + + @Test + public void testChunkedTransfer() { + // bufsiz is reported as 8 bytes, so a 20 byte transfer must be split into 8 + 8 + 4. + var callCount = new AtomicInteger(); + var maxChunkSize = new AtomicInteger(); + var spiTestData = new IoctlNativeMock.IoctlTestData(SpiTransferBuffer.class, (answer) -> { + SpiTransferBuffer buffer = answer.getArgument(2); + callCount.incrementAndGet(); + maxChunkSize.accumulateAndGet(buffer.getTxBuffer().length, Math::max); + return new SpiTransferBuffer(buffer.getTxBuffer(), buffer.getTxBuffer(), buffer.getTxBuffer().length); + }); + var bufsiz = new FileDescriptorTestData(SPIDEV_BUFSIZ_PATH, 99, "8\n".getBytes()); + try (var _ = FileDescriptorNativeMock.setup(bufsiz); + var _ = IoctlNativeMock.setup(spiTestData)) { + + var spi = pi4j.spi().create(SpiConfigBuilder.newInstance() + .bus(SpiBus.BUS_0) + .channel(5) + .mode(0) + .baud(50_000) + .build()); + + var write = new byte[20]; + for (var i = 0; i < write.length; i++) { + write[i] = (byte) i; + } + var read = new byte[20]; + var result = spi.transfer(write, 0, read, 0, write.length); + + assertEquals(20, result); + assertArrayEquals(write, read); + // 20 bytes split into chunks of at most 8 bytes -> 8 + 8 + 4 = 3 ioctl calls + assertEquals(3, callCount.get()); + assertTrue(maxChunkSize.get() <= 8, "no chunk may exceed the reported bufsiz"); + } + } + + @Test + public void testChunkedTransferWithOffsets() { + // Verify that read/write offsets are honored while chunking. + var spiTestData = new IoctlNativeMock.IoctlTestData(SpiTransferBuffer.class, (answer) -> { + SpiTransferBuffer buffer = answer.getArgument(2); + return new SpiTransferBuffer(buffer.getTxBuffer(), buffer.getTxBuffer(), buffer.getTxBuffer().length); + }); + var bufsiz = new FileDescriptorTestData(SPIDEV_BUFSIZ_PATH, 99, "4".getBytes()); + try (var _ = FileDescriptorNativeMock.setup(bufsiz); + var _ = IoctlNativeMock.setup(spiTestData)) { + + var spi = pi4j.spi().create(SpiConfigBuilder.newInstance() + .bus(SpiBus.BUS_0) + .channel(6) + .mode(0) + .baud(50_000) + .build()); + + var write = new byte[]{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12}; + var read = new byte[12]; + // transfer 9 bytes starting at write index 2 into read starting at index 1 + var result = spi.transfer(write, 2, read, 1, 9); + + assertEquals(9, result); + var expected = new byte[]{0, 3, 4, 5, 6, 7, 8, 9, 10, 11, 0, 0}; + assertArrayEquals(expected, read); + } + } + + @Test + public void testChunkedWriteThenRead() { + // bufsiz is reported as 4 bytes, so a 10 byte write and 10 byte read must each be split into + // 4 + 4 + 2 separate SPI_IOC_MESSAGE(1) calls (write phase, then read phase). + var stored = new ByteArrayOutputStream(); + var readPos = new AtomicInteger(); + var writeChunks = new AtomicInteger(); + var readChunks = new AtomicInteger(); + var maxChunkSize = new AtomicInteger(); + var spiTestData = new IoctlNativeMock.IoctlTestData(SpiTransferBuffer.class, (answer) -> { + SpiTransferBuffer buffer = answer.getArgument(2); + var tx = buffer.getTxBuffer(); + var rx = buffer.getRxBuffer(); + if (tx != null && tx.length > 0) { + // write-only chunk (null rx): remember the bytes that were written + writeChunks.incrementAndGet(); + maxChunkSize.accumulateAndGet(tx.length, Math::max); + stored.write(tx, 0, tx.length); + return new SpiTransferBuffer(tx, null, tx.length); + } + // read-only chunk (null tx): echo back the next slice of what was previously written + readChunks.incrementAndGet(); + maxChunkSize.accumulateAndGet(rx.length, Math::max); + var all = stored.toByteArray(); + var pos = readPos.getAndAdd(rx.length); + var out = Arrays.copyOfRange(all, pos, pos + rx.length); + return new SpiTransferBuffer(null, out, out.length); + }); + var bufsiz = new FileDescriptorTestData(SPIDEV_BUFSIZ_PATH, 99, "4".getBytes()); + try (var _ = FileDescriptorNativeMock.setup(bufsiz); + var _ = IoctlNativeMock.setup(spiTestData)) { + + var spi = pi4j.spi().create(SpiConfigBuilder.newInstance() + .bus(SpiBus.BUS_0) + .channel(7) + .mode(0) + .baud(50_000) + .build()); + + var write = new byte[10]; + for (var i = 0; i < write.length; i++) { + write[i] = (byte) (i + 1); + } + var read = new byte[10]; + spi.writeThenRead(write, 0, write.length, 1000, read, 0, read.length); + + assertArrayEquals(write, read); + // 10 bytes split into chunks of at most 4 bytes -> 4 + 4 + 2 = 3 calls per phase + assertEquals(3, writeChunks.get()); + assertEquals(3, readChunks.get()); + assertTrue(maxChunkSize.get() <= 4, "no chunk may exceed the reported bufsiz"); + } + } }