search

cjlawson02 / ntcore-ts-client

A TypeScript library for WPILib's NT4.1 spec
https://ntcore.chrislawson.dev
22 stars 7 forks source link

Draft: NT4.1 support #112

Closed CrispyBacon1999 closed 7 months ago

CrispyBacon1999 commented 9 months ago

Description

This pull request introduces support for NetworkTables v4.1 while maintaining backward compatibility with version 4.0. The primary goal is to enable seamless integration with NetworkTables servers that have upgraded to v4.1, while ensuring that existing functionality for v4.0 remains intact.

Currently, there are no tests written for 4.1, which should absolutely be added. All the tests for v4.0 still pass, with minor modifications to ensure that the code is running at 4.0 instead.

Changes Made

New Feature: Added support for NetworkTables version 4.1.

Compatibility: The code has been structured to preserve compatibility with NetworkTables version 4.0. The default behavior remains as it was for version 4.0, ensuring that users running older servers won't experience any disruptions.

Conditional Logic: Implemented conditional logic to detect the server's version during runtime. If the server supports version 4.1, the code will automatically switch to using the new features and improvements introduced in this version.

Questions for Reviewers

Are there any concerns about maintaining backward compatibility with version 4.0? Have we covered all possible scenarios where compatibility might be affected?

Have we implemented the conditional logic for version detection in an efficient and reliable manner? Are there any potential edge cases that we might have missed?

Are there any performance considerations or optimizations that need to be addressed to ensure the smooth operation of the code with version 4.1?

Any feedback and suggestions for improvement are highly welcomed.

Note: The changes for the documentation make PRs get a bit messy. Would it be worthwhile changing it so the docs build only on merges to main?

cjlawson02 commented 9 months ago

Hi, please merge in beta and run CI again. I took a quick glance... did we implement the PING and PONG requirements? Maybe RTT sufices

CrispyBacon1999 commented 9 months ago

Not 100% sure what's needed for the PING and PONG. Most of the logic for this was pulled from AdvantageScope since they had already done the upgrade and have their own library for it.

cjlawson02 commented 8 months ago

@CrispyBacon1999 If you have hardware to test this on, I rebased in beta

CrispyBacon1999 commented 8 months ago

I don't have it tonight, but I should be able to test tomorrow if I remember to.

CrispyBacon1999 commented 7 months ago

I was able to test the connection to a physical robot tonight and it worked perfectly! Connected properly over NT4.1 and I was able to poll values from the robot.