-
Notifications
You must be signed in to change notification settings - Fork 56
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.
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
I2CRegister.write ignores length value #27
Comments
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.
See if you can trace the variables here and make sure all the input parameters are still correct and Pi4J PiGpio Provider Pluginpi4j-v2/plugins/pi4j-plugin-pigpio/src/main/java/com/pi4j/plugin/pigpio/provider/i2c/PiGpioI2C.java Lines 117 to 121 in a54d626
Pi4J PiGpio JNI (Wrapper) Librarypi4j-v2/libraries/pi4j-library-pigpio/src/main/java/com/pi4j/library/pigpio/PiGpio_I2C.java Line 1428 in a54d626
Then .. depending on if using native PiGpio or remote via TCP ... TCP/SOCKET IMPL: Lines 1117 to 1126 in a54d626
NATIVE (JNI WRAPPER): Lines 1170 to 1180 in a54d626
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
|
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? |
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 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: |
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? |
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 Fingers crossed. Thanks, Robert |
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. |
I've read that compiler flags alone are not enough. See: 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 If you want to experiment with the compiler flags and try out Based on this line the Makefile:
I think you can export your own The best place to add that export is here in the pi4j-v2/libraries/pi4j-library-pigpio/src/main/native/build.sh Lines 102 to 109 in 75e1909
You may have to add a
|
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:
when it should be:
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! |
@hackerjimbo what JDK or you using on v6 architecture? I've not been successful on that side... |
@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). |
applied changes per @hackerjimbo ref: #27 (comment)
@hackerjimbo, I created a pull request (#29) for you to review and test out that includes the fixes you mentioned. |
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 |
FIX: Issue #27; I2CRegister.write ignores length value Received confirmation from @hackerjimbo the fix is working as expected.
Received confirmation from @hackerjimbo the fix is working as expected. |
As reported by @hackerjimbo
There is an issue with I2CRegister.write (data, offset, length). It seems to ignore the length and fire the whole amount of data in one go.
Starting points for research
pi4j-v2/pi4j-core/src/main/java/com/pi4j/io/i2c/I2CRegisterDataWriter.java
Line 111 in 75e1909
The text was updated successfully, but these errors were encountered: