linuxfs used for SPI operations. .. Most this code base is from mpil…

[taartspi]

…one. I had no access to Mikes repo so I copied the files to ours. From Mikes initial work I made a few fixes for code he had not unit tested, and changes to use of config parms. Also parm bit shift direction and LSB first are not complete. Question whether we update the spi config. I do not want to use/allow the flags parm as it has bits no longer supported and would confuse things. I did read write and transfer testing for basic operation. I know there is a problem when read() is called multiple times in succession. pushing this for discussion

I also had a PR for an implementation of SPI. Mikes and mine were similar in the user space code but I implemented the library as well. Mike's implementation use a system library, so less code to maintain so better use. I marked my PR as closed as I don't want to lose the JNA and C code I wrote

@eitch @FDelporte

[taartspi]

this is ready for review comments. I had SPI chips to test transfer, write byte, write array, read byte read array. As said earlier a few changes addition to complete code and Config usage. In LinuxFsSpi there are areas marker with // TODO These need agreement on how to resolve the difference against PIGPIO-SPI

And any other comments you have

@eitch @FDelporte Ready for review

[FDelporte]

@mpilone FYI

[mpilone]

Thanks for moving it along. I was able to talk to an ST7735 display via this SPI implementation so it's working for what I need. I did run into one issue that isn't directly related to the library but you might want to document it so Pi4j doesn't get blamed for write failures. It looks like the default SPI buffer size on Linux is 4096 so sending more than that in a single write fails with a 90 error code. For example, trying to write the display memory for 160x128 pixels at 16 bit requires writing 40,960 bytes. See this discussion for more info.

[eitch]

@taartspi do you have a comment on @mpilone 's comment about the buffer size? Maybe we can detect a too large buffer being written, and then write it in sequences ourselves in the library?

[eitch]

Basically the API looks fine. I can't test it easily, as i still need to set up SPI device, so i'm grateful of @taartspi for testing and if you say it works, then we can use it.

I didn't see any todos, so not sure what is still open. Maybe we can add that then in different issues?

[eitch]

Since the class LinuxFsSpi has a dependency on jna, this comment is moot =)

[taartspi]

Todo. They are in plugin LinuxFsSpi. As i mentioned in the opening comment not sure where we should add the parms.
Buffer limit. It would be hard to do this for the user. It would require multiple ioctl calls which requires we know the CE line they used to keep it low across the ioctl calls. I haven't looked to see if we could change the spi driver thru ioctl data to skip the CE logic. Since trying to explain my thought are not clear i think better to write it as a limitation rather than under the cover magic. Throw an error. Recommend user control the CE them selves and make the needed write read or transfer calls to handle their buffer size

Also my questioning multiple calls being an error is not any problem

[mpilone]

I agree that I don't think the SPI implementation should split the user's request into multiple ioctl calls. Maybe just some docs in the manual around checking that buffer size and linking out to some docs that describe setting it (if there are any for the Pi).

Currently the code logs the error and just returns -1:

LOG.error("Could not write SPI message. ret {}, error: {}", ret, Native.getLastError());
numberOfBytes = -1;

It would be nice to get an exception with the underlying error code but I don't know if that's a normal pattern for Pi4J or the -1 indicates the issue and the user would have to enable logging to see the details. I suppose you could throw a specific exception based on the error code, but I don't know if you want to get into mapping ioctl error codes to exceptions. That seems like a pain to maintain.

[eitch]

If we know that there was an error, then we must throw an exception. We can just pass the ioctl error code, but in my opinion it would be really bad to just carry on and return -1, or are there other opinions?

[taartspi]

The exception seems better. The neg1 may imply they can just try again.

[taartspi]

tonights commit handles the TODO items It remains work to use an exception rather then RC for buffer size problems and to edit the JNA library requirement.
You can integrate this now if you want and I will do a new PR for those two items. Or I will try to get to these items this weekend. @eitch @FDelporte @mpilone

[taartspi]

The buffer size error does not see the exception, Assume lower level code ate it and used the RC of -1. I read the post when the problem was investigated and it looks like the limit is not changing soon. But, there are workarounds to increase the buff limit. So, if we saw a -1 AND checked their buffer was greater than 4096 we could throw an exception might but be wrong. I think best we add text to the error message and document the possibility of error on the Provider web page.

[eitch]

I think we can merge, any objections @FDelporte ?

[FDelporte]

@eitch please do merge! thanks

[taartspi]

Yes. Merge. I can use the snapshot to test on pi4 and do any followup changes on a new PR

[eitch]

So, merge done. Thanks @mpilone for the start of this, and thanks to @taartspi for completing it!

[FDelporte]

Correct this needs more testing and finetuning before we make a V2.7.1 release?