allwpilib
allwpilib copied to clipboard
`SmartDashboard.setDefault*` documentation
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:
- Would you suggest different wording for documenting the behavior of these methods?
- Should documentation in
NetworkTableEntry.java
,NetworkTablesJNI.java
,ntcore_c.h
, andntcore_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
.
/format