-
Notifications
You must be signed in to change notification settings - Fork 63
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
Feature/gpiod #322
Feature/gpiod #322
Conversation
Here the PR for the builder: Pi4J/pi4j-docker#2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At this point, my brain only allows me to do a quick "esthetical" review. Only minor comments...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't it be easier to keep everything in one gitignore file at the root level of the project?
* @version $Id: $Id | ||
*/ | ||
public class GpioChip implements Closeable { | ||
private final long cPtr; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I personally prefer full variable names... Same remark also for other classes with the same variable.
@Override | ||
protected void finalize() { | ||
if(next == null) { | ||
if(noCoseCurrent) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
noCose or noClose?
@@ -52,5 +52,5 @@ chmod +x build.sh | |||
# FOR ARMv6,ARMv7, ARMv8 32-BIT (ARMHF) | |||
# FOR ARMv8 64-BIT (ARM64) | |||
# ------------------------------------------------------------- | |||
docker pull pi4j/pi4j-builder-native:2.0 | |||
#docker pull pi4j/pi4j-builder-native:2.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better to remove unused code instead of commenting it
@@ -55,5 +55,5 @@ chmod +x build.sh | |||
# FOR ARMv6,ARMv7, ARMv8 32-BIT (ARMHF) | |||
# FOR ARMv8 64-BIT (ARM64) | |||
# ------------------------------------------------------------- | |||
docker pull pi4j/pi4j-builder-native:2.0 | |||
#docker pull pi4j/pi4j-builder-native:2.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
idem
.initial(DigitalState.HIGH) | ||
.build(); | ||
DigitalOutput pin = pi4j.create(config); | ||
/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove
lastEvent.set(System.currentTimeMillis()); | ||
}); | ||
Thread.sleep(1000 * 60); | ||
/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove
@FDelporte I applied the review comments. |
This pull requests enables Pi4j to take advantage of the GpioD library, that gets developed together with the linux kernel.
Contents:
I'm not sure if this will immediately compile in your CI/CD pipeline. It might miss some dependencies that are needed to build the JNI library for gpiod. These are required packets: https://github.com/Pi4J/pi4j-v2/blob/501a50f1594a9bae2ea2cd794d6998db8d801faf/libraries/pi4j-library-gpiod/src/main/native/build-prerequisites.sh
Closes #321