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

[wpilib] Tracer Overhaul #7099

Open
wants to merge 62 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 58 commits
Commits
Show all changes
62 commits
Select commit Hold shift + click to select a range
427d3c1
rework tracer
oh-yes-0-fps Sep 19, 2024
2294b0b
fixed CommandScheduler for now
oh-yes-0-fps Sep 19, 2024
890b7c4
reimplemented old tracer on class instances with deprecation warnings
oh-yes-0-fps Sep 20, 2024
2cc9aa4
added cpp version
oh-yes-0-fps Sep 23, 2024
b2d90b5
Merge branch 'main' into tracer
oh-yes-0-fps Sep 23, 2024
de78a0c
Added robot thread naming
oh-yes-0-fps Sep 23, 2024
5a23d75
fix some compilation errors
oh-yes-0-fps Sep 24, 2024
1c9183b
ran fmt
oh-yes-0-fps Sep 24, 2024
7668034
it compiles :flushed:
oh-yes-0-fps Sep 24, 2024
4c545ec
added dprecation ignores in iterative robot base
oh-yes-0-fps Sep 24, 2024
526403d
Fixed Includes in Tracer.cpp
oh-yes-0-fps Sep 24, 2024
1fa81c8
fixed deprecation warnings in watchdog
oh-yes-0-fps Sep 24, 2024
52c74a3
class docs
oh-yes-0-fps Sep 24, 2024
c8d1a62
deprecated scoped tracer and added new equivalent
oh-yes-0-fps Sep 25, 2024
6878049
:sob:
oh-yes-0-fps Sep 25, 2024
7832f67
fix deprecated errors in tracer
oh-yes-0-fps Sep 25, 2024
1ad1f8e
added cpp test
oh-yes-0-fps Sep 25, 2024
b0b6dd6
updated deps in test
oh-yes-0-fps Sep 25, 2024
2600422
fixed javadoc
oh-yes-0-fps Sep 25, 2024
8bb7e73
removed anything to do with epochs from iterative robot and command s…
oh-yes-0-fps Sep 25, 2024
f76185d
Added docs for implicit tracing
oh-yes-0-fps Sep 25, 2024
1415c46
added tracing to named commands
oh-yes-0-fps Sep 25, 2024
f4781b3
Fixed javadoc fr fr
oh-yes-0-fps Sep 25, 2024
3be07d5
fixed some formatting issues
oh-yes-0-fps Sep 25, 2024
ffe9713
fixed cpp unit test
oh-yes-0-fps Sep 25, 2024
c93e2af
added scoped tracer unit test
oh-yes-0-fps Sep 25, 2024
7b168a3
added java unit test
oh-yes-0-fps Sep 25, 2024
a297add
oopsie
oh-yes-0-fps Sep 25, 2024
e224e8a
Fixed javaformat
oh-yes-0-fps Sep 25, 2024
c0cb3cc
javaformat on commands
oh-yes-0-fps Sep 25, 2024
2bb31da
Fixed an issue introduced into the command scheduler
oh-yes-0-fps Sep 25, 2024
03a1c43
Fixed typos
oh-yes-0-fps Sep 25, 2024
dd30d5d
fixed some errors in Tracer.cpp
oh-yes-0-fps Sep 26, 2024
a11c815
added new `Traced` decorator
oh-yes-0-fps Sep 26, 2024
b6f0358
Updated watchdog overrun log message
oh-yes-0-fps Sep 26, 2024
a536db3
ran fmt
oh-yes-0-fps Sep 26, 2024
c1bcc63
added some more unit tests to java
oh-yes-0-fps Sep 26, 2024
71a5717
ran fmt
oh-yes-0-fps Sep 26, 2024
77539c5
fixed some typos
oh-yes-0-fps Sep 26, 2024
7296648
Added a way to prune tracer call stack frames from stack traces on fu…
oh-yes-0-fps Sep 26, 2024
c89cf18
ran fmt
oh-yes-0-fps Sep 26, 2024
057aae9
Refine enabling logic, fix some typos and remove stack trace shenanigans
oh-yes-0-fps Sep 28, 2024
c742e94
added mechanism for substituting tracer state
oh-yes-0-fps Sep 29, 2024
d1a38f7
fixed error with timeslice robot
oh-yes-0-fps Sep 30, 2024
7f68a91
fixed deprecation warnings and added more `addPeriodic` overloads
oh-yes-0-fps Oct 1, 2024
06fb68a
ran fmt
oh-yes-0-fps Oct 1, 2024
2000ef9
Update wpilibj/src/main/java/edu/wpi/first/wpilibj/TimedRobot.java
oh-yes-0-fps Oct 1, 2024
e7b9e2c
Update wpilibj/src/main/java/edu/wpi/first/wpilibj/TimesliceRobot.java
oh-yes-0-fps Oct 3, 2024
3829f2b
Fix epilogue test
oh-yes-0-fps Oct 3, 2024
1721f3c
fixed time robot docs
oh-yes-0-fps Oct 3, 2024
16e2adc
not sure why this isn't working
oh-yes-0-fps Oct 3, 2024
ac78543
added more cpp tests and updated timed robot in cpp
oh-yes-0-fps Oct 3, 2024
c88e29d
ran fmt
oh-yes-0-fps Oct 3, 2024
bd8e779
fixed deprecation warnings
oh-yes-0-fps Oct 3, 2024
e803048
ran fmt
oh-yes-0-fps Oct 3, 2024
322aeba
fmt
oh-yes-0-fps Oct 3, 2024
0cbbb01
fixed missing tracer input
oh-yes-0-fps Oct 3, 2024
b82fa6c
rerun ci
oh-yes-0-fps Oct 3, 2024
5d7f2e3
ran fmt
oh-yes-0-fps Oct 3, 2024
07e643b
I'm so confused
oh-yes-0-fps Oct 3, 2024
2706c93
removed ternary in `PopTraceStack`
oh-yes-0-fps Oct 3, 2024
9272208
fixed tests
oh-yes-0-fps Oct 3, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,9 @@ public static boolean shouldLog(Logged.Importance importance) {
out.println(" robot.addPeriodic(() -> {");
out.println(" update(robot);");
out.println(
" }, config.loggingPeriod.in(Seconds), config.loggingPeriodOffset.in(Seconds));");
" },");
out.println(" \"EpiloguePeriodic\",");
out.println(" config.loggingPeriod.in(Seconds), config.loggingPeriodOffset.in(Seconds));");
out.println(" }");
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,9 @@ public static void bind(edu.wpi.first.epilogue.Example robot) {

robot.addPeriodic(() -> {
update(robot);
}, config.loggingPeriod.in(Seconds), config.loggingPeriodOffset.in(Seconds));
},
"EpiloguePeriodic",
config.loggingPeriod.in(Seconds), config.loggingPeriodOffset.in(Seconds));
}
}
""";
Expand Down Expand Up @@ -256,7 +258,9 @@ public static void bind(edu.wpi.first.epilogue.AlphaBot robot) {

robot.addPeriodic(() -> {
update(robot);
}, config.loggingPeriod.in(Seconds), config.loggingPeriodOffset.in(Seconds));
},
"EpiloguePeriodic",
config.loggingPeriod.in(Seconds), config.loggingPeriodOffset.in(Seconds));
}

/**
Expand Down Expand Up @@ -288,7 +292,9 @@ public static void bind(edu.wpi.first.epilogue.BetaBot robot) {

robot.addPeriodic(() -> {
update(robot);
}, config.loggingPeriod.in(Seconds), config.loggingPeriodOffset.in(Seconds));
},
"EpiloguePeriodic",
config.loggingPeriod.in(Seconds), config.loggingPeriodOffset.in(Seconds));
}
}
""";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import edu.wpi.first.util.sendable.Sendable;
import edu.wpi.first.util.sendable.SendableBuilder;
import edu.wpi.first.util.sendable.SendableRegistry;
import edu.wpi.first.wpilibj.Tracer;
import java.util.Collection;
import java.util.HashSet;
import java.util.Set;
Expand Down Expand Up @@ -581,6 +582,29 @@ public WrapperCommand withName(String name) {
return wrapper;
}

/**
* Decorates this Command so that it's `Execute` method is implicitly timed using the {@link
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's -> its

* Tracer} class. This decorator also names the command similarly to the {@link #withName(String)}
* method.
*
* @param name name
* @return the decorated Command
*/
public WrapperCommand traced(String name) {
WrapperCommand wrapper =
new WrapperCommand(Command.this) {
@Override
public void execute() {
// Avoid `Trace.traceFunc` to keep it out of the stack trace
Tracer.startTrace(name);
super.execute();
Tracer.endTrace();
}
};
wrapper.setName(name);
return wrapper;
}

@Override
public void initSendable(SendableBuilder builder) {
builder.setSmartDashboardType("Command");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import edu.wpi.first.wpilibj.RobotBase;
import edu.wpi.first.wpilibj.RobotState;
import edu.wpi.first.wpilibj.TimedRobot;
import edu.wpi.first.wpilibj.Tracer;
import edu.wpi.first.wpilibj.Watchdog;
import edu.wpi.first.wpilibj.event.EventLoop;
import edu.wpi.first.wpilibj.livewindow.LiveWindow;
Expand Down Expand Up @@ -97,7 +98,13 @@ public static synchronized CommandScheduler getInstance() {
private final List<Optional<Command>> m_toCancelInterruptors = new ArrayList<>();
private final Set<Command> m_endingCommands = new LinkedHashSet<>();

private final Watchdog m_watchdog = new Watchdog(TimedRobot.kDefaultPeriod, () -> {});
private final Watchdog m_watchdog =
new Watchdog(
TimedRobot.kDefaultPeriod,
() -> {
System.out.println(
"CommandScheduler loop overrun, check NetworkTables for timing info");
});

CommandScheduler() {
HAL.report(tResourceType.kResourceType_Command, tInstances.kCommand2_Scheduler);
Expand Down Expand Up @@ -170,8 +177,6 @@ private void initCommand(Command command, Set<Subsystem> requirements) {
for (Consumer<Command> action : m_initActions) {
action.accept(command);
}

m_watchdog.addEpoch(command.getName() + ".initialize()");
}

/**
Expand Down Expand Up @@ -251,29 +256,32 @@ public void schedule(Command... commands) {
* have their end methods called and are removed.
*
* <p>Any subsystems not being used as requirements have their default methods started.
*
* <p>This method is implicitly timed using the {@link Tracer} class.
*/
public void run() {
if (m_disabled) {
return;
}

Tracer.startTrace("CommandScheduler");
m_watchdog.reset();

// Run the periodic method of all registered subsystems.
for (Subsystem subsystem : m_subsystems.keySet()) {
subsystem.periodic();
Tracer.traceFunc(subsystem.getName() + "Periodic", subsystem::periodic);
if (RobotBase.isSimulation()) {
subsystem.simulationPeriodic();
Tracer.traceFunc(subsystem.getName() + "SimulationPeriodic", subsystem::simulationPeriodic);
}
m_watchdog.addEpoch(subsystem.getName() + ".periodic()");
}

// Cache the active instance to avoid concurrency problems if setActiveLoop() is called from
// inside the button bindings.
EventLoop loopCache = m_activeButtonLoop;
// Poll buttons for new commands to add.
loopCache.poll();
m_watchdog.addEpoch("buttons.run()");
Tracer.traceFunc("PollButtons", loopCache::poll);

Tracer.startTrace("Commands");
m_inRunLoop = true;
boolean isDisabled = RobotState.isDisabled();
// Run scheduled commands, remove finished commands.
Expand All @@ -289,7 +297,6 @@ public void run() {
for (Consumer<Command> action : m_executeActions) {
action.accept(command);
}
m_watchdog.addEpoch(command.getName() + ".execute()");
if (command.isFinished()) {
m_endingCommands.add(command);
command.end(false);
Expand All @@ -300,7 +307,6 @@ public void run() {
iterator.remove();

m_requirements.keySet().removeAll(command.getRequirements());
m_watchdog.addEpoch(command.getName() + ".end(false)");
}
}
m_inRunLoop = false;
Expand All @@ -325,12 +331,9 @@ public void run() {
schedule(subsystemCommand.getValue());
}
}
Tracer.endTrace(); // Commands

m_watchdog.disable();
if (m_watchdog.isExpired()) {
System.out.println("CommandScheduler loop overrun");
m_watchdog.printEpochs();
}
oh-yes-0-fps marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand Down Expand Up @@ -487,7 +490,6 @@ private void cancel(Command command, Optional<Command> interruptor) {
m_endingCommands.remove(command);
m_scheduledCommands.remove(command);
m_requirements.keySet().removeAll(command.getRequirements());
m_watchdog.addEpoch(command.getName() + ".end(true)");
}

/** Cancels all commands that are currently scheduled. */
Expand Down Expand Up @@ -530,11 +532,6 @@ public void enable() {
m_disabled = false;
}

/** Prints list of epochs added so far and their times. */
public void printWatchdogEpochs() {
m_watchdog.printEpochs();
}

/**
* Adds an action to perform on the initialization of any command by the scheduler.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,17 @@ public interface Subsystem {
* subsystem-specific state that you don't want to offload to a {@link Command}. Teams should try
* to be consistent within their own codebases about which responsibilities will be handled by
* Commands, and which will be handled here.
*
* <p>This method is implicitly timed using the {@link edu.wpi.first.wpilibj.Tracer}.
*/
default void periodic() {}

/**
* This method is called periodically by the {@link CommandScheduler}. Useful for updating
* subsystem-specific state that needs to be maintained for simulations, such as for updating
* {@link edu.wpi.first.wpilibj.simulation} classes and setting simulated sensor readings.
*
* <p>This method is implicitly timed using the {@link edu.wpi.first.wpilibj.Tracer}.
*/
default void simulationPeriodic() {}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,10 @@ CommandPtr Command::WithName(std::string_view name) && {
return std::move(*this).ToPtr().WithName(name);
}

CommandPtr Command::Traced(std::string_view name) && {
return std::move(*this).ToPtr().Traced(name);
}

void Command::Schedule() {
CommandScheduler::GetInstance().Schedule(this);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@
#include "frc2/command/CommandPtr.h"

#include <memory>
#include <string>
#include <utility>
#include <vector>

#include <frc/Errors.h>
#include <frc/Tracer.h>

#include "frc2/command/CommandScheduler.h"
#include "frc2/command/ConditionalCommand.h"
Expand Down Expand Up @@ -250,6 +252,30 @@ CommandPtr CommandPtr::WithName(std::string_view name) && {
return std::move(wrapper).ToPtr();
}

namespace {
class TracedCommand : public WrapperCommand {
public:
TracedCommand(std::unique_ptr<Command>&& command, std::string_view name)
: WrapperCommand(std::move(command)), m_name(name) {}

void Execute() override {
frc::Tracer::StartTrace(m_name);
WrapperCommand::Execute();
frc::Tracer::EndTrace();
}

private:
std::string m_name;
};
} // namespace

CommandPtr CommandPtr::Traced(std::string_view name) && {
AssertValid();
m_ptr = std::make_unique<TracedCommand>(std::move(m_ptr), name);
m_ptr->SetName(name);
return std::move(*this);
}

Command* CommandPtr::get() const& {
AssertValid();
return m_ptr.get();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include <frc/RobotBase.h>
#include <frc/RobotState.h>
#include <frc/TimedRobot.h>
#include <frc/Tracer.h>
#include <frc/livewindow/LiveWindow.h>
#include <hal/FRCUsageReporting.h>
#include <hal/HALBase.h>
Expand Down Expand Up @@ -71,7 +72,9 @@ static bool ContainsKey(const TMap& map, TKey keyToCheck) {

CommandScheduler::CommandScheduler()
: m_impl(new Impl), m_watchdog(frc::TimedRobot::kDefaultPeriod, [] {
std::puts("CommandScheduler loop time overrun.");
std::puts(
"CommandScheduler loop overrun, check NetworkTables for timing "
"info");
}) {
HAL_Report(HALUsageReporting::kResourceType_Command,
HALUsageReporting::kCommand2_Scheduler);
Expand Down Expand Up @@ -154,7 +157,6 @@ void CommandScheduler::Schedule(Command* command) {
for (auto&& action : m_impl->initActions) {
action(*command);
}
m_watchdog.AddEpoch(command->GetName() + ".Initialize()");
}
}

Expand All @@ -179,24 +181,29 @@ void CommandScheduler::Run() {
return;
}

frc::Tracer::StartTrace("CommandScheduler");
m_watchdog.Reset();

// Run the periodic method of all registered subsystems.
for (auto&& subsystem : m_impl->subsystems) {
subsystem.getFirst()->Periodic();
frc::Tracer::TraceFunc(
subsystem.getFirst()->GetName() + "Periodic",
std::bind(&Subsystem::Periodic, subsystem.getFirst()));
if constexpr (frc::RobotBase::IsSimulation()) {
subsystem.getFirst()->SimulationPeriodic();
frc::Tracer::TraceFunc(
subsystem.getFirst()->GetName() + "SimulationPeriodic",
std::bind(&Subsystem::SimulationPeriodic, subsystem.getFirst()));
}
m_watchdog.AddEpoch(subsystem.getFirst()->GetName() + ".Periodic()");
}

// Cache the active instance to avoid concurrency problems if SetActiveLoop()
// is called from inside the button bindings.
frc::EventLoop* loopCache = m_impl->activeButtonLoop;
// Poll buttons for new commands to add.
loopCache->Poll();
m_watchdog.AddEpoch("buttons.Run()");
frc::Tracer::TraceFunc("PollButtons",
std::bind(&frc::EventLoop::Poll, loopCache));

frc::Tracer::StartTrace("Commands");
m_impl->inRunLoop = true;
bool isDisabled = frc::RobotState::IsDisabled();
// Run scheduled commands, remove finished commands.
Expand All @@ -210,8 +217,6 @@ void CommandScheduler::Run() {
for (auto&& action : m_impl->executeActions) {
action(*command);
}
m_watchdog.AddEpoch(command->GetName() + ".Execute()");

if (command->IsFinished()) {
m_impl->endingCommands.insert(command);
command->End(false);
Expand All @@ -224,8 +229,6 @@ void CommandScheduler::Run() {
for (auto&& requirement : command->GetRequirements()) {
m_impl->requirements.erase(requirement);
}

m_watchdog.AddEpoch(command->GetName() + ".End(false)");
}
}
m_impl->inRunLoop = false;
Expand All @@ -249,11 +252,9 @@ void CommandScheduler::Run() {
Schedule({subsystem.getSecond().get()});
}
}
frc::Tracer::EndTrace();

m_watchdog.Disable();
if (m_watchdog.IsExpired()) {
m_watchdog.PrintEpochs();
}
oh-yes-0-fps marked this conversation as resolved.
Show resolved Hide resolved
}

void CommandScheduler::RegisterSubsystem(Subsystem* subsystem) {
Expand Down Expand Up @@ -356,7 +357,6 @@ void CommandScheduler::Cancel(Command* command,
m_impl->requirements.erase(requirement.first);
}
}
m_watchdog.AddEpoch(command->GetName() + ".End(true)");
}

void CommandScheduler::Cancel(Command* command) {
Expand Down Expand Up @@ -432,10 +432,6 @@ void CommandScheduler::Enable() {
m_impl->disabled = false;
}

void CommandScheduler::PrintWatchdogEpochs() {
m_watchdog.PrintEpochs();
}

void CommandScheduler::OnCommandInitialize(Action action) {
m_impl->initActions.emplace_back(std::move(action));
}
Expand Down
Loading
Loading