Skip to content

Commit

Permalink
Allow CommandPtrs to be owned by the scheduler
Browse files Browse the repository at this point in the history
Previously users would have to keep track of dynamically created CommandPtrs. This adds an ownership-taking version of schedule which places the command in a temporary store in the scheduler. The command will be freed when the command's lifecycle ends.
  • Loading branch information
rzblue committed Oct 15, 2024
1 parent bedfc09 commit 58277a9
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,11 @@ class CommandScheduler::Impl {
wpi::SmallVector<Command*, 4> toCancelCommands;
wpi::SmallVector<std::optional<Command*>, 4> toCancelInterruptors;
wpi::SmallSet<Command*, 4> endingCommands;

// Map of Command* -> CommandPtr for CommandPtrs transferred to the scheduler
// via Schedule(CommandPtr&&). These are erased (destroyed) at the very end of
// the loop cycle when the command lifecycle is complete.
std::unordered_map<Command*, CommandPtr> ownedCommands;
};

template <typename TMap, typename TKey>
Expand Down Expand Up @@ -174,6 +179,12 @@ void CommandScheduler::Schedule(const CommandPtr& command) {
Schedule(command.get());
}

void CommandScheduler::Schedule(CommandPtr&& command) {
auto ptr = command.get();
m_impl->ownedCommands.emplace(ptr, std::move(command));
Schedule(ptr);
}

void CommandScheduler::Run() {
if (m_impl->disabled) {
return;
Expand Down Expand Up @@ -226,6 +237,8 @@ void CommandScheduler::Run() {
}

m_watchdog.AddEpoch(command->GetName() + ".End(false)");
// remove owned commands after everything else is done
m_impl->ownedCommands.erase(command);
}
}
m_impl->inRunLoop = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,13 +92,26 @@ class CommandScheduler final : public wpi::Sendable,
*/
void Schedule(const CommandPtr& command);

/**
* Schedules a command for execution. Does nothing if the command is already
* scheduled. If a command's requirements are not available, it will only be
* started if all the commands currently using those requirements are
* interruptible. If this is the case, they will be interrupted and the
* command will be scheduled.
*
* @param command the command to schedule
*/
void Schedule(CommandPtr&& command);

/**
* Schedules a command for execution. Does nothing if the command is already
* scheduled. If a command's requirements are not available, it will only be
* started if all the commands currently using those requirements have been
* scheduled as interruptible. If this is the case, they will be interrupted
* and the command will be scheduled.
*
* The pointer must remain valid through the entire lifecycle of the command.
*
* @param command the command to schedule
*/
void Schedule(Command* command);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,3 +171,52 @@ TEST_F(SchedulerTest, ScheduleScheduledNoOp) {

EXPECT_EQ(counter, 1);
}

class TrackDestroyCommand
: public frc2::CommandHelper<Command, TrackDestroyCommand> {
public:
TrackDestroyCommand(wpi::unique_function<void()> deleteFunc)
: m_deleteFunc{std::move(deleteFunc)} {}
TrackDestroyCommand(TrackDestroyCommand&& other)
: m_deleteFunc{std::exchange(other.m_deleteFunc, [] {})} {}
TrackDestroyCommand& operator=(TrackDestroyCommand&& other) {
m_deleteFunc = std::exchange(other.m_deleteFunc, [] {});
return *this;
}
~TrackDestroyCommand() override { m_deleteFunc(); }

private:
wpi::unique_function<void()> m_deleteFunc;
};

TEST_F(SchedulerTest, ScheduleCommandPtr) {
CommandScheduler scheduler = GetScheduler();
int destructionCounter = 0;
int runCounter = 0;

bool finish = false;

{
auto commandPtr =
TrackDestroyCommand([&destructionCounter] { destructionCounter++; })
.AlongWith(std::move(
frc2::InstantCommand([&runCounter] { runCounter++; }).ToPtr()))
.Until([&finish] { return finish; });
EXPECT_EQ(destructionCounter, 0) << "Composition should not delete command";

scheduler.Schedule(std::move(commandPtr));
EXPECT_EQ(destructionCounter, 0)
<< "Scheduling should not delete CommandPtr";
}
EXPECT_EQ(destructionCounter, 0)
<< "Scheduler should own CommandPtr after scheduling";
scheduler.Run();
EXPECT_EQ(runCounter, 1);
EXPECT_EQ(destructionCounter, 0) << "Scheduler should not destroy CommandPtr "
"until command lifetime is complete";
finish = true;
scheduler.Run();
EXPECT_EQ(runCounter, 1);
EXPECT_EQ(destructionCounter, 1)
<< "Scheduler should delete command after command completes";
}

0 comments on commit 58277a9

Please sign in to comment.