Skip to content

Refactored GPIO to be testable and gradle upgrade #116

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

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Refactored GPIO to be testable and gradle upgrade #116

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
a3a4f91
upgrade to gradle-4.10 and junit-5
ecsedigergo Sep 3, 2018
b600c7e
xtend upgrade
ecsedigergo Sep 11, 2018
74d5923
Fixed trainmovementestimator test and moved to photon
ecsedigergo Sep 11, 2018
61d887c
GPIO tests implemented#1
ecsedigergo Sep 23, 2018
5f0a827
Refactored GPIO tests
ecsedigergo Sep 23, 2018
078b669
Implemented GPIOManager component tests
ecsedigergo Nov 27, 2018
e5a9b23
TEC call to GPIOManager fixed
ecsedigergo Nov 27, 2018
27f23f5
covered GPIO exception branches also
ecsedigergo Dec 16, 2018
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
1 change: 1 addition & 0 deletions src/java/.gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@
**/xtend-gen/
**/*.java._trace
**/*.xtendbin
!/src/main/vql-gen/*
23 changes: 16 additions & 7 deletions src/java/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//Gradle plugins
plugins {
id "org.xtext.xtend" version "1.0.12" apply false
id "org.xtext.xtend" version "2.0.1" apply false
id "com.google.protobuf" version "0.8.3" apply false

// id "org.sonarqube" version "2.5"
Expand All @@ -15,11 +15,11 @@ ssh.settings { knownHosts = allowAnyHosts }
//sonarqube {
// properties { property "sonar.projectName", "MoDeS3" }
//}

subprojects {
ext{
viatraVersion = '1.7.0'
xtendVersion = '2.9.0'
xtendVersion = '2.11.0'
componentsPrefix = gradle.ext.componentsPrefix
safetylogicPrefix = gradle.ext.safetylogicPrefix
messagingPrefix = gradle.ext.messagingPrefix
Expand All @@ -35,24 +35,33 @@ subprojects {
maven { url "https://plugins.gradle.org/m2/" }
maven { url "http://dl.bintray.com/andsel/maven/" }
}




plugins.withType(JavaPlugin) {

sourceCompatibility = 1.8
targetCompatibility = 1.8

test {
useJUnitPlatform()
}

dependencies {
compile 'org.slf4j:slf4j-api:1.7.21'
compile 'org.slf4j:slf4j-simple:1.7.21'
//compile 'org.eclipse.xtend:org.eclipse.xtend.lib:2.9.0'

compile group: 'org.eclipse.xtend', name: 'org.eclipse.xtend.lib', version: xtendVersion

testCompile 'junit:junit:4.12'
testCompile "org.mockito:mockito-core:1.10.19"
testCompile 'org.mockito:mockito-core:2.22.0'
testCompile("org.junit.jupiter:junit-jupiter-api:5.2.0")
testRuntime("org.junit.jupiter:junit-jupiter-engine:5.2.0")

}

}



plugins.withType(org.xtext.gradle.XtendLanguagePlugin) {
sourceSets {
Expand Down
16 changes: 13 additions & 3 deletions src/java/components/barrier/.classpath
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,18 @@
<?xml version="1.0" encoding="UTF-8"?>
<classpath>
<classpathentry kind="src" path="src/main/java"/>
<classpathentry kind="src" path="src/main/xtend-gen"/>
<classpathentry kind="src" output="bin/main" path="src/main/java">
<attributes>
<attribute name="gradle_scope" value="main"/>
<attribute name="gradle_used_by_scope" value="main,test"/>
</attributes>
</classpathentry>
<classpathentry kind="src" output="bin/main" path="src/main/xtend-gen">
<attributes>
<attribute name="gradle_scope" value="main"/>
<attribute name="gradle_used_by_scope" value="main,test"/>
</attributes>
</classpathentry>
<classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER/org.eclipse.jdt.internal.debug.ui.launcher.StandardVMType/JavaSE-1.8/"/>
<classpathentry kind="con" path="org.eclipse.buildship.core.gradleclasspathcontainer"/>
<classpathentry kind="output" path="bin"/>
<classpathentry kind="output" path="bin/default"/>
</classpath>
23 changes: 19 additions & 4 deletions src/java/components/dashboard/.classpath
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,24 @@
<?xml version="1.0" encoding="UTF-8"?>
<classpath>
<classpathentry kind="src" path="src/main/java"/>
<classpathentry kind="src" path="src/main/webapp"/>
<classpathentry kind="src" path="src/main/resources"/>
<classpathentry kind="src" output="bin/main" path="src/main/java">
<attributes>
<attribute name="gradle_scope" value="main"/>
<attribute name="gradle_used_by_scope" value="main,test"/>
</attributes>
</classpathentry>
<classpathentry kind="src" output="bin/main" path="src/main/webapp">
<attributes>
<attribute name="gradle_scope" value="main"/>
<attribute name="gradle_used_by_scope" value="main,test"/>
</attributes>
</classpathentry>
<classpathentry kind="src" output="bin/main" path="src/main/resources">
<attributes>
<attribute name="gradle_scope" value="main"/>
<attribute name="gradle_used_by_scope" value="main,test"/>
</attributes>
</classpathentry>
<classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER/org.eclipse.jdt.internal.debug.ui.launcher.StandardVMType/JavaSE-1.8/"/>
<classpathentry kind="con" path="org.eclipse.buildship.core.gradleclasspathcontainer"/>
<classpathentry kind="output" path="bin"/>
<classpathentry kind="output" path="bin/default"/>
</classpath>
22 changes: 19 additions & 3 deletions src/java/components/gpiomanager/.classpath
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,24 @@
<?xml version="1.0" encoding="UTF-8"?>
<classpath>
<classpathentry kind="src" path="src/main/java"/>
<classpathentry kind="src" path="src/main/resources"/>
<classpathentry kind="src" output="bin/main" path="src/main/java">
<attributes>
<attribute name="gradle_scope" value="main"/>
<attribute name="gradle_used_by_scope" value="main,test"/>
</attributes>
</classpathentry>
<classpathentry kind="src" output="bin/test" path="src/test/java">
<attributes>
<attribute name="gradle_scope" value="test"/>
<attribute name="gradle_used_by_scope" value="test"/>
</attributes>
</classpathentry>
<classpathentry kind="src" output="bin/main" path="src/main/resources">
<attributes>
<attribute name="gradle_scope" value="main"/>
<attribute name="gradle_used_by_scope" value="main,test"/>
</attributes>
</classpathentry>
<classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER/org.eclipse.jdt.internal.debug.ui.launcher.StandardVMType/JavaSE-1.8/"/>
<classpathentry kind="con" path="org.eclipse.buildship.core.gradleclasspathcontainer"/>
<classpathentry kind="output" path="bin"/>
<classpathentry kind="output" path="bin/default"/>
</classpath>
9 changes: 9 additions & 0 deletions ...gpiomanager/src/main/java/hu/bme/mit/inf/modes3/components/gpiomanager/CommandReader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package hu.bme.mit.inf.modes3.components.gpiomanager;

import java.io.IOException;

import hu.bme.mit.inf.modes3.components.gpiomanager.Gpio.Level;

public interface CommandReader {
Level getGpioValue(String targetPath) throws IOException;
}
7 changes: 7 additions & 0 deletions ...gpiomanager/src/main/java/hu/bme/mit/inf/modes3/components/gpiomanager/CommandWriter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package hu.bme.mit.inf.modes3.components.gpiomanager;

import java.io.IOException;

public interface CommandWriter {
void executeCommand(String value, String targetFile) throws IOException;
}
68 changes: 36 additions & 32 deletions ...mponents/gpiomanager/src/main/java/hu/bme/mit/inf/modes3/components/gpiomanager/Gpio.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,8 @@
package hu.bme.mit.inf.modes3.components.gpiomanager;

import java.io.BufferedReader;
import java.io.BufferedWriter;
import java.io.FileNotFoundException;
import java.io.FileReader;
import java.io.FileWriter;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
Expand Down Expand Up @@ -57,13 +55,17 @@ public interface InputStateListener {

private TimerTask _inputListenerTask;

private static CommandWriter WRITER;
Copy link
Member

@benedekh benedekh Oct 8, 2018

Choose a reason for hiding this comment

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

Why is this field static instead of instance level?

Copy link
Member

@benedekh benedekh Oct 8, 2018

Choose a reason for hiding this comment

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

If the field will be instance-level, then please follow the lowercase naming convention also.

Copy link
Collaborator Author

@ecsedigergo ecsedigergo Oct 9, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Because the GIPO Manager is a Singleton class and I want to set this writer from outside of the manager. So anyway it will have 1 instance.

Copy link
Member

@benedekh benedekh Oct 11, 2018

Choose a reason for hiding this comment

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

But here the Gpio is neither a singleton nor a static-only class. So its methods and fields should be instance level.

That's another question, why is GpioManager a Singleton class with static-only methods? It should be either singleton or static-only. Mixing the two does not make too much sense.


private static CommandReader READER;

public Gpio(int pin, Direction direction) throws IOException, GpioNotConfiguratedException {
Copy link
Member

@benedekh benedekh Oct 8, 2018

Choose a reason for hiding this comment

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

The initialization of READER and WRITER are missing here. You may end up having a NullPointerException.

Copy link
Collaborator Author

@ecsedigergo ecsedigergo Oct 9, 2018

Choose a reason for hiding this comment

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

If I got in this case (with static variables) a nullpointer, that is definitely a misconfiguration (you must set those before usage), so the exception must occur. Should I rather instantiate the variables with GPIOWriter/Reader when the GPIO manager is instantiated?

Copy link
Member

@benedekh benedekh Oct 11, 2018

Choose a reason for hiding this comment

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

You should always make sure, that whenever the constructor is invoked, all the fields and variables that this constructor uses (invokes) must have been instantiated.

Having a separate setter of a field and making sure that that setter must have been invoked before anyone creates an instance of Gpio is very dangerous. (You rely on someone else to invoke those setters before any instance of this class is created.)

this._pin = pin;
this._direction = direction;

// export gpio pin first
try {
executeCommand(String.valueOf(_pin), GPIO_BASE_FOLDER + "export");
WRITER.executeCommand(String.valueOf(_pin), GPIO_BASE_FOLDER + "export");
} catch (Exception ex) {
Logger.error(TAG, "Pin export failed! Pin: %d", _pin);

Expand All @@ -72,8 +74,8 @@ public Gpio(int pin, Direction direction) throws IOException, GpioNotConfigurate
// in this case, we should unexport it and try to export it again
// if unexport fails also, there should be something wrong
try {
executeCommand(String.valueOf(_pin), GPIO_BASE_FOLDER + "unexport");
executeCommand(String.valueOf(_pin), GPIO_BASE_FOLDER + "export");
WRITER.executeCommand(String.valueOf(_pin), GPIO_BASE_FOLDER + "unexport");
WRITER.executeCommand(String.valueOf(_pin), GPIO_BASE_FOLDER + "export");
} catch (Exception ex2) {
Logger.error(TAG, "Pin re-export failed! Pin: %d", _pin);
throw ex2;
Expand All @@ -84,7 +86,7 @@ public Gpio(int pin, Direction direction) throws IOException, GpioNotConfigurate

// set direction of pin
try {
executeCommand(this._direction.toString().toLowerCase(), _gpioFolder + "direction");
WRITER.executeCommand(this._direction.toString().toLowerCase(), _gpioFolder + "direction");
} catch (Exception ex) {
Logger.error(TAG, "Pin direction setup failed! Pin: %d", _pin);
throw ex;
Expand All @@ -93,7 +95,7 @@ public Gpio(int pin, Direction direction) throws IOException, GpioNotConfigurate
switch (this._direction) {
case IN:
// setup edge detection as well
executeCommand("both", _gpioFolder + "edge");
WRITER.executeCommand("both", _gpioFolder + "edge");

setupInputChangeListening();
break;
Expand All @@ -107,7 +109,7 @@ public Gpio(int pin, Direction direction) throws IOException, GpioNotConfigurate

public final void setLevel(Level level) throws IOException {
try {
executeCommand(level == Level.HIGH ? "1" : "0", _gpioFolder + "value");
WRITER.executeCommand(level == Level.HIGH ? "1" : "0", _gpioFolder + "value");
this._level = level;
} catch (IOException ex) {
Logger.error(TAG, "Level setting failed! Pin: %d", _pin);
Expand Down Expand Up @@ -155,37 +157,36 @@ public final void removeInputStateListener(InputStateListener listener) {
this.listeners.remove(listener);
}

private void executeCommand(String value, String targetFile) throws IOException {
Logger.info(TAG, "Trying to write \"%s\" to %s", value, targetFile);
try (BufferedWriter w = new BufferedWriter(new FileWriter(targetFile))) {
w.append(value);
w.newLine();
w.flush();
}
Logger.info(TAG, "Succeeded!");
public static void setWriter(CommandWriter writer) {
Copy link
Member

@benedekh benedekh Oct 8, 2018

Choose a reason for hiding this comment

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

Please make this setter instance-level, instead of class-level.

Copy link
Collaborator Author

@ecsedigergo ecsedigergo Oct 9, 2018

Choose a reason for hiding this comment

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

Makes instance level setter any sense with Singleton GPIO manager?

Copy link
Member

@benedekh benedekh Oct 11, 2018

Choose a reason for hiding this comment

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

Yes, because Gpio itself is neither Singleton, nor static-only. Here we are talking about Gpio.java and not GpioManager.java.

WRITER = writer;
}

public static void setReader(CommandReader reader) {
READER = reader;
}

private class InputStateChangeListenerTask extends TimerTask {

@Override
public void run() {
try {
try (BufferedReader reader = new BufferedReader(new FileReader(_gpioFolder + "value"))) {
Level newLevel = reader.readLine().equals("1") ? Level.HIGH : Level.LOW;
if (!newLevel.equals(_level)) {
Logger.info(TAG, "Pin state changed! Pin: %d Value: %s", _pin, newLevel.toString());
_level = newLevel;
listeners.stream().forEach((listener) -> {
try {
listener.levelStateChanged(_level);
} catch (Exception ex) {
Logger.error(TAG, "Error while notifying the InputStateChangeListener (%s)",
listener.toString());
Logger.error(TAG, ex.getMessage());
}
});
}
Level newLvl = READER.getGpioValue(_gpioFolder + "value");
// try (BufferedReader reader = new BufferedReader(new FileReader(_gpioFolder + "value"))) {
// Level newLevel = reader.readLine().equals("1") ? Level.HIGH : Level.LOW;
if (newLvl != null && !newLvl.equals(_level)) {
Logger.info(TAG, "Pin state changed! Pin: %d Value: %s", _pin, newLvl.toString());
_level = newLvl;
listeners.stream().forEach((listener) -> {
try {
listener.levelStateChanged(_level);
} catch (Exception ex) {
Logger.error(TAG, "Error while notifying the InputStateChangeListener (%s)",
listener.toString());
Logger.error(TAG, ex.getMessage());
}
});
}
// }

} catch (FileNotFoundException ex) {
Logger.error(TAG, "InputStateChangeListenerTask is aborted, because pin #%d's value file not found!",
Expand All @@ -195,6 +196,9 @@ public void run() {
Logger.error(TAG, "InputStateChangeListenerTask is aborted, IO error during file read on pin #%d",
_pin);
Logger.error(TAG, ex.getMessage());
} catch (Exception ex) {
ex.printStackTrace();
Logger.error(TAG, "Something is wrong here: " + ex.getMessage());
}
}

Expand All @@ -220,7 +224,7 @@ public final void cleanup() throws IOException, InterruptedException {
_isInputListenerRunning = false;
Logger.info(TAG, "Listening service for pin #%d stopped.", _pin);
}
executeCommand(String.valueOf(_pin), GPIO_BASE_FOLDER + "unexport");
WRITER.executeCommand(String.valueOf(_pin), GPIO_BASE_FOLDER + "unexport");
} catch (IOException ex) {
Logger.error(TAG, "Pin unexport failed! Pin: %d", _pin);
throw ex;
Expand Down
Loading