Skip to content

Implement defaults for missing I2C methods based on writeThenRead #744

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.

Sign up for GitHub

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

Merged
stefanhaustein merged 1 commit into from
Aug 17, 2026
Merged

Implement defaults for missing I2C methods based on writeThenRead #744

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 58 additions & 0 deletions pi4j-core/src/main/java/com/pi4j/io/i2c/I2C.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,4 +158,62 @@ default I2CRegister register(int address) {
* @return the value returned by the action
*/
<T> T execute(Callable<T> action);

@taartspi taartspi Aug 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I agree on the plan to reduce what is implemented in providers. We need to force any i2c provider to implement

   default void writeThenRead(byte[] writeBuffer, int writeOffset, int writeSize, int readDelayNanos, byte[] readBuffer, int readOffset, int readSize) {

The default implementation is not correct. This is meant to be a start- write-restart-read-stop, a single atomic operation. When I tested FFM it was doing a single operation, we need to force the next implementation too do the same.
Can this single method be marked abstract to force its implementation ?

@stefanhaustein stefanhaustein Aug 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, let's do this in a followup where we switch over ffm to only implement this method and make it abstract in an atomic small change

EDIT: Expanding defaults and fixing implementations can be seen as two two different steps, that's why I'd prefer to keep them separated. And cleaning up implementations is probably easier when we have all the defaults in place.


@Override
default int read() {
byte[] ioBuffer = new byte[1];
read(ioBuffer, 0, 1);
return ioBuffer[0] & 255;
}

@Override
default int write(byte b) {
write(new byte[] {b}, 0, 1);
return 1;
}

@Override
default int readRegister(int i) {
byte[] ioBuffer = new byte[] {(byte) i};
writeThenRead(ioBuffer, 0, 1, 0, ioBuffer, 0, 1);
return ioBuffer[0] & 255;
}

@Override
default int readRegister(byte[] register, byte[] data, int offset, int length) {
writeThenRead(register, 0, register.length, 0, data, offset, length);
return length;
}

@Override
default int readRegister(int i, byte[] bytes, int offset, int count) {
byte[] ioBuffer = new byte[] {(byte) i};
writeThenRead(ioBuffer, 0, 1, 0, bytes, offset, count);
return count;
}

@Override
default int writeRegister(int i, byte b) {
write(new byte[] {(byte) i, b}, 0, 2);
return 1;
}

@Override
default int writeRegister(int i, byte[] bytes, int offset, int length) {
byte[] combined = new byte[length + 1];
combined[0] = (byte) i;
System.arraycopy(bytes, offset, combined, 1, length);
write(combined, 0, combined.length);
return length;
}

@Override
default int writeRegister(byte[] register, byte[] data, int offset, int length) {
byte[] combined = new byte[ register.length + length];
System.arraycopy(register, 0, combined, 0, register.length);
System.arraycopy(data, offset, combined, register.length, length);
write(combined, 0, combined.length);
return length;
}
}
Loading