search

wpilibsuite / allwpilib

Official Repository of WPILibJ and WPILibC
https://wpilib.org/
Other
1.09k stars 611 forks source link

Allow SendableRegistry to accept multiple Sendables #5481

Open Gold856 opened 1 year ago

Gold856 commented 1 year ago

Is your feature request related to a problem? Please describe. Since we're considering allowing Sendables to be logged to DataLog, we'd need someplace to store the Sendables and their builders so they can be updated and logged to DataLog.

Describe the solution you'd like SendableRegistry should be expanded to allow one Sendable to have different builders. To ensure Sendables aren't published multiple times with the same builder class, we can check the builder backend and reject duplicates.

Describe alternatives you've considered We could create a clone of SendableRegistry just for DataLog, but SendableRegistry already has all the infrastructure.

Starlight220 commented 1 year ago

I'd argue that we should remove SendableRegistry completely, as part of the LiveWindow/Sendable rework. Less global registries is better.

What helpful purpose does SendableRegistry currently serve?

Gold856 commented 1 year ago

The only thing I can think of is that it stores the names of certain Sendables like CommandScheduler, which gets published under "Scheduler" by default if you call SmartDashboard.putData without a name. That only works for certain classes that register on construction, and can be replaced by forcing a name to be specified when calling putData. Its main purpose is to store Sendables for updating, both for dashboards/NT and LiveWindow. If we remove SendableRegistry, it seems like classes like SmartDashboard, LiveWindow, and possibly/eventually DataLog would each manage their own Sendables, update them, etc.

What exactly do you have in mind to replace SendableRegistry?

Starlight220 commented 1 year ago

Each sendable tree (SmartDashboard, Shuffleboard, DataLog instances, etc) holds its sendables. It's not very far from the current implementations, and it would allow easier destruction/cleanup. EDIT: it could complicate lifetimes a bit, as SendableRegistry is used to unpublish sendables when they're destroyed. C++ can be solved similar to how it is now, just with a variable destruction callback. Java could also use a CallbackStore mechanism, or possibly the new WPICleaner can be used.

The main difference would be the question of default name -- I think that forcing providing a name when publishing would be more robust, but in case not - it can be added as a field set from initSendable.