Update networktables to ntcore 2019.2.1 by SamCarlberg · Pull Request #916 · WPIRoboticsProjects/GRIP

SamCarlberg · 2019-01-24T21:05:00Z

Removes dependency on old networktables

Call flush after entire pipeline runs to keep all published values synchronized as much as possible - old behavior was to flush for every publisher (before the publisher would make its changes)

Removes the auto-shutdown behavior for NTHandler since clients now only maintain entries set on them (eg from publishers). I believe this behavior was present to prevent GRIP from keeping phantom entries around that it had no business using.

Since NetworkTables now requires libc 2.2.7, GRIP will not run on Ubuntu-based distros older than 18.04.

Removes dependency on old networktables Increase update rate to 100Hz (from 10Hz) to keep latencies low Call flush after entire pipeline runs to keep all published values synchronized as much as possible

mrtidy · 2019-01-26T12:10:06Z

azure-pipelines.yml

 jobs:
  - job: Linux
    pool:
      vmImage: 'Ubuntu 16.04'


I see that this was changed to 18.04 in commit 5e8ee9d but changed back to 16.04 in 63797a9 Since other changes also happened in 63797a9 it wasn't clear to me if this reset was intentional.

It was. Azure pipelines only has Ubuntu 16.04, not 18.04

JLLeitschuh · 2019-01-28T16:13:19Z

This totally cans Travis CI right?

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

JLLeitschuh · 2019-01-28T16:15:52Z

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

-  /*
-   * Nasty hack that is unavoidable because of how NetworkTables works.
-   */
-  private static final AtomicInteger count = new AtomicInteger(0);


I forget why this was required. Can you remind me why we had it?

Basically, just tracking usages of the NetworkTables connection. The client would only run if there was at least one source or one publishing operation that used it - if not, then the client would be shut down. I believe this was to prevent GRIP from keeping old entries around, but ntcore makes it so that clients only keep around entries that they have written to (the old library would make the client keep around all entries and re-send them to a server on a reconnect).

JLLeitschuh · 2019-01-28T19:05:43Z

What do you want to do with Travis CI at this point?

SamCarlberg · 2019-01-28T20:48:03Z

I think Travis can be removed now. It's just doing the javadoc push to the github site, but I think that's broken anyway. Azure has everything else.

There has been some chatter about compiling ntcore with compatibility for 14.04 or 16.04... I'm tempted to hold out to see if that goes anywhere - or maybe wait until Azure has an 18.04 image (issue here) instead of mucking around with Docker

JLLeitschuh

@SamCarlberg I'm going to leave this up to you to either wait on or merge.
Totally up to you.

codecov-io · 2019-04-18T04:36:57Z

Codecov Report

Merging #916 into master will increase coverage by 0.07%.
The diff coverage is 36.17%.

@@             Coverage Diff              @@
##             master     #916      +/-   ##
============================================
+ Coverage     52.75%   52.82%   +0.07%     
  Complexity        1        1              
============================================
  Files           327      327              
  Lines          8853     8852       -1     
  Branches        564      560       -4     
============================================
+ Hits           4670     4676       +6     
+ Misses         3980     3972       -8     
- Partials        203      204       +1

This reverts commit 90d0f0d.

Instead of catchall 0xFF

JLLeitschuh · 2019-04-18T20:39:39Z

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

+      ntInstance.deleteAllEntries();
+      ntInstance.startClient();
+      ntInstance.setServer(projectSettings.getPublishAddress());
+    }


Is there any chance that any of these things could hang?

JLLeitschuh · 2019-04-18T20:40:16Z

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

-    testTable.putBoolean(BOOLEAN_PATH, true);
-    testTable.putDouble(NUMBER_PATH, TEST_NUMBER);
-    testTable.putString(STRING_PATH, TEST_STRING);
+    ntInstance = NetworkTableInstance.create();


Hate this dam singleton pattern.

It's not a singleton? There's a default instance, but create() will return a new instance. Makes it useful for tests since there's no global state

JLLeitschuh

LTGM!

Update networktables to ntcore 2019.2.1

27a9c25

Removes dependency on old networktables Increase update rate to 100Hz (from 10Hz) to keep latencies low Call flush after entire pipeline runs to keep all published values synchronized as much as possible

SamCarlberg added type: enhancement location: core labels Jan 24, 2019

SamCarlberg added 3 commits January 24, 2019 16:05

Use RunStoppedEvent to guarantee timely flushes

0b89e3a

Update readme

e337dd7

Bump azure to ubuntu 18.04

5e8ee9d

SamCarlberg force-pushed the update-networktables branch from e7c1bd8 to 30564d8 Compare January 24, 2019 22:03

Use WPILib docker image on azure

63797a9

SamCarlberg force-pushed the update-networktables branch from 30564d8 to 63797a9 Compare January 25, 2019 18:32

mrtidy reviewed Jan 26, 2019

View reviewed changes

JLLeitschuh reviewed Jan 28, 2019

View reviewed changes

core/src/main/java/edu/wpi/grip/core/operations/network/networktables/NTManager.java Outdated Show resolved Hide resolved

JLLeitschuh reviewed Jan 28, 2019

View reviewed changes

JLLeitschuh approved these changes Jan 28, 2019

View reviewed changes

SamCarlberg added this to the v1.6.0 milestone Apr 2, 2019

Merge branch 'master' into update-networktables

32330e1

SamCarlberg force-pushed the update-networktables branch from 7895462 to 32330e1 Compare April 18, 2019 02:35

Install fakeroot

90d0f0d

SamCarlberg added 2 commits April 18, 2019 00:42

Revert "Install fakeroot"

541de19

This reverts commit 90d0f0d.

Listen to specific entry events on GRIP/run

2064989

Instead of catchall 0xFF

SamCarlberg requested a review from JLLeitschuh April 18, 2019 19:11

JLLeitschuh reviewed Apr 18, 2019

View reviewed changes

JLLeitschuh approved these changes Apr 18, 2019

View reviewed changes

SamCarlberg merged commit 73884fe into WPIRoboticsProjects:master Apr 18, 2019

SamCarlberg deleted the update-networktables branch April 18, 2019 21:31

ThadHouse mentioned this pull request Apr 19, 2019

Update GRIP dependency to Network Tables 4.0 #868

Closed

Comments

Conversation

SamCarlberg commented Jan 24, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JLLeitschuh commented Jan 28, 2019

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JLLeitschuh commented Jan 28, 2019

Uh oh!

SamCarlberg commented Jan 28, 2019

Uh oh!

JLLeitschuh left a comment

Choose a reason for hiding this comment

Uh oh!

codecov-io commented Apr 18, 2019

Codecov Report

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!

Choose a reason for hiding this comment

Uh oh!

JLLeitschuh left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants