robotdotnet / NetworkTables

FRC NetworkTables for .NET. This is all old code and should not be used anymore.
3 stars 5 forks source link

Small modification so SmartDashboard can monitor network connection status #38

Closed sipsorcery closed 8 years ago

sipsorcery commented 8 years ago

At the moment the SmartDashboard does a one off check of the NetworkTable connection status. As such the icon on the SmartDashboard cannot be relied upon should the status change. This pull request is to add an event to the NetworkTable class so the SmartDashboard can receive notifications when the connection status changes.

ThadHouse commented 8 years ago

I like the idea of this change, however in order to accept this a complete implementation would be necessary, including a way to remove any listener added, and remove it by the delegate type instead of only by the callback id. I'd also like to see methods for IRemoteConnectionListener added so I can merge the code upstream to WPILib as well.

sipsorcery commented 8 years ago

It would be trivial to add the wrapper method for NtCore.RemoveConnectionListener that would satisfy the listener removal request.

I'm not really sure what you mean by "complete implementation"? My own little was just to get the SmartDashboard connection icon working so I didn't have to puzzle over why it was red when the connection was fine.

ThadHouse commented 8 years ago

The completeimplementation would mean an identical API to the instance version, using the same callbacks as well. We do not want the raw callback ID passed outside of the library, unless the user is directly using the NtCore static class. For the static version of the connection listeners, IRemove can be null, but the rest of the information needs to be correct.

ThadHouse commented 8 years ago

I added these fixes separately. Closed by #41