[telemetry] Add new Telemetry implementation

[PeterJohnson]

Significantly simplified version of #6453. The design approach here is to make Telemetry purely imperative/immediate and write only. Read capabilities would be added as a separate Tunable implementation.

Telemetry is a utility class with only static functions to allow simple use such as Telemetry.log("name", value); (ala System.out.println()), and is intended as the primary user-facing class. Nested (structured) telemetry is available via TelemetryTable, instances of which can be gotten by calling Telemetry.getTable() (or TelemetryTable.getTable() for further nesting).

The Sendable concept equivalent is TelemetryLoggable (not a great name, but naming is hard). Unlike the current Sendable / SmartDashboard.putData(), this is designed to be immediate, not callback-based. The log(TelemetryTable) function of a TelemetryLoggable should immediately log the desired data to the provided TelemetryTable and not store the table for later use.

While we aim to fast-path most types through specific overloads, we do have a generic Object-taking overload to avoid potential overload ambiguity (particularly between StructSerializable and ProtobufSerializable, where many types implement both) and provide a fallback toString path. There's a type registry mechanism to register specific type handlers; the intent here is to use this for things like Unit types, where you might want to both set a property (indicating the unit type) and provide the value as a double.

Backends can be configured at any level; the most specific backend is used based on longest prefix match of the telemetry path (this allows for e.g. setting up NT logging at the top level, but making some tables DataLog-only).

The initial backend implementation takes the type of the first log() call as the "forever" type for that particular name. Trying to later log to the same name with a different type is ignored and emits a warning. Changing types dynamically both significantly increases implementation complexity and will likely result in difficult-to-debug behavior in downstream tooling; it's hard to see the user benefits to supporting this.

TODO:

• Complete DataLog backend
• Add NT backend
• Add no-op backend
• Introspection for struct/protobuf (to get .struct / .proto objects)
• C++ implementation
• Port current use cases of SmartDashboard/Sendable (and remove the implementations)
• Implement int and short arrays for DataLog and NT
• Registry should cache Entry so they are safe to use with ConcurrentMap and can be reset on reset()
• Registry should potentially cache SendableTable objects too?
• RobotBase set up NT as the default global backend
• Unit tests and examples
• Add early no-op checks (avoid work if going to no-op backend)
• Make no-op easier to use (table function instead of needing to use backend API?)
• When new backend registered, remove matching cached entries from tables (or just invalidate all caches)
• Implement type handler(s) for unit types
• Implement warnings on type mismatch
• Port current use of DataLog/NT APIs (in particular, DS joysticks) (TBR)
• Add tunables (maybe separate PR?)

[alan412]

I don't see any locking to make sure this works from multiple threads.

I also don't understand why there are a lot of individual functions with things like "log" naming.

[Gold856]

I also don't understand why there are a lot of individual functions with things like "log" naming.

What do you mean by this? Are you asking why everything is called log or why there's so many overloads? log was chosen because lots of other 3rd party telemetry libraries use that name, and people seem to prefer the shorter name. If you're asking about the overloads, we have them to prevent accidental casting from, say, a double to a float, or a long to an int.

[alan412]

I also don't understand why there are a lot of individual functions with things like "log" naming.

What do you mean by this? Are you asking why everything is called log or why there's so many overloads? log was chosen because lots of other 3rd party telemetry libraries use that name, and people seem to prefer the shorter name. If you're asking about the overloads, we have them to prevent accidental casting from, say, a double to a float, or a long to an int.

I wasn't clear here. This is in reference to TelemetryBackEnd.java

[PeterJohnson]

I also don't understand why there are a lot of individual functions with things like "log" naming.

I wasn't clear here. This is in reference to TelemetryBackEnd.java

I haven't finished implementing the backends, but basically all the backends have type safety and type-specific functionality. So writing a double to a datalog file or NetworkTables is different than writing a string (or an integer). If you're asking why they are uniquely named instead of overloaded, overloading is bad practice with virtual functions (particularly in C++). A bunch of overloaded "log" functions on the user-facing side makes sense for ease of use, but is a potential footgun on the backend.

[PeterJohnson]

I don't see any locking to make sure this works from multiple threads.

It should be thread safe as written already. The backends will have locking/atomics included as needed (there are some synchronized blocks there already in the start of the DataLogSendableBackend implementation). We use ConcurrentMap etc to avoid explicit locking on the frontend (e.g. TelemetryTable caching uses ConcurrentMap), at least in Java--in C++, we'll use explicit mutexes.

[KangarooKoala]

Would it make sense for the type setting and checking to be handled by the telemetry library instead of the telemetry implementation?
I'm thinking of doing the following:

@FunctionalInterface
public interface TelemetryLoggable {
  void updateTelemetry(TelemetryTable table);
  default String getType() {
    return null;
  }
}

public final class TelemetryTable {
  // No need for setType() anymore

  public <T> void log(String name, T value) {
    if (value instanceof TelemetryLoggable v) {
      TelemetryTable table = getTable(name);
      String type = v.getType();
      if (type != null) {
        if (m_type != null) {
          if (!m_type.equals(type)) {
            return;
          }
        } else {
          m_type = type;
          log(".type", type);
        }
      }
      v.updateTelemetry(table);
    } else {
      // ...
    }
  }
}

Unfortunately this makes it hard to implement the equivalent of publishConst()- Maybe the type set could happen after the call to updateTelemetry()?
It also means that you can't specify the type string of a lambda being logged, though I don't know how major of a use case that is.

It makes sense if these things mean we can't really do something like this- I was concerned about having more that implementations need to get right (especially since we don't have [[nodiscard]] in Java) and that we would need to update, but it isn't the biggest issue.

[PeterJohnson]

Yeah, I considered that approach but discarded it because of the publishConst() equivalent problem, although you're right that it could be addressed by setting the type after the call to updateTelemetry (although that may have its own issues with derived classes and super calls?). Definitely worth thinking about, it's still relatively easy to change at this point. The other downside of this approach is it's more expensive in C++ unless we make the interface function return a string_view instead of a string (which is admittedly probably okay, as it's very unlikely to be a dynamic type).

[KangarooKoala]

Shouldn't this go into a separate file to match the other struct definitions?

[KangarooKoala]

Suggested change

	template <typename T>
	void ExpectLastValue(std::string_view path, const T& expected) {
	auto value = mock->GetLastValue<T>(path);
	ASSERT_TRUE(value);
	EXPECT_EQ(expected, *value);
	}

This should work and would reduce boilerplate, though it doesn't work for the types that gtest won't nicely format if the assertion fails. (If it doesn't recognize the type it outputs the hexademical of the raw memory contents- I remember encountering that for failing quaternion equality checks)