I2CRegister.write ignores length value

[pi4j]

I2CRegister.write ignores length value

[hackerjimbo]

@FDelporte that's exactly the one I'm using. It seems to work very well. It appears to probe what the hardware can actually do at run time and then compile for that. In fact, part of its probing is what's causing issues with pigpio's stealing of signals (another bug I'm close to fixing).

[savageautomate]

@hackerjimbo,

If the error is not in the Pi4J core library, then its possible its either in the PiGpio Provider Plugin or the PiGpio JNI (wrapper) Library code.

• Are you using the native or remote (TCP socket) implementation of PiGpio?

---

See if you can trace the variables here and make sure all the input parameters are still correct and length has not been overwritten/modified.

Pi4J PiGpio Provider Plugin

pi4j-v2/plugins/pi4j-plugin-pigpio/src/main/java/com/pi4j/plugin/pigpio/provider/i2c/PiGpioI2C.java

Lines 117 to 121 in a54d626

	public int write(byte[] data, int offset, int length) throws IOException {
	Objects.checkFromIndexSize(offset, length, data.length);
	piGpio.i2cWriteDevice(this.handle, data, offset, length);
	return length;
	}

Pi4J PiGpio JNI (Wrapper) Library

pi4j-v2/libraries/pi4j-library-pigpio/src/main/java/com/pi4j/library/pigpio/PiGpio_I2C.java

Line 1428 in a54d626

int i2cWriteDevice(int handle, byte[] data, int offset, int length) throws IOException;

Then .. depending on if using native PiGpio or remote via TCP ...

TCP/SOCKET IMPL:

pi4j-v2/libraries/pi4j-library-pigpio/src/main/java/com/pi4j/library/pigpio/impl/PiGpioSocketImpl.java

Lines 1117 to 1126 in a54d626

	public int i2cWriteDevice(int handle, byte[] data, int offset, int length) throws IOException {
	logger.trace("[I2C::WRITE] -> [{}]; I2C Raw Write [{} bytes]", handle, data.length);
	validateReady();
	validateHandle(handle);
	PiGpioPacket tx = new PiGpioPacket(I2CWD, handle).data(data, offset, length);
	PiGpioPacket rx = sendPacket(tx);
	logger.trace("[I2C::WRITE] <- HANDLE={}; SUCCESS={}", handle, rx.success());
	validateResult(rx, false);
	return rx.result();
	}

NATIVE (JNI WRAPPER):

pi4j-v2/libraries/pi4j-library-pigpio/src/main/java/com/pi4j/library/pigpio/impl/PiGpioNativeImpl.java

Lines 1170 to 1180 in a54d626

	public int i2cWriteDevice(int handle, byte[] data, int offset, int length) throws IOException {
	logger.trace("[I2C::WRITE] -> [{}]; I2C Raw Write [{} bytes]", handle, data.length);
	validateReady();
	validateHandle(handle);
	Objects.checkFromIndexSize(offset, length, data.length);
	// write data array to I2C device
	int result = PIGPIO.i2cWriteDevice(handle, data, offset, length);
	logger.trace("[I2C::WRITE] <- HANDLE={}; SUCCESS={}", handle, (result>=0));
	validateResult(result, false);
	return result;
	}

NATIVE C IMPL

pi4j-v2/libraries/pi4j-library-pigpio/src/main/native/com_pi4j_library_pigpio_internal_PIGPIO.c

Lines 1616 to 1639 in a54d626

	JNIEXPORT jint JNICALL Java_com_pi4j_library_pigpio_internal_PIGPIO_i2cWriteDevice
	(JNIEnv *env, jclass class, jint handle, jbyteArray data, jint offset, jint count)
	{
	// obtain a pointer to the elements of the array and pin the memory
	jbyte *buffer = (*env)->GetByteArrayElements(env, data, 0);
	// get the maximum size of the Java data array; subtract any offset value
	jsize max_length = (*env)->GetArrayLength(env, data) - offset;
	// bounds check to make sure byte count does not exceed max array length (minus offset)
	int length = (count > max_length) ? max_length : count;
	// create a new buffer pointer using the offset
	jbyte *offsetBuffer = buffer + offset;
	// perform the actual I2C write operation into the native buffer array
	jint result = i2cWriteDevice((unsigned)handle, (char *)offsetBuffer, (unsigned)length);
	// unpin the reserved memory for 'data'; abort preserving any changes back to the Java array
	(*env)->ReleaseByteArrayElements(env, data, buffer, JNI_ABORT);
	// return the result
	return result;
	}

[hackerjimbo]

It's in the native library rather than running over TCP. Looks like the JNI wrapper is the place to enable tracing and see what happens. I'm not that familiar with how to set the debugging though. If I recall correctly you need some sort of library (slf4j-simple rings a bell) and to set a Java property. Could someone remind me of the details?

[savageautomate]

Well it looks like the instrumented trace info is not really printing out the actual length and offset so you may have to add you own messages to print out the length and offset values.

Yes, include slf4j-simple JAR in your dependencies and set this system property to print log/debug/trace message to the console.

        // configure default lolling level, accept a log level as the fist program argument
        System.setProperty("org.slf4j.simpleLogger.defaultLogLevel", "TRACE");

Example:

https://github.com/Pi4J/pi4j-example-telegraph/blob/c07f4a92f50f60ecdc2229621b8cc57445e67330/pom.xml#L32-L37

https://github.com/Pi4J/pi4j-example-telegraph/blob/master/src/main/java/com/pi4j/demo/telegraph/Telegraph.java#L60-L61

[hackerjimbo]

Well this is exciting! I had to download and then rebuild all of pi4j-v2 because of updates in the system (not that I blame anyone, it's a work in progress). However, once it was all rebuilt I couldn't get the result to run.

Interesting issue and probably worth raising elsewhere. I was building on a 64-bit Pi 4 running the 64-bit OS. It did produce the 32-bit binaries for the Pi zero I'm running on but it wasn't compiled for the v6 architecture so it didn't work! This is probably a bug that needs fixing. In the meantime are there any workarounds anyone can suggest?

[savageautomate]

Interesting issue and probably worth raising elsewhere. I was building on a 64-bit Pi 4 running the 64-bit OS. It did produce the 32-bit binaries for the Pi zero I'm running on but it wasn't compiled for the v6 architecture so it didn't work! This is probably a bug that needs fixing. In the meantime are there any workarounds anyone can suggest?

@hackerjimbo ,

I was afraid that might happen with the new cross-compiler stuff I was working on -- I'll have to revisit the new compiler stuff and get it building for ARMv6 as well.

In the meantime, try to clone from this older commit (using the previous native compiler logic) which should build for all 32-bit RaspberryPis. You will have to build on a 32-bit system or build using the -Pnative -Pdocker profiles and build using the cross-compiler Docker image. You should be able to use the Docker build if you install Docker on your 64-bit Pi : https://github.com/Pi4J/pi4j-v2/tree/ec2815015bf269b2365d90be000f67024fe9013a

Fingers crossed.

Thanks, Robert

[hackerjimbo]

Well… It loads the cross-compiler on the 64-bit machine and produces a 32-bit library. So that's not too bad. All I think it needs is he right compiler flags and we're sorted. If I had to guess, and with a bit of poking around, I'd support setting -march=armv6+fp -mfloat-abi=hard -mfpu=vfp will work just fine as it's GCC's default options on the Pi.

A little more poking around reveals that the default for the cross-compiler on the 64-bit Raspberry OS is -mfloat-abi=hard -mfpu=vfpv3-d16 -mthumb -march=armv7-a+fp which explains why it doesn't work (for so many reasons).

My knowledge of maven is amazingly poor. The same applies to the build scripts for pi4j-v2's native libraries. However, using -Pnative I can at least get it to build the 32-bit libraries (if wrongly). All I need is to know the tweaks to add those compiler flags in (-march=armv6+fp -mfloat-abi=hard -mfpu=vfp). The probably need to be in the shell scripts that build the libraries.

I think having the 64-bit and 32-bit (as just v6) is a good mix. I see little value in adding other 32-bit architectures as almost all of the rest of the 32-bit OS is compiled for v6.

[savageautomate]

@hackerjimbo

I've read that compiler flags alone are not enough.
Granted I have not tested it myself, but I suspect a custom compiler toolchain is needed.

See:

• #28
• https://github.com/Pro/raspi-toolchain

The older commit that I referenced will build using an older Docker cross-compiler image that I created that uses the RaspberryPi Tools GCC v4 cross compiler toolchain. From this older commit, you can build directly on a 32-bit RaspberryPi or build with the Docker image and it should produce ARMv6 compatible binaries.

As for the latest build logic in the master branch, this all uses newer builder docker images that I recently created and those are not using the custom compiler toolchains.

---

If you want to experiment with the compiler flags and try out march=armv6+fp -mfloat-abi=hard -mfpu=vfp I believe you can simply ...

Based on this line the Makefile:

pi4j-v2/libraries/pi4j-library-pigpio/src/main/native/Makefile

Line 29 in 75e1909

CFLAGS := $(DEBUG) -Wall $(INCLUDE) -Winline -pipe $(CARGS) -fPIC

I think you can export your own CARGS with whatever custom arguments you want to include.

The best place to add that export is here in the build.sh script where the 32-bit build is located:

pi4j-v2/libraries/pi4j-library-pigpio/src/main/native/build.sh

Lines 102 to 109 in 75e1909

	# ------------------------------------------------------
	# BUILD NATIVE LIBRARIES FOR ARMv6,ARMv7,ARMv8 32-BIT (ARMHF)
	# USING THE LOCALLY INSTALLED ARM CROSS-COMPILER
	# ------------------------------------------------------
	export CROSS_PREFIX=arm-linux-gnueabihf-
	export CC=arm-linux-gnueabihf-gcc
	export ARCH=armhf
	./build-libpi4j.sh $@

You may have to add a CARGS=${CARGS} argument here too where the make is getting called. I'm not 100% certain.

pi4j-v2/libraries/pi4j-library-pigpio/src/main/native/build-libpi4j.sh

Lines 64 to 70 in 75e1909

	# perform compile
	make clean all \
	--always-make \
	CROSS_PREFIX=${CROSS_PREFIX} \
	CC=${CC} \
	ARCH=${ARCH} \
	TARGET=lib/${ARCH}/libpi4j-pigpio.so $@

[hackerjimbo]

Well, I've totally failed to get the libraries built for the v6 architecture. On the plus side that's not this bug!

It turns out that the bug is not about getting the length wrong, ironically it's about checking the length. In the i2cWriteI2CBlockData method of PiGpioNativeImpl then length check it wrong! Line 1134 is:

validateI2cBlockLength(data.length);

when it should be:

validateI2cBlockLength(length);

so that means we have a fix for this bug. Now all we need to do is to get the system working again on the v6 architecture!

[FDelporte]

@hackerjimbo what JDK or you using on v6 architecture? I've not been successful on that side...
@savageautomate mentioned this one before but didn't try it out yet: https://www.azul.com/downloads/zulu-community/?version=java-11-lts&os=linux&architecture=arm-32-bit-hf&package=jdk

[savageautomate]

@hackerjimbo, I created a pull request (#29) for you to review and test out that includes the fixes you mentioned.

[savageautomate]

@hackerjimbo

I have sent you a JAR to test with that should be compatible with ARMv6 and include the fixes from the commits in branch: https://github.com/Pi4J/pi4j-v2/tree/issue/%2327

Let me know if the issue is fully resolved of if there are any lingering bugs.

Thanks, Robert

[savageautomate]

Received confirmation from @hackerjimbo the fix is working as expected.
Merged pull request into master.
Issue closed.