Feedback Thread: WPILib

[byteit101]

I have to agree strongly with FRC4ME, the OOP is poorly implemented. When I look at the autocomplete for a digital input, I don't want to see stuff not related to a digital input.
I also don't like the error classes, aside from only being used in the PCVideoServer, they flood the autocomplete with unnecessary items. If all classes inherit from ErrorBase, it should have two methods: HasError() and GetError(), and GetError would return a object that can be used to set, clear, and get the error
Two alternate ideas for the PID classes would be to have every class implement a sensor interface that has a Get(), and then you could pass it to the PID stuff, or have a sort of function pointer that points to the method and/or objects to call on.
A better OOP sensor class inheritance would be nice, as well as a Get() and Set(value) for all sensors.
Some classes are somewhat confusing in name to a novice: AnalogChannel, AnalogModule, AnalogTrigger, AnalogTriggerOutput, etc...

One thing that would be really nice would be a cleanup of the public members. For instance, the DigitalInput has tons of stuff you don't really need:
CancelInterrupts (Whoop-de-do!)
CheckAnalogChannel (This is not analog. I don't care)
CheckAnalogModule (This is not analog. I don't care)
CheckDigitalChannel (Potentially for debugging, what does it check? the module is present? nope, the channel # is valid. how confusing)
CheckDigitalModule (Potentially for debugging, what does it check? the module is present? nope, the channel # is valid. how confusing)
CheckPWMChannel (confusing and unrelated)
CheckPWMModdule (confusing and unrelated)
CheckRelayChannel (confusing and unrelated)
CheckRelayModule (confusing and unrelated)
CheckSolenoidChannel (confusing and unrelated)
CheckSolenoidModule (confusing and unrelated)
ClearError (Not useful, I don't see an error ever being set in the code)
DeleleSingletons (what singletons?)
DisableInterrupts (Wow! The first potentially useful and non confusing method! But you do need to know what Interrupts are)
EnableInterrupts (Wow! another!)
Get (And another! This one will be use by most people, and is not confusing in any way!)
GetAnalogTrggerForRouting( Routing? whats that? why do I care? and why is it an analogtrigger when this is a digitalinput)
GetChannel (Didn't I set the channel? did it change?)
GetChannelForRouting( Routing? whats that? why do I care?)
GetDefaultAnalogModule (I don't need this, and its unrelated to digital inputs)
GetDefaultDigitalModule (Can't I just use the one item constructor?)
GetDefaultSolenoidModule (I don't need this, and its unrelated to digital inputs)
GetError (Do we have an Error? If the code set errors, yes, but otherwise, this is unnecessary)
GetGlobalError (A global error on a non-global object? what is this madness?)
GetModuleForRouting (Why so much rooting? is roto-rooter here?)
ReadInterruptTimestamp (Why do I care? Isn't WPILib supposed to be a higer level framework? do I have to manually compare timestamps?)
RequestInterrupts (I'm taking a nap until a useful function come up, so Interrupt my nap when one does)
SetDefaultAnalogModule (Its a DigitalInput, not Analog!)
SetDefaultDigitalModule (Useful for pointers if you like making work, but you could just use the two arg constructor)
SetDefaultSolenoidModule (Solenoid !=(in your wildest dreams) DigitalInput)
SetError (I can cause errors, he he he!)
SetUpSourceEdge(source edge. ok. sure (smile and nod))
WaitForInterrupt(A useful function if you know what interrupts are! state-changers, Its a final useful function!)

A better DigitalInput: (dashes represent members of the object the function above returns)
Disable
Enable
Get
GetError
-IsFatal
-Set
-Get
-Clear
-...
GetInterruptManager
-Cancel
-Request
-Enable
-Disable
-Wait
-...
HasError
WaitForStateChange

Also, the C interfaces are kind of strange, I don't really think we need them (also given the fact that I have not heard of any teams using C this year)
Namespacing would be nice, although possibly confusing to new users

Better documentation and help would be very nice. And along this line, a better commented default SimpleTemplate would also be helpful

It would be great if you fixed this TODO as well

/**
* @todo If this is going to last until release, it needs a better name.
*/
class SimpleRobot