search

DeepBlueRobotics / lib199

Code that we reuse in different projects/years.
Other
2 stars 0 forks source link

Smart numbers #65

Closed TuskAct2 closed 1 month ago

TuskAct2 commented 11 months ago

adds SmartNumber along with SmartBoolean and SmartString code and completed test code as a replacement for the smart dashboard methods

brettle commented 1 month ago

  1. Shouldn't the get() methods return the values currently in NetworkTables, not the value passed to the constructor? Note that the user could have changed the value in NetworkTables via SmartDashboard.

  2. userValue is not a particularly meaningful variable name. Seems like it should be something like initialValue.

  3. Does this PR offer significant advantages over adding lines like the following to the appropriate initSendable() method for each new SmartDashboard value?

builder.addDoubleProperty("Name", () -> field, newValue -> { field = newValue });

Do those advantages outweigh the effort associated with implementing, maintaining, and training programmers on the use of these classes?

brettle commented 1 month ago

After discussion with @CoolSpy3 @ProfessorAtomicManiac and @FriedLongJohns, we decided to abort this branch in favor of using initSendable() whenever possible.

The last commit on this branch can be retrieved via the aborted-SmartNumbers-attempt tag.