-
Notifications
You must be signed in to change notification settings - Fork 6
add telemetry design doc #5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
A half baked NT4 js interface: https://github.com/RobotCasserole1736/RobotCasserole2022/blob/main/RobotCode/src/main/deploy/www/interfaces/nt4.js Some ideas on dashboard widgets: Gauges/dials, field2d equivalent, swerve and tank drive specific mechanism widgets, compass style gyro indicator, simple text display, icons with on/off/blink ability.... Try to incorporate some "at a glance" notion of "ok/not ok". Being able to color code things red or green helps with this. Use browser animation capabilities to keep the appearance silky smooth. |
|
Proposal. Focus on driver-dashboard first. With the work put into glass, this is the bigger gap today. Leave the path open to a unified tool in the future if desired. One more widget idea, a simple time series data plot that auto scales and auto advances. We also used a sound widget in the past years for memes. Zero usefulness. But fun is a core value. |
|
Reading through the lastest version of the proposal.... I can think through a handful of ways to go about the sendable deprivation. Fwiw... I'm biased here, we've avoided both sendable and livewindow for the past few years for the reasons mentioned in the document. I think the best way to go about it is through vendor opinion. Especially with the recent dominance of swerve, the vast majority of teams will care about one or both major vendors. Teams are mostly going to want something that "just works" as much as possible, and that's really the design goal. General strategy I think will be needed: don't fully depricate functionality in one year. Just move the old behavior to be non-default (ex: default livewindow to disabled) and make it possible for teams to get back to old behavior. Give it a year or two and then remove the behavior. Having a dashboard agnostic way to define the look of a dashboard is definitely a key enabler. Still, there is potential here to make it harder for a new dashboard to implement unique features. As a user I wouldn't be horrified if there was a new class called 'webdashboard' which did a lot of the things the same, but enabled new or unique features. However, whether this extendability is done at the json metadata layer or at a class layer... Can't say I see a major difference yet. |
| The `LiveWindow` design philosophy holds that all sensor and actuator objects in code should be accessible via the dashboard while the robot is in Test Mode, *regardless of what the user has done in their robot code.* While this is a nice idea in theory, it places *huge* constraints on the handling of the `Sendable` interface which often interfere with the latter's use as a telemetry handler: | ||
|
|
||
| * While writes to the dashboard work as expected, reads from the dashboard (i.e. calls to the bound setters) are suspended *except* in LiveWindow mode. This is not documented anywhere on the class, and is extremely inconvenient. | ||
| * The API includes an `isActuator` flag which seemingly serves no purpose other than to disable certain widgets on ShuffleBoard (it does not interact with the other dashboards). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These two bullets may be related, as this may be a bug introduced with the Sendable rewrite a couple years ago. We could make setters work in other modes for isActuator=false only (e.g. do the current LW-only behavior only if isActuator=true).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would make sense if we want to keep the feature; I'm not averse to that, but I'd rather focus development effort on a new API entirely at this point. Still might be worth fixing even if it gets deprecated, though?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As for the first point, isn't that how SendableChoosers work? and they work not only when LW is enabled....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SendableChooser is hardcoded, afaik. The Sendable interface does not ever call the consumers except in test mode.
TelemetryDesignDoc.md
Outdated
|
|
||
| Shuffleboard defines tabs, layouts, and widgets in a set of metadata tables, which themselves are ordinary NT entries, and the tab/layout/widget hierarchy is represented by the pathstrings used for the entry keys. These entries live under a reserved `metadata` content root. | ||
|
|
||
| With the addition of an explicit JSON metadata layer to NT4, we can replaec this entirely with a *single* JSON document that defines the structure of the dashboard, located under the `dashboard` topic. This might look something like so: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not depend on the metadata layer, as this should just be a string value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, per discord discussion.
|
|
||
| It's sometimes useful to bind fields or children to paths that do not necessarily follow the in-code object tree - for example, to put certain data under a shared "diagnostics" aggregation even when it is held across different classes in code. | ||
|
|
||
| To do this, users can call `bind`, `publish`, or `subscribe` directly on `Telemetry` instead of on the `TelemetryBuilder`. The APIs will be identically-shaped, and will differ only in the property path locations. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My team hasn't used Command-Based but will next season so I don't know the "right" way to do things. But a shared sensor might be GYRO that is used in autonomous to drive to a heading and in TELEOP is used to flip the joystick for field-orientation. Maybe these are both the drive train class but maybe not. Structure of code isn't a strong point of the students but it seems important for Commands and for this discussion of telemetry. This has to be easy to use by mentors who are barely surviving herding a dozen students who don't know much and as we know don't read much documentation.
| ```java | ||
| @Publish(path="diagnostics") | ||
| double publishedValue; | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use java so this annotation seems simple to use - until we get to the breaking out of the object tree.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no way to specify a non-inferred pathstring without passing it as an argument somewhere. Think of passing a path as saying "ignore where this came from in code, and put it on NT here instead"
|
I'm SLAB-Mr.Thomas on Chief Delphi and I'll make some naïve comments in the file - you guys are way beyond my shallow depth of WPILib architecture knowledge. |
| The "bind" method fulfills the role of `initSendable`, with a few key differences: | ||
|
|
||
| Unlike `Sendable`, both `TelemetryNode` will support hierarchical nesting of implementing objects - that is, in addition to exposing binding handles for fields of primitive data type, it will allow *entire* `TelemetryNode`s to be added as children: | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We shouldn't have to add a new TelemetryNode often, still, it should be fairly easy for adding custom nodes and even similar nodes with vastly different properties that could drastically change the appearance of the (custom) widget needed to display the Node.
Examples might be LimeLightVision has very different properties than the similar functions of my team written RPi vision. Both should be easily displayed by the students once the Nodes have been defined. (I don't think LimeLightVision has even defined a Sendable example - it's just a bunch of getEntry.)
Another example is the color sensor is getting a lot of different implementations (often with co-processors) as teams struggle to work-around I2C problems so what will a Color Sensor TelemetryNode look like as each team gets its own?
We've also written our own code for a LIDAR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WPI classes themselves will have a canonical TelemetryNode implementation (similar to how they have a canonical Sendable implementation now) - if you want to override it you can wrap them and re-define it. Vendor code will remain the responsibility of vendors; it's not clear to me that there's any easy way to enforce that the data shapes of WPILib classes match the data shapes of vendor classes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I understand what you mean by data shapes - what I called properties. The 3D accelerometer has a shape of 3 doubles (X, Y, Z). A color sensor can have a much different shape say 5 ints (C, R, G, B, P).
After reading your design doc I see my team has always misunderstood and misused in multiple ways TEST mode and SmartDashBoard LiveWindow. I ran some tests of TEST mode now and I see 2 majors problems such that TEST mode isn't very useful to us and the "shape" of data is a big one.
SmartDashboard has no idea where to put C, R, G, B, P on the screen in test mode and it doesn't even try to display them. It does make up a half way decent layout while not in test mode.
I was thinking the JSON metadata layer/file mentioned in this design doc was going to store this layout information but maybe that's one level of detail finer than what you had in mind.
A 1-D table would suffice for a layout of most data presented as an array if there are multiple values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is definitely a need to be able to communicate complex data types in an atomic fashion (for both dashboards and coprocessors), but we've not yet been able to figure out an easy to use and performant method for teams (or even vendors) to do this, so largely we've hacked around it with arrays etc. NT supports raw data transport, but time and space efficient serialization/deserialization into data structures is an open problem. Most other frameworks (e.g. ROS, DDS, protobuf) do this with data description files and codegen of the serialization/deserialization bits. NT4's metadata enables communicating the format of the data, so something like protobuf is potentially an option, but what is needed is build system support and documentation. Unfortunately Java makes this even more challenging from a performance standpoint, as with our constrained memory system, we don't necessarily want to be slinging around lots of short lifetime small objects for the GC to pick up later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After reading your design doc I see my team has always misunderstood and misused in multiple ways TEST mode and SmartDashBoard LiveWindow. I ran some tests of TEST mode now and I see 2 majors problems such that TEST mode isn't very useful to us and the "shape" of data is a big one.
SmartDashboard has no idea where to put C, R, G, B, P on the screen in test mode and it doesn't even try to display them. It does make up a half way decent layout while not in test mode.
Note that the color sensor doesn't implement sendable, which is why it didn't appear in test mode.
The way that smartdashboard and shuffleboard currently handle the layout of data is through widgets. As an example, the ADXL_345 accelerometer defines the metadeta tSmartDashboardType as "3AxisAccelerometer". https://github.com/wpilibsuite/allwpilib/blob/1023c34b1c45598e6a925467bc79ab2b5b3d5e4f/wpilibj/src/main/java/edu/wpi/first/wpilibj/ADXL345_I2C.java#L227
In Smartdashboard, there is a ThreeAxisAccelerometer Widget that displays the data. https://github.com/wpilibsuite/SmartDashboard/blob/main/src/main/java/edu/wpi/first/smartdashboard/livewindow/elements/ThreeAxisAccelerometer.java
In Shuffleboard, similarly, the widget is here: https://github.com/wpilibsuite/shuffleboard/blob/main/plugins/base/src/main/java/edu/wpi/first/shuffleboard/plugin/base/widget/ThreeAxisAccelerometerWidget.java
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that, per the suggested implementation in this document, the "widget type" tag will live in the dashboard JSON, not in the metadata.
|
The document discusses the problems with the existing dashboards, but then completely ignores them with the solution. Is the intent to update glass and shuffleboard as part of this proposal? Or is it to leave it up to the users to implement their own dashboard using the new features? |
|
@sciencewhiz The idea is to replace Shuffleboard, eventually, with a webdash (I think we can adopt |
|
I'm hesitant about the widget / dashboard layout section. On one hand, having a cross-dashboard spec is good. On the other, a lot of features aren't shared between dashboards. Additionally, I'm not sure how much we actually need a cross-dashboard layout API: most codebases are used with one or two dashboards for different purposes: a common case being Shuffleboard for drivers and Glass for development. |
Conceivably we could support multiple dashboard named “roots” / keys and the dashboard would default to an appropriately named one and/or provide a drop down selection to choose the desired one. I don’t anticipate glass will ever have support for code defined layouts. Detail display defaults (eg robot size on Field2D) is on the roadmap, but that’s different than layouts. |
|
What you're saying helps my point: widget selection should be separate from dashboard layouts. The former is much easier to support in a cross-platform way, and is more closely tied to specific code elements being published. Therefore my suggestion is that widget selection would be included in the value's metadata. Dashboard layout data (in a special topic, or also in object metadata) would not include widget data. The more difficult problem would be complex widgets that combine data from multiple independent sources, such as plots, Field2d (with multiple objects), and Mechanism2d. |
|
Per discussion on Discord, widgets should be declared as metadata (topic metadata or SmartDashboard-esque metadata topics) rather than in a layout: presentation is more closely tied to the data than where it's presented, and widgets are easier than code-driven layouts for a dashboard to support. The one complication, widgets with multiple independent sources (such as multi-series plots, multi-object Field2d, and Mech2d) would have separate objects and be treated as first-class telemetry data. |
|
What does widget mean in this context? You mean something more than data type, but the actual graphical display? E.g. show this gyro angle as a compass, rather than as something else? |
|
Yes. Something similar to the Shuffleboard API's |
|
@Oblarg a slight issue with layout data being in a single dashboard JSON topic: any update to it (especially if it's distributed on the robot code side) will necessitate a lot of redraws. I'm not a fan of that idea. Maybe it would be better to have it in topic metadata similar to widget selection? If we're doing both layout and widget in metadata, then it probably shouldn't be flat JSON. Also, @PeterJohnson -- is there a way to append to topic metadata rather than replacing the whole JSON string? |
|
Opened wpilibsuite/allwpilib#5177, for continuity, let's concentrate discussion there. |
No description provided.