allwpilib icon indicating copy to clipboard operation
allwpilib copied to clipboard

`SmartDashboard.setDefault*` documentation

Open chauser opened this issue 5 months ago • 1 comments

I would like to address #6334, but realized that some discussion about how the behavior should be documented would help before changing all of them. The code diff here contains a proposal that reflects my understanding of the actual behavior.

Questions:

  1. Would you suggest different wording for documenting the behavior of these methods?
  2. Should documentation in NetworkTableEntry.java, NetworkTablesJNI.java, ntcore_c.h, and ntcore_cpp.h also be addressed in the same PR, or in a second PR specific to the ntcore component?

More details about NetworkTableEntry.java, NetworkTablesJNI.java, ntcore_c.h, and ntcore_cpp.h:

SetDefault* methods in NetworkTableEntry.java called by SmartDashboard.setDefault* document the return value as @return False if the entry exists with a different type whereas the actual behavior is @return False if the entry exists. Interestingly, the descriptive documentation here is Sets the entry's value if it does not exist. which is correct.

The next level down is NetworkTablesJNI.java. Here the documentation is

/**
   * Sets default topic value.
   *
   * @param entry Entry handle.
   * @param time Time in microseconds.
   * @param defaultValue Default value.
   * @return True if set succeeded.
   */

which is correct but doesn't identify criteria for success.

On the C/C++ side:

ntcore_c.h has

/**
 * Set Default Entry Value.
 *
 * Returns copy of current entry value if it exists.
 * Otherwise, sets passed in value, and returns set value.
 * Note that one of the type options is "unassigned".
 *
 * @param entry     entry handle
 * @param default_value     value to be set if name does not exist
 * @return 0 on error (value not set), 1 on success
 */

having both the "Returns a copy of..." error and lack of specificity about success. ntcore_cpp.h has essentially the same issues as ntcore_c.h.

chauser avatar Mar 30 '24 05:03 chauser

/format

chauser avatar Apr 01 '24 18:04 chauser