Skip to content
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

Add loop timing telemetry #609

Open
PeterJohnson opened this issue Aug 19, 2017 · 17 comments · May be fixed by #7099
Open

Add loop timing telemetry #609

PeterJohnson opened this issue Aug 19, 2017 · 17 comments · May be fixed by #7099
Labels
component: telemetry High level telemetry functionality type: discussion Questions, proposals and info that requires discussion. type: feature Brand new functionality, features, pages, workflows, endpoints, etc.

Comments

@PeterJohnson
Copy link
Member

It would be useful for users to know loop timing of various pieces of their code. This should be published via NT so it can be recorded and played back on the dashboard. For command based this could be pretty fine-grained.

@PeterJohnson PeterJohnson added the type: feature Brand new functionality, features, pages, workflows, endpoints, etc. label Aug 19, 2017
@gerth2
Copy link
Contributor

gerth2 commented May 7, 2018

Could this also be exposed through an API inside user software? Being able to read it directly (rather than only NT) would be useful to us.

@auscompgeek
Copy link
Member

@gerth2 One can already read directly from NT on the robot (without having to wait for updates), so that shouldn't be a problem.

@virtuald
Copy link
Member

Yeah, I've been bit by this sort of thing a lot. It would be good to do this.

@gerth2
Copy link
Contributor

gerth2 commented May 16, 2018

Would a histogram of execution times be useful as well? Definitely wouldn't be needed at first-pass, but our team would find use in it.

@auscompgeek
Copy link
Member

@gerth2 if you have a NT client that logs the values as they are updated, you could rather easily use your favourite data analysis tools to create a histogram from that.

@gerth2
Copy link
Contributor

gerth2 commented May 16, 2018

@auscompgeek makes sense!

@PeterJohnson PeterJohnson added the type: discussion Questions, proposals and info that requires discussion. label Jul 30, 2018
@Starlight220
Copy link
Member

Hasn't this been resolved by the Tracer class PRs?
If teams want to post the data to NT they can use the overload that accepts an output lambda. Perhaps something should be added to frc-docs about it, but I think this is resolved. If it isn't, then what needs to be done should be cleared up.

@auscompgeek
Copy link
Member

My understanding is that there is still no fine-grained timing telemetry for the command scheduler, which would definitely be desirable.

@Starlight220
Copy link
Member

There is a Watchdog instance used in CommandScheduler, though I don't think there's any user-facing API for receiving the printed epochs (they're printed directly in case of loop overrun). Is there anything else needed?

@calcmogul
Copy link
Member

calcmogul commented Sep 17, 2020

What's implemented now doesn't address the intent of the original post. The idea is that Tracer data would be published to NetworkTables so teams can easily see it on every loop iteration (well, it gets actually sent out at 100ms by NT and intermediate values are dropped, so network I/O shouldn't be too bad).

Watchdog only warns on overruns, but it's useful to know how much of the timing budget you're using in nominal cases too. Having more information like this makes it easier to optimize or find performance concerns early and fix them. (In other words, "the first rule of performance is to measure".)

@Starlight220
Copy link
Member

So perhaps a method in CommandScheduler that logs a listener that receives the Tracer data at the end of each loop? Then teams are free to do whatever they want with that data, including sending it over NT in a format that they prefer. Would this address the OP?

@prateekma
Copy link
Member

For C++, you could create a simple ScopedTracer class (that either inherits Tracer or uses composition) such that the timer begins when the object is constructed (with an NT entry as a parameter) and the timer ends (and data is published) when the object is destroyed.

void Periodic() {
  frc::ScopedTracer timer(m_telemetryEntry);
  ...
}

@Starlight220
Copy link
Member

Why can't C++ get the same treatment as Java and just have the data published to a listener?

@prateekma
Copy link
Member

prateekma commented Sep 17, 2020

Why can't C++ get the same treatment as Java and just have the data published to a listener?

You can create a similar version of a ScopedTracer that takes a listener instead; perhaps a secondary constructor that takes an NT entry for those who don't want to deal with that.

The main point I was trying to make was that it would be nice for the C++ variant to be scoped so that teams wouldn't have to call start() and stop() manually (or whatever the API ends up being for Java).

@Starlight220
Copy link
Member

You missed my point. I think that the team can (and should) be responsible for posting the data to wherever they want, whether it be NT, reportWarning(), or any other data logging format a team has. We accept a listener and give it the data. The rest is the team's responsibility; assuming they want the data.

@prateekma
Copy link
Member

I think that the team can (and should) be responsible for posting the data to wherever they want, whether it be NT, reportWarning(), or any other data logging format a team has.

Yes, I agree. As I pointed in my last comment, the primary constructor should be a listener, but we can have a secondary constructor that takes in an NT Entry (which calls the primary constructor internally) because that is a common use case and specifically requested by the OP.

@Starlight220
Copy link
Member

I want to avoid the middleman "dummy" class - just have an overload that takes an NTEntry, and calls the lambda overload with entry.putString() or whatever.

pjbuterbaugh pushed a commit to pjbuterbaugh/allwpilib that referenced this issue Jun 15, 2023
* Use dynamic width for sidebar version picker

* Update frc-rtd.css

* Remove unnecessary width override

* Landscape support
@calcmogul calcmogul added the component: telemetry High level telemetry functionality label Aug 3, 2023
@oh-yes-0-fps oh-yes-0-fps linked a pull request Sep 25, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: telemetry High level telemetry functionality type: discussion Questions, proposals and info that requires discussion. type: feature Brand new functionality, features, pages, workflows, endpoints, etc.
Projects
None yet
7 participants