Skip to content

Commit abd31fb

Browse files
AustinShalitAustinShalit
committed
Solve comments
1 parent daa5110 commit abd31fb

File tree

5 files changed

+55
-44
lines changed

5 files changed

+55
-44
lines changed

‎core/src/main/java/edu/wpi/grip/core/operations/network/NetworkReceiver.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import java.util.function.Consumer;
44

55
import static com.google.common.base.Preconditions.checkArgument;
6+
import static com.google.common.base.Preconditions.checkNotNull;
67

78
/**
89
* Manages the interface between the {@link PublishAnnotatedOperation} and the actual network
@@ -18,7 +19,8 @@ public abstract class NetworkReceiver implements AutoCloseable {
1819
* @param path The path of the object to get
1920
*/
2021
public NetworkReceiver(String path) {
21-
checkArgument(!path.isEmpty(), "Name cannot be an empty string");
22+
checkNotNull(path, "Path cannot be null");
23+
checkArgument(!path.isEmpty(), "Path cannot be an empty string");
2224
this.path = path;
2325
}
2426

‎core/src/main/java/edu/wpi/grip/core/operations/network/networktables/NTManager.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -178,9 +178,9 @@ public void close() {
178178
NetworkTablesJNI.removeEntryListener(entryListenerFunctionUid);
179179

180180
synchronized (NetworkTable.class) {
181-
// This publisher is no longer used.
181+
// This receiver is no longer used.
182182
if (NTManager.count.addAndGet(-1) == 0) {
183-
// We are the last publisher so shut it down
183+
// We are the last resource using NetworkTables so shut it down
184184
NetworkTable.shutdown();
185185
}
186186
}
@@ -247,7 +247,7 @@ public void close() {
247247
synchronized (NetworkTable.class) {
248248
// This publisher is no longer used.
249249
if (NTManager.count.addAndGet(-1) == 0) {
250-
// We are the last publisher so shut it down
250+
// We are the last resource using NetworkTables so shut it down
251251
NetworkTable.shutdown();
252252
}
253253
}

‎core/src/main/java/edu/wpi/grip/core/sources/NetworkTableEntrySource.java

Lines changed: 31 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import edu.wpi.grip.core.sockets.SocketHints;
1111
import edu.wpi.grip.core.util.ExceptionWitness;
1212

13+
import com.google.common.base.CaseFormat;
1314
import com.google.common.collect.ImmutableList;
1415
import com.google.common.eventbus.EventBus;
1516
import com.google.common.eventbus.Subscribe;
@@ -24,12 +25,13 @@
2425
/**
2526
* Provides a way to get a {@link Types Type} from a NetworkTable that GRIP is connected to.
2627
*/
27-
@XStreamAlias("grip:NetworkValue")
28+
@XStreamAlias("grip:NetworkTableValue")
2829
public class NetworkTableEntrySource extends Source {
2930

3031
private static final String PATH_PROPERTY = "networktable_path";
3132
private static final String TYPE_PROPERTY = "BOOLEAN";
3233

34+
private final EventBus eventBus;
3335
private final OutputSocket output;
3436
private final String path;
3537
private final Types type;
@@ -42,13 +44,35 @@ public interface Factory {
4244
}
4345

4446
public enum Types {
45-
BOOLEAN, NUMBER, STRING;
47+
BOOLEAN {
48+
@Override
49+
protected SocketHint createSocketHint() {
50+
return SocketHints.Outputs.createBooleanSocketHint(Types.BOOLEAN.toString(), false);
51+
}
52+
},
53+
NUMBER {
54+
@Override
55+
protected SocketHint createSocketHint() {
56+
return SocketHints.Outputs.createNumberSocketHint(Types.NUMBER.toString(), 0.0);
57+
}
58+
},
59+
STRING {
60+
@Override
61+
protected SocketHint createSocketHint() {
62+
return SocketHints.Outputs.createStringSocketHint(Types.STRING.toString(), "");
63+
}
64+
};
65+
66+
Types() {
67+
68+
}
69+
70+
protected abstract SocketHint createSocketHint();
4671

4772
@Override
4873
public String toString() {
49-
return super.toString().charAt(0) + super.toString().substring(1).toLowerCase();
74+
return CaseFormat.UPPER_UNDERSCORE.to(CaseFormat.UPPER_CAMEL, name());
5075
}
51-
5276
}
5377

5478
@AssistedInject
@@ -75,12 +99,11 @@ public String toString() {
7599
@Assisted String path,
76100
@Assisted Types type) {
77101
super(exceptionWitnessFactory);
102+
this.eventBus = eventBus;
78103
this.path = path;
79104
this.type = type;
80105
networkReceiver = networkReceiverFactory.create(path);
81-
output = osf.create(createOutputSocket(type));
82-
83-
networkReceiver.addListener(o -> eventBus.post(new SourceHasPendingUpdateEvent(this)));
106+
output = osf.create(type.createSocketHint());
84107
}
85108

86109
@Override
@@ -117,7 +140,7 @@ public Properties getProperties() {
117140

118141
@Override
119142
public void initialize() {
120-
updateOutputSockets();
143+
networkReceiver.addListener(o -> eventBus.post(new SourceHasPendingUpdateEvent(this)));
121144
}
122145

123146
@Subscribe
@@ -126,22 +149,4 @@ public void onSourceRemovedEvent(SourceRemovedEvent event) {
126149
networkReceiver.close();
127150
}
128151
}
129-
130-
/**
131-
* Create a SocketHint from the given type.
132-
*
133-
* @param type The type of SocketHint to create
134-
*/
135-
private static SocketHint createOutputSocket(Types type) {
136-
switch (type) {
137-
case BOOLEAN:
138-
return SocketHints.Outputs.createBooleanSocketHint(Types.BOOLEAN.toString(), false);
139-
case NUMBER:
140-
return SocketHints.Outputs.createNumberSocketHint(Types.NUMBER.toString(), 0.0);
141-
case STRING:
142-
return SocketHints.Outputs.createStringSocketHint(Types.STRING.toString(), "");
143-
default:
144-
throw new IllegalArgumentException("Invalid NetworkTable source type");
145-
}
146-
}
147152
}

‎core/src/test/java/edu/wpi/grip/core/operations/network/MockMapNetworkPublisher.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,5 +46,4 @@ public void close() {
4646
public <T> MapNetworkPublisher<T> create(Set<String> keys) {
4747
return new MockMapNetworkPublisher<>(keys);
4848
}
49-
5049
}

‎core/src/test/java/edu/wpi/grip/core/sources/NetworkTableEntrySourceTest.java

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,12 @@
1010
import edu.wpi.first.wpilibj.networktables.NetworkTablesJNI;
1111

1212
import org.junit.After;
13-
import org.junit.Assert;
1413
import org.junit.Test;
1514

15+
import static org.junit.Assert.assertEquals;
16+
import static org.junit.Assert.assertFalse;
17+
import static org.junit.Assert.assertTrue;
18+
1619
public class NetworkTableEntrySourceTest {
1720

1821
private EventBus eventBus;
@@ -51,8 +54,9 @@ public void testBoolean() {
5154
BOOLEAN_PATH,
5255
NetworkTableEntrySource.Types.BOOLEAN);
5356

54-
Assert.assertTrue(source.updateOutputSockets());
55-
Assert.assertTrue((boolean) source.getOutputSockets().get(0).getValue().get());
57+
assertTrue("Socket could not be updated", source.updateOutputSockets());
58+
assertTrue("The socket's value was false, expected true.",
59+
(boolean) source.getOutputSockets().get(0).getValue().get());
5660
}
5761

5862
@Test
@@ -64,7 +68,7 @@ public void testBooleanWrongTypeNumber() {
6468
NUMBER_PATH,
6569
NetworkTableEntrySource.Types.BOOLEAN);
6670

67-
Assert.assertFalse(source.updateOutputSockets());
71+
assertFalse("The socket was able to update with an invalid type", source.updateOutputSockets());
6872
}
6973

7074
@Test
@@ -76,7 +80,7 @@ public void testBooleanWrongTypeString() {
7680
STRING_PATH,
7781
NetworkTableEntrySource.Types.BOOLEAN);
7882

79-
Assert.assertFalse(source.updateOutputSockets());
83+
assertFalse("The socket was able to update with an invalid type", source.updateOutputSockets());
8084
}
8185

8286
@Test
@@ -88,8 +92,8 @@ public void testNumber() {
8892
NUMBER_PATH,
8993
NetworkTableEntrySource.Types.NUMBER);
9094

91-
Assert.assertTrue(source.updateOutputSockets());
92-
Assert.assertEquals(
95+
assertTrue("Socket could not be updated", source.updateOutputSockets());
96+
assertEquals("Expected numbers to be equal -- they are not equal",
9397
TEST_NUMBER, (double) source.getOutputSockets().get(0).getValue().get(), 0.00001);
9498
}
9599

@@ -102,7 +106,7 @@ public void testNumberWrongTypeBoolean() {
102106
BOOLEAN_PATH,
103107
NetworkTableEntrySource.Types.NUMBER);
104108

105-
Assert.assertFalse(source.updateOutputSockets());
109+
assertFalse("The socket was able to update with an invalid type", source.updateOutputSockets());
106110
}
107111

108112
@Test
@@ -114,7 +118,7 @@ public void testNumberWrongTypeString() {
114118
STRING_PATH,
115119
NetworkTableEntrySource.Types.NUMBER);
116120

117-
Assert.assertFalse(source.updateOutputSockets());
121+
assertFalse("The socket was able to update with an invalid type", source.updateOutputSockets());
118122
}
119123

120124
@Test
@@ -126,8 +130,9 @@ public void testString() {
126130
STRING_PATH,
127131
NetworkTableEntrySource.Types.STRING);
128132

129-
Assert.assertTrue(source.updateOutputSockets());
130-
Assert.assertEquals(TEST_STRING, source.getOutputSockets().get(0).getValue().get());
133+
assertTrue("Socket could not be updated", source.updateOutputSockets());
134+
assertEquals("Expected Strings to be equal -- they are not equal",
135+
TEST_STRING, source.getOutputSockets().get(0).getValue().get());
131136
}
132137

133138
@Test
@@ -139,7 +144,7 @@ public void testStringWrongTypeBoolean() {
139144
BOOLEAN_PATH,
140145
NetworkTableEntrySource.Types.STRING);
141146

142-
Assert.assertFalse(source.updateOutputSockets());
147+
assertFalse("The socket was able to update with an invalid type", source.updateOutputSockets());
143148
}
144149

145150
@Test
@@ -151,7 +156,7 @@ public void testStringWrongTypeNumber() {
151156
NUMBER_PATH,
152157
NetworkTableEntrySource.Types.STRING);
153158

154-
Assert.assertFalse(source.updateOutputSockets());
159+
assertFalse("The socket was able to update with an invalid type", source.updateOutputSockets());
155160
}
156161

157162
}

0 commit comments

Comments
 (0)