Add NetworkTableSource by AustinShalit · Pull Request #646 · WPIRoboticsProjects/GRIP

AustinShalit · 2016-07-30T08:42:10Z

closes #363
closes #572
closes #635

SamCarlberg · 2016-07-30T15:50:52Z

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

+
+    private void addListener() {
+      entryListenerFunctionUid = NetworkTablesJNI.addEntryListener(path, (uid, key, value, flags)
+              -> objectProperty.set(value),


Arguments should each be on their own line

codecov-io · 2016-07-30T18:46:29Z

Current coverage is 58.43% (diff: 70.63%)

Merging #646 into master will increase coverage by 0.39%

@@             master       #646   diff @@
==========================================
  Files           196        198     +2   
  Lines          6165       6282   +117   
  Methods           0          0          
  Messages          0          0          
  Branches        561        569     +8   
==========================================
+ Hits           3578       3671    +93   
- Misses         2422       2442    +20   
- Partials        165        169     +4

Powered by Codecov. Last update bbea07c...fc680f8

SamCarlberg · 2016-08-02T00:11:41Z

Write tests for the network tables source

AustinShalit · 2016-08-03T01:18:29Z

~~Does anyone know why edu.wpi.grip.core.sources.SourcesSanityTest > testNulls is failing?~~

SamCarlberg · 2016-08-03T01:54:04Z

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

+   * @param path The path of the object to get
+   */
+  public NetworkReceiver(String path) {
+    checkArgument(!path.isEmpty(), "Name cannot be an empty string");


Should also add an explicit nullcheck with checkNotNull(path, ...)

Haha.... Thank you!

AustinShalit · 2016-08-08T16:34:55Z

Tests randomly fail...

JLLeitschuh · 2016-08-08T17:24:51Z

@SamCarlberg Can you help out Austin?

SamCarlberg · 2016-08-08T18:19:45Z

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

+      networkReceiver.close();
+    }
+  }
+}


Newline at EOF

AustinShalit · 2016-08-09T05:18:20Z

So maybe the error was a fluke? I reran the travis tests 3 times and the build did not fail.

JLLeitschuh · 2016-08-09T17:22:30Z

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

+/**
+ * Provides a way to get a {@link Types Type} from a NetworkTable that GRIP is connected to.
+ */
+@XStreamAlias("grip:NetworkTableValue")


Do we want this to be "grip:NetworkTableEntry"?

AustinShalit · 2016-08-09T18:27:44Z

I labeled this as ready for review because I have not seen the tests failing after merging master into this branch.

JLLeitschuh · 2016-08-10T01:44:43Z

So what I really don't want is Network Tables initialized in tests that don't need it. That's why, up until this point I've kept network tables out of tests.
So, since we have DI, I'd love if network tables was not injected where it's not needed. Do you think you can do that?

AustinShalit · 2016-08-10T08:32:45Z

ummmm..... no promises but I will give it a shot

# Conflicts: # ui/src/test/java/edu/wpi/grip/ui/pipeline/PipelineUITest.java

AustinShalit · 2016-08-12T16:29:02Z

@JLLeitschuh I have no idea how to do this. If you could please lend a hand 😃

JLLeitschuh · 2016-08-27T17:22:51Z

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

+ */
+public abstract class NetworkReceiver implements AutoCloseable {
+
+  protected final String path;


Why is this a protected field?

As discussed in person

JLLeitschuh · 2016-08-27T17:44:00Z

LGTM!

![Approved with PullApprove](https://img.shields.io/badge/GRIP Merge Approvers-approved-brightgreen.svg)

AustinShalit added the in progress label Jul 30, 2016

AustinShalit added this to the v2.0.0 milestone Jul 30, 2016

SamCarlberg reviewed Jul 30, 2016
View reviewed changes

AustinShalit force-pushed the ntSource branch from d409cc5 to 4b7291d Compare July 30, 2016 18:38

Add NetworkTableSource

640cf32

AustinShalit force-pushed the ntSource branch from 4b7291d to 640cf32 Compare July 30, 2016 21:13

AustinShalit added ready for review and removed in progress labels Jul 30, 2016

AustinShalit added in progress needs: tests help wanted and removed ready for review needs: tests labels Aug 2, 2016

Add Tests

daa5110

AustinShalit force-pushed the ntSource branch from b7d85c2 to daa5110 Compare August 3, 2016 00:58

SamCarlberg reviewed Aug 3, 2016
View reviewed changes

AustinShalit force-pushed the ntSource branch from a1d41bb to c05e53d Compare August 3, 2016 02:10

AustinShalit force-pushed the ntSource branch from 77c74e7 to c9efca1 Compare August 8, 2016 16:17

AustinShalit added help wanted in progress and removed ready for review labels Aug 8, 2016

SamCarlberg reviewed Aug 8, 2016
View reviewed changes

Merge branch 'master' into ntSource

fd1075b

AustinShalit force-pushed the ntSource branch from f72c5ef to fd1075b Compare August 8, 2016 22:35

JLLeitschuh reviewed Aug 9, 2016
View reviewed changes

Update XStreamAlias

e8cd49e

AustinShalit added ready for review and removed help wanted in progress labels Aug 9, 2016

AustinShalit added the type: feature label Aug 9, 2016

Merge branch 'master' into ntSource

d2e9bd8

# Conflicts: # ui/src/test/java/edu/wpi/grip/ui/pipeline/PipelineUITest.java

AustinShalit force-pushed the ntSource branch from 817bc52 to d2e9bd8 Compare August 12, 2016 03:07

AustinShalit mentioned this pull request Aug 15, 2016

Add NetworkTable Source. #572

Closed

Only use networktables when we need it

40450c5

AustinShalit added the Looks Good To Me! (DEV Approval) label Aug 27, 2016

JLLeitschuh reviewed Aug 27, 2016
View reviewed changes

Cleanup

fc680f8

AustinShalit merged commit a9659d0 into WPIRoboticsProjects:master Aug 27, 2016

AustinShalit deleted the ntSource branch August 27, 2016 17:44

Comments

Conversation

AustinShalit commented Jul 30, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov-io commented Jul 30, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Current coverage is 58.43% (diff: 70.63%)

Uh oh!

SamCarlberg commented Aug 2, 2016

Uh oh!

AustinShalit commented Aug 3, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AustinShalit commented Aug 8, 2016

Uh oh!

JLLeitschuh commented Aug 8, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AustinShalit commented Aug 9, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AustinShalit commented Aug 9, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JLLeitschuh commented Aug 10, 2016

Uh oh!

AustinShalit commented Aug 10, 2016

Uh oh!

AustinShalit commented Aug 12, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JLLeitschuh commented Aug 27, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

AustinShalit commented Jul 30, 2016 •

edited

Loading

codecov-io commented Jul 30, 2016 •

edited

Loading

AustinShalit commented Aug 3, 2016 •

edited

Loading

AustinShalit commented Aug 9, 2016 •

edited

Loading

JLLeitschuh commented Aug 27, 2016 •

edited

Loading